diff --git a/package/yast2-bootloader.changes b/package/yast2-bootloader.changes index c41065ef3..52fc40df8 100644 --- a/package/yast2-bootloader.changes +++ b/package/yast2-bootloader.changes @@ -1,3 +1,10 @@ +------------------------------------------------------------------- +Wed Mar 25 07:57:13 UTC 2020 - Ancor Gonzalez Sosa + +- Fixed the calculation of the udev name used to reference devices + that are not formatted, like PReP partitions (bsc#1166096). +- 4.2.19 + ------------------------------------------------------------------- Tue Mar 24 08:10:33 UTC 2020 - Steffen Winterfeldt diff --git a/package/yast2-bootloader.spec b/package/yast2-bootloader.spec index f13257db2..f374f4133 100644 --- a/package/yast2-bootloader.spec +++ b/package/yast2-bootloader.spec @@ -17,7 +17,7 @@ Name: yast2-bootloader -Version: 4.2.18 +Version: 4.2.19 Release: 0 Summary: YaST2 - Bootloader Configuration License: GPL-2.0-or-later @@ -29,8 +29,8 @@ Source0: %{name}-%{version}.tar.bz2 BuildRequires: yast2 >= 3.1.176 BuildRequires: yast2-devtools >= 4.2.2 BuildRequires: yast2-ruby-bindings >= 1.0.0 -# Y2Storage::Mountable#preferred_mount_by -BuildRequires: yast2-storage-ng >= 4.2.90 +# Y2Storage::BlkDevice#preferred_name +BuildRequires: yast2-storage-ng >= 4.2.102 # lenses needed also for tests BuildRequires: augeas-lenses BuildRequires: rubygem(%rb_default_ruby_abi:cfa_grub2) >= 1.0.1 @@ -46,8 +46,8 @@ Requires: yast2 >= 3.1.176 Requires: yast2-core >= 2.18.7 Requires: yast2-packager >= 2.17.24 Requires: yast2-pkg-bindings >= 2.17.25 -# Y2Storage::Mountable#preferred_mount_by -Requires: yast2-storage-ng >= 4.2.90 +# Y2Storage::BlkDevice#preferred_name +Requires: yast2-storage-ng >= 4.2.102 # Support for multiple values in GRUB_TERMINAL Requires: rubygem(%rb_default_ruby_abi:cfa_grub2) >= 1.0.1 # lenses are needed as cfa_grub2 depends only on augeas bindings, but also diff --git a/src/lib/bootloader/udev_mapping.rb b/src/lib/bootloader/udev_mapping.rb index 079489cc0..3c7c5aa80 100644 --- a/src/lib/bootloader/udev_mapping.rb +++ b/src/lib/bootloader/udev_mapping.rb @@ -91,22 +91,37 @@ def kernel_to_udev(dev) return dev end - udev = mount_by_udev(device) || device.name + udev = udev_path(device) log.info "udev device for #{dev.inspect} is #{udev.inspect}" udev end - # @return [String, nil] nil if the udev name cannot be found - def mount_by_udev(device) + # Most convenient udev name of the device + # + # If possible, the udev path is chosen based on the mount_by attribute of + # the filesystem. If the device is not mounted or there is no path for + # the specified mount_by, a preferred (and available) name is calculated. + # + # @param device [Y2Storage::BlkDevice] + # @return [String] + def udev_path(device) filesystem = device.filesystem - return nil unless filesystem - # If the device is not mounted, a preferred mount by option is calculated. - mount_by = filesystem.mount_by || filesystem.preferred_mount_by - return nil unless mount_by - - device.path_for_mount_by(mount_by) + if filesystem + mount_by_name = filesystem.mount_by_name + + if mount_by_name + log.info "udev_path: using the udev name of the configured mount_by" + mount_by_name + else + log.info "udev_path: using the preferred udev name for the filesystem" + filesystem.preferred_name + end + else + log.info "udev_path: not formatted, using preferred udev name for the block device" + device.preferred_name + end end end end diff --git a/test/data/bug_1166096.xml b/test/data/bug_1166096.xml new file mode 100644 index 000000000..fd7e48dc3 --- /dev/null +++ b/test/data/bug_1166096.xml @@ -0,0 +1,342 @@ + + + + + + 43 + /dev/vda + vda + /devices/pci0000:00/0000:00:06.0/virtio3/block/vda + + 41943040 + 512 + + + pci-0000:00:06.0 + 256 + true + + + 51 + + + 52 + /dev/vda1 + vda1 + /devices/pci0000:00/0000:00:06.0/virtio3/block/vda/vda1 + + 2048 + 16384 + 512 + + + pci-0000:00:06.0-part1 + primary + 65 + + + 53 + /dev/vda2 + vda2 + /devices/pci0000:00/0000:00:06.0/virtio3/block/vda/vda2 + + 18432 + 39047168 + 512 + + + pci-0000:00:06.0-part2 + primary + 131 + + + 54 + DEFAULT + DEFAULT + + + 55 + 5 + + + + 56 + / + uuid + btrfs + true + true + 0 + 0 + + + 57 + -1 + @ + true + + + 58 + -1 + @/boot/grub2/powerpc-ieee1275 + + + 59 + /boot/grub2/powerpc-ieee1275 + uuid + subvol=/@/boot/grub2/powerpc-ieee1275 + btrfs + true + true + 0 + 0 + + + 60 + -1 + @/home + + + 61 + /home + uuid + subvol=/@/home + btrfs + true + true + 0 + 0 + + + 62 + -1 + @/opt + + + 63 + /opt + uuid + subvol=/@/opt + btrfs + true + true + 0 + 0 + + + 64 + -1 + @/root + + + 65 + /root + uuid + subvol=/@/root + btrfs + true + true + 0 + 0 + + + 66 + -1 + @/srv + + + 67 + /srv + uuid + subvol=/@/srv + btrfs + true + true + 0 + 0 + + + 68 + -1 + @/tmp + + + 69 + /tmp + uuid + subvol=/@/tmp + btrfs + true + true + 0 + 0 + + + 70 + -1 + @/usr/local + + + 71 + /usr/local + uuid + subvol=/@/usr/local + btrfs + true + true + 0 + 0 + + + 72 + -1 + @/var + true + + + 73 + /var + uuid + subvol=/@/var + btrfs + true + true + 0 + 0 + + + 74 + /dev/vda3 + vda3 + /devices/pci0000:00/0000:00:06.0/virtio3/block/vda/vda3 + + 39065600 + 2877407 + 512 + + + pci-0000:00:06.0-part3 + primary + 130 + + + 75 + + + 76 + swap + uuid + swap + true + true + 0 + 0 + + + + + 43 + 51 + + + 51 + 52 + + + 51 + 53 + + + 54 + 55 + + + 53 + 54 + + + 54 + 56 + + + 55 + 57 + + + 57 + 58 + + + 58 + 59 + + + 57 + 60 + + + 60 + 61 + + + 57 + 62 + + + 62 + 63 + + + 57 + 64 + + + 64 + 65 + + + 57 + 66 + + + 66 + 67 + + + 57 + 68 + + + 68 + 69 + + + 57 + 70 + + + 70 + 71 + + + 57 + 72 + + + 72 + 73 + + + 51 + 74 + + + 74 + 75 + + + 75 + 76 + + + diff --git a/test/udev_mapping_test.rb b/test/udev_mapping_test.rb index 09023ec26..579edc2b2 100755 --- a/test/udev_mapping_test.rb +++ b/test/udev_mapping_test.rb @@ -45,62 +45,78 @@ before do # find by name creates always new instance, so to make mocking easier, mock it to return always same instance allow(Y2Storage::BlkDevice).to receive(:find_by_name).and_return(device) - - allow(device).to receive(:path_for_mount_by).with(mount_by).and_return(udev_name) end let(:device) { find_device("/dev/sda3") } - let(:mount_by) { Y2Storage::Filesystems::MountByType.new(mount_by_option) } - - context "when the device is mounted" do + context "when the device is formatted" do before do - device.filesystem.mount_point.mount_by = mount_by + # The libstorage-ng bindings constantly create new instances of the storage objects. + # To make mocking easier, mock it to return always the same instances + allow(device).to receive(:filesystem).and_return(device.filesystem) end - let(:mount_by_option) { :label } + context "and mounted (or marked to be mounted)" do + before do + allow(device.filesystem).to receive(:mount_by_name).and_return mount_by_name + end + + context "and the udev name is available for the mount_by option in the mount point" do + let(:mount_by_name) { "/dev/something" } - context "and the udev name is available for the mount by option in the mount point" do - let(:udev_name) { "/dev/disk/by-label/test" } + it "returns the udev name according to the mount_by option in the mount point" do + expect(subject.to_mountby_device(device.name)).to eq mount_by_name + end + end + + context "and the udev name is not available for the mount by option in the mount point" do + let(:mount_by_name) { nil } - it "returns the udev name according to the mount by option in the mount point" do - expect(subject.to_mountby_device(device.name)).to eq(udev_name) + it "returns as fallback the name preferred by storage-ng" do + expect(device.filesystem).to receive(:preferred_name).and_return "/dev/preferred/fs" + + expect(subject.to_mountby_device(device.name)).to eq "/dev/preferred/fs" + end end end - context "and the udev name is not available for the mount by option in the mount point" do - let(:udev_name) { nil } + context "but not mounted" do + before { device.filesystem&.remove_mount_point } + + it "returns the preferred udev name for the filesystem" do + expect(device.filesystem).to receive(:preferred_name).and_return "/dev/preferred/fs" - it "returns the kernel name as fallback" do - expect(subject.to_mountby_device(device.name)).to eq("/dev/sda3") + expect(subject.to_mountby_device(device.name)).to eq "/dev/preferred/fs" end end end - context "when the device is not mounted" do - before do - device.filesystem&.remove_mount_point + context "when the device is not formatted" do + before { device.remove_descendants } - allow_any_instance_of(Y2Storage::MountPoint).to receive(:preferred_mount_by) - .and_return(mount_by) - end + it "returns the preferred udev name for the block device" do + expect(device).to receive(:preferred_name).and_return "/dev/preferred/blk_dev" - let(:mount_by_option) { :label } + expect(subject.to_mountby_device(device.name)).to eq "/dev/preferred/blk_dev" + end + end - context "and the udev name is available for the preferred mount by option" do - let(:udev_name) { "/dev/disk/by-label/test" } + # Regression test for bsc#1166096 + context "for a regular PReP partition (contains no filesystem and is not mounted)" do + before do + # First, disable the general mocking + allow(Y2Storage::BlkDevice).to receive(:find_by_name).and_call_original - it "returns the udev name according to the preferred mount by option" do - expect(subject.to_mountby_device(device.name)).to eq(udev_name) - end + # Then, just stub the whole devicegraph to reproduce the scenario + devicegraph_stub("bug_1166096.xml") end - context "and the udev name is not available for the preferred mount by option" do - let(:udev_name) { nil } + # These mocks are not needed + let(:device) { nil } + let(:mount_by) { nil } - it "returns the kernel name as fallback" do - expect(subject.to_mountby_device(device.name)).to eq("/dev/sda3") - end + it "returns an udev link if there is any available" do + expect(subject.to_mountby_device("/dev/vda1")).to eq "/dev/disk/by-path/pci-0000:00:06.0-part1" end end end