Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Correct boot flag #245

Merged
merged 3 commits into from
Jun 29, 2015
Merged

Correct boot flag #245

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4df5994
set only coresponding boot flag (bnc930903)
jreidinger Jun 29, 2015
7f1b450
changes
jreidinger Jun 29, 2015
399397c
more descriptive changes
jreidinger Jun 29, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions package/yast2-bootloader.changes
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
-------------------------------------------------------------------
Mon Jun 29 12:46:58 UTC 2015 - jreidinger@suse.com

- set only proper boot boot flags, otherwise it can confuse some
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicated boot ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, thanks for catch

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The term proper boot flags is a bit vague, could you be more specific if possible?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly. I see that the bug report does not help much because you wrote it yourself :) but please try a user's perspective: if the old code failed, the circumstances and symptoms would be such and such. Probably include "Distinguish between MSDOS and GPT tables when setting boot flags" ?

firmware (bnc#930903)
- 3.1.136

-------------------------------------------------------------------
Mon Jun 22 11:01:16 UTC 2015 - jreidinger@suse.com

Expand Down
2 changes: 1 addition & 1 deletion package/yast2-bootloader.spec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@


Name: yast2-bootloader
Version: 3.1.135
Version: 3.1.136
Release: 0

BuildRoot: %{_tmppath}/%{name}-%{version}-build
Expand Down
10 changes: 6 additions & 4 deletions src/lib/bootloader/mbr_update.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,13 @@ def activate_partitions
next unless can_activate_partition?(num)

log.info "Activating partition #{num} on #{mbr_dev}"
# this is needed only on gpt disks but we run it always
# anyway; parted just fails, then
set_parted_flag(mbr_dev, num, "legacy_boot")
# set corresponding flag only bnc#930903
if mbr_is_gpt?
out = set_parted_flag(mbr_dev, num, "legacy_boot")
else
out = set_parted_flag(mbr_dev, num, "boot")
end

out = set_parted_flag(mbr_dev, num, "boot")
ret &&= out["exit"].zero?
end
ret
Expand Down
224 changes: 122 additions & 102 deletions test/mbr_update_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,112 +176,132 @@
allow(Yast::WFM).to receive(:Execute).and_return("exit" => 0, "stdout" => "")
end

it "sets boot flag on all partitions in Bootloader devices" do
allow(Yast::BootCommon).to receive(:GetBootloaderDevices)
.and_return(["/dev/sda1", "/dev/sdb1"])

expect(Yast::WFM).to receive(:Execute)
.with(anything, /parted -s \/dev\/sda set 1 boot on/)
.and_return("exit" => 0)
subject.run
end

it "resets all old boot flags on disk before set boot flag" do
allow(Yast::BootCommon).to receive(:GetBootloaderDevices)
.and_return(["/dev/sda1", "/dev/sdb1"])

parted_output = "BYT;\n" \
"/dev/sda:500GB:scsi:512:4096:gpt:ATA WDC WD5000BPKT-7:;\n" \
"1:1049kB:165MB:164MB:fat16:primary:boot, legacy_boot;\n" \
"2:165MB:8760MB:8595MB:linux-swap(v1):primary:;\n" \
"3:8760MB:30.2GB:21.5GB:ext4:primary:boot;\n" \
"4:30.2GB:500GB:470GB:ext4:primary:legacy_boot;"

allow(Yast::WFM).to receive(:Execute)
.with(anything, /parted -m \/dev\/sda print/)
.and_return(
"exit" => 0,
"stdout" => parted_output
)
expect(Yast::WFM).to receive(:Execute)
.with(anything, /parted -s \/dev\/sda set 1 boot off/)
.and_return(
"exit" => 0,
"stdout" => parted_output
context "disk label is DOS mbr" do
before do
allow(Yast::Storage).to receive(:GetTargetMap).and_return(
double(:fetch => { "label" => "msdos" },
:[] => { "label" => "msdos" }
)
)
expect(Yast::WFM).to receive(:Execute)
.with(anything, /parted -s \/dev\/sda set 3 boot off/)
.and_return(
"exit" => 0,
"stdout" => parted_output
)

subject.run
end

it "returns false if any setting of boot flag failed" do
allow(Yast::BootCommon).to receive(:GetBootloaderDevices)
.and_return(["/dev/sda1", "/dev/sdb1"])

allow(Yast::WFM).to receive(:Execute)
.with(anything, /parted -s \/dev\/sda set 1 boot on/)
.and_return("exit" => 1)

expect(subject.run).to be false
end

it "sets boot flag on all partitions in Bootloader devices" do
allow(Yast::BootCommon).to receive(:GetBootloaderDevices)
.and_return(["/dev/sda1", "/dev/sdb1"])

expect(Yast::WFM).to receive(:Execute)
.with(anything, /parted -s \/dev\/sda set 1 boot on/)
.and_return("exit" => 0)
subject.run
end

it "resets all old boot flags on disk before set boot flag" do
allow(Yast::BootCommon).to receive(:GetBootloaderDevices)
.and_return(["/dev/sda1", "/dev/sdb1"])

parted_output = "BYT;\n" \
"/dev/sda:500GB:scsi:512:4096:gpt:ATA WDC WD5000BPKT-7:;\n" \
"1:1049kB:165MB:164MB:fat16:primary:boot, legacy_boot;\n" \
"2:165MB:8760MB:8595MB:linux-swap(v1):primary:;\n" \
"3:8760MB:30.2GB:21.5GB:ext4:primary:boot;\n" \
"4:30.2GB:500GB:470GB:ext4:primary:legacy_boot;"

allow(Yast::WFM).to receive(:Execute)
.with(anything, /parted -m \/dev\/sda print/)
.and_return(
"exit" => 0,
"stdout" => parted_output
)
expect(Yast::WFM).to receive(:Execute)
.with(anything, /parted -s \/dev\/sda set 1 boot off/)
.and_return(
"exit" => 0,
"stdout" => parted_output
)
expect(Yast::WFM).to receive(:Execute)
.with(anything, /parted -s \/dev\/sda set 3 boot off/)
.and_return(
"exit" => 0,
"stdout" => parted_output
)

subject.run
end

it "returns false if any setting of boot flag failed" do
allow(Yast::BootCommon).to receive(:GetBootloaderDevices)
.and_return(["/dev/sda1", "/dev/sdb1"])

allow(Yast::WFM).to receive(:Execute)
.with(anything, /parted -s \/dev\/sda set 1 boot on/)
.and_return("exit" => 1)

expect(subject.run).to be false
end
end

it "sets legacy_boot flag on all partitions in Bootloader devices" do
allow(Yast::BootCommon).to receive(:GetBootloaderDevices)
.and_return(["/dev/sda1", "/dev/sdb1"])

expect(Yast::WFM).to receive(:Execute)
.with(anything, /parted -s \/dev\/sda set 1 legacy_boot on/)
.and_return("exit" => 0)
subject.run
end

it "resets all old boot flags on disk before set boot flag" do
allow(Yast::BootCommon).to receive(:GetBootloaderDevices)
.and_return(["/dev/sda1", "/dev/sdb1"])

parted_output = "BYT;\n" \
"/dev/sda:500GB:scsi:512:4096:gpt:ATA WDC WD5000BPKT-7:;\n" \
"1:1049kB:165MB:164MB:fat16:primary:boot, legacy_boot;\n" \
"2:165MB:8760MB:8595MB:linux-swap(v1):primary:;\n" \
"3:8760MB:30.2GB:21.5GB:ext4:primary:boot;\n" \
"4:30.2GB:500GB:470GB:ext4:primary:legacy_boot;"

allow(Yast::WFM).to receive(:Execute)
.with(anything, /parted -m \/dev\/sda print/)
.and_return(
"exit" => 0,
"stdout" => parted_output
)
expect(Yast::WFM).to receive(:Execute)
.with(anything, /parted -s \/dev\/sda set 1 legacy_boot off/)
.and_return(
"exit" => 0,
"stdout" => parted_output
context "disk label is GPT" do
before do
allow(Yast::Storage).to receive(:GetTargetMap).and_return(
double(:fetch => { "label" => "gpt" },
:[] => { "label" => "gpt" }
)
)
expect(Yast::WFM).to receive(:Execute)
.with(anything, /parted -s \/dev\/sda set 4 legacy_boot off/)
.and_return(
"exit" => 0,
"stdout" => parted_output
)

subject.run
end

it "do not return false if setting legacy_boot failed" do
allow(Yast::BootCommon).to receive(:GetBootloaderDevices)
.and_return(["/dev/sda1", "/dev/sdb1"])

allow(Yast::WFM).to receive(:Execute)
.with(anything, /parted -s \/dev\/sda set 1 legacy_boot on/)
.and_return("exit" => 1)

expect(subject.run).to be true
end

it "sets legacy_boot flag on all partitions in Bootloader devices" do
allow(Yast::BootCommon).to receive(:GetBootloaderDevices)
.and_return(["/dev/sda1", "/dev/sdb1"])

expect(Yast::WFM).to receive(:Execute)
.with(anything, /parted -s \/dev\/sda set 1 legacy_boot on/)
.and_return("exit" => 0)
subject.run
end

it "resets all old boot flags on disk before set boot flag" do
allow(Yast::BootCommon).to receive(:GetBootloaderDevices)
.and_return(["/dev/sda1", "/dev/sdb1"])

parted_output = "BYT;\n" \
"/dev/sda:500GB:scsi:512:4096:gpt:ATA WDC WD5000BPKT-7:;\n" \
"1:1049kB:165MB:164MB:fat16:primary:boot, legacy_boot;\n" \
"2:165MB:8760MB:8595MB:linux-swap(v1):primary:;\n" \
"3:8760MB:30.2GB:21.5GB:ext4:primary:boot;\n" \
"4:30.2GB:500GB:470GB:ext4:primary:legacy_boot;"

allow(Yast::WFM).to receive(:Execute)
.with(anything, /parted -m \/dev\/sda print/)
.and_return(
"exit" => 0,
"stdout" => parted_output
)
expect(Yast::WFM).to receive(:Execute)
.with(anything, /parted -s \/dev\/sda set 1 legacy_boot off/)
.and_return(
"exit" => 0,
"stdout" => parted_output
)
expect(Yast::WFM).to receive(:Execute)
.with(anything, /parted -s \/dev\/sda set 4 legacy_boot off/)
.and_return(
"exit" => 0,
"stdout" => parted_output
)

subject.run
end

it "returns false if setting legacy_boot failed" do
allow(Yast::BootCommon).to receive(:GetBootloaderDevices)
.and_return(["/dev/sda1", "/dev/sdb1"])

allow(Yast::WFM).to receive(:Execute)
.with(anything, /parted -s \/dev\/sda set 1 legacy_boot on/)
.and_return("exit" => 1)

expect(subject.run).to be false
end
end

it "do not set any flag on old DOS MBR for logical partitions" do
Expand Down