diff --git a/package/yast2-bootloader.changes b/package/yast2-bootloader.changes index e304702df..1cc9fc4b1 100644 --- a/package/yast2-bootloader.changes +++ b/package/yast2-bootloader.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Tue Aug 30 08:42:25 UTC 2016 - jreidinger@suse.com + +- fix crash when activating partition on md raid (bnc#995627) +- 3.1.202 + ------------------------------------------------------------------- Fri Aug 26 09:25:49 UTC 2016 - jsrain@suse.cz diff --git a/package/yast2-bootloader.spec b/package/yast2-bootloader.spec index 330e7812d..61ff823c5 100644 --- a/package/yast2-bootloader.spec +++ b/package/yast2-bootloader.spec @@ -17,7 +17,7 @@ Name: yast2-bootloader -Version: 3.1.201 +Version: 3.1.202 Release: 0 BuildRoot: %{_tmppath}/%{name}-%{version}-build diff --git a/src/lib/bootloader/mbr_update.rb b/src/lib/bootloader/mbr_update.rb index feb302432..8819e53d6 100644 --- a/src/lib/bootloader/mbr_update.rb +++ b/src/lib/bootloader/mbr_update.rb @@ -1,6 +1,7 @@ require "yast" require "bootloader/boot_record_backup" +require "bootloader/stage1_device" require "yast2/execute" Yast.import "Arch" @@ -45,7 +46,7 @@ def create_backups end def mbr_is_gpt? - mbr_storage_object = Yast::Storage.GetTargetMap[mbr_disk] + mbr_storage_object = target_map[mbr_disk] raise "Cannot find in storage mbr disk #{mbr_disk}" unless mbr_storage_object mbr_type = mbr_storage_object["label"] log.info "mbr type = #{mbr_type}" @@ -138,24 +139,35 @@ def disks_to_rewrite ret = [mbr_disk] # Add to disks only if part of raid on base devices lives on mbr_disk ret.concat(mbrs) if mbrs.include?(mbr_disk) + # get only real disks + ret = ret.each_with_object([]) do |disk, res| + res.concat(::Bootloader::Stage1Device.new(disk).real_devices) + end ret.uniq end def first_base_device_to_boot(md_device) - md = Yast::BootStorage.Md2Partitions(md_device) - md.reduce do |res, items| - device, bios_id = items - next device unless res + md = ::Bootloader::Stage1Device.new(md_device) + md.real_devices.min_by { |device| bios_id_for(device) } + end - bios_id < md[res] ? device : res - end + MAX_BIOS_ID = 1000 + def bios_id_for(device) + disk = Yast::Storage.GetDiskPartition(device)["disk"] + disk_info = target_map[disk] + return MAX_BIOS_ID unless disk_info + + bios_id = disk_info["bios_id"] + # prefer device without bios id over ones without disk info + return MAX_BIOS_ID - 1 if !bios_id || bios_id !~ /0x[0-9a-fA-F]+/ + + bios_id[2..-1].to_i(16) - 0x80 end # List of partition for disk that can be used for setting boot flag def activatable_partitions(disk) - tm = Yast::Storage.GetTargetMap - partitions = tm.fetch(disk, {}).fetch("partitions", []) + partitions = target_map.fetch(disk, {}).fetch("partitions", []) # do not select swap and do not select BIOS grub partition # as it clear its special flags (bnc#894040) partitions.select do |p| @@ -173,28 +185,20 @@ def extended_partition_num(disk) end # Given a device name to which we install the bootloader (loader_device), - # get the name of the partition which should be activated. - # Also return the device file name of the disk device that corresponds to - # loader_device (i.e. where the corresponding MBR can be found). + # gets back disk and partition number to activate. If empty Hash is returned + # then no suitable partition to activate found. # @param [String] loader_device string the device to install bootloader to - # @return a map $[ "mbr": string, "num": any] - # containing device (eg. "/dev/hda4"), disk (eg. "/dev/hda") and - # partition number (eg. 4) + # @return a Hash `{ "mbr" => String, "num" => Integer }` + # containing disk (eg. "/dev/hda") and partition number (eg. 4) def partition_to_activate(loader_device) - p_dev = Yast::Storage.GetDiskPartition(loader_device) + real_device = first_base_device_to_boot(loader_device) + log.info "real devices for #{loader_device} is #{real_device}" + + p_dev = Yast::Storage.GetDiskPartition(real_device) num = p_dev["nr"].to_i mbr_dev = p_dev["disk"] raise "Invalid loader device #{loader_device}" unless mbr_dev - # If loader_device is /dev/md* (which means bootloader is installed to - # /dev/md*), then call recursive method with partition that lays on device - # with the lowest bios id number or first one if noone have bios id - # FIXME: use ::storage to detect md devices, not by name! - if loader_device.start_with?("/dev/md") - base_device = first_base_device_to_boot(loader_device) - return partition_to_activate(base_device) if base_device - end - # (bnc # 337742) - Unable to boot the openSUSE (32 and 64 bits) after installation # if loader_device is disk Choose any partition which is not swap to # satisfy such bios (bnc#893449) @@ -234,5 +238,9 @@ def partitions_to_activate result.uniq end + + def target_map + @target_map ||= Yast::Storage.GetTargetMap + end end end diff --git a/src/modules/BootStorage.rb b/src/modules/BootStorage.rb index efd972349..aaca48192 100644 --- a/src/modules/BootStorage.rb +++ b/src/modules/BootStorage.rb @@ -325,7 +325,6 @@ def encrypted_boot? publish :variable => :BootPartitionDevice, :type => "string" publish :variable => :RootPartitionDevice, :type => "string" publish :variable => :ExtendedPartitionDevice, :type => "string" - publish :function => :Md2Partitions, :type => "map (string)" end BootStorage = BootStorageClass.new diff --git a/test/mbr_update_test.rb b/test/mbr_update_test.rb index 8b17f78dc..6b588069a 100644 --- a/test/mbr_update_test.rb +++ b/test/mbr_update_test.rb @@ -119,6 +119,20 @@ def stage1(devices: [], activate: false, generic_mbr: false) subject.run(stage1(generic_mbr: true)) end + it "always uses real devices" do + allow(Yast::BootStorage).to receive(:mbr_disk) + .and_return("/dev/md0") + + allow(::Bootloader::Stage1Device).to receive(:new).with("/dev/md0") + .and_return(double(real_devices: ["/dev/sda1", "/dev/sdb1"])) + expect(Yast::Execute).to receive(:on_target).at_least(:twice) do |*args| + next nil unless args.first =~ /dd/ + next nil unless args.include?("of=/dev/sdb") + expect(args).to be_include("of=/dev/sda") + end + subject.run(stage1(generic_mbr: true)) + end + it "install syslinux if non on initial stage" do allow(Yast::Stage).to receive(:initial).and_return(false) expect(Yast::PackageSystem).to receive(:Install).with("syslinux") @@ -188,6 +202,20 @@ def stage1(devices: [], activate: false, generic_mbr: false) subject.run(stage1(activate: true, devices: ["/dev/sda1", "/dev/sdb1"])) end + + it "sets boot flag on boot device with the lowest bios id when stage1 partition is on md" do + allow(Yast::Storage).to receive(:GetTargetMap).and_return( + "/dev/sda" => { "label" => "msdos", "bios_id" => "0x81" }, + "/dev/sdb" => { "label" => "msdos", "bios_id" => "0x80" } + ) + + allow(::Bootloader::Stage1Device).to receive(:new).with("/dev/md1") + .and_return(double(real_devices: ["/dev/sda1", "/dev/sdb1"])) + expect(Yast::Execute).to receive(:locally) + .with(/parted/, "-s", "/dev/sdb", "set", 1, "boot", "on") + + subject.run(stage1(activate: true, devices: ["/dev/md1"])) + end end context "disk label is GPT" do