Improved error message for broken by-path device names (bsc#1122008)

src/lib/bootloader/exceptions.rb

@@ -26,6 +26,27 @@ def initialize(msg)
     end
   end
 
+  # Specialized exception for invalid by-path device names
+  # (bsc#1122008, bsc#1116305)
+  class BrokenByPathDeviceName < RuntimeError
+    include Yast::I18n
+    attr_reader :dev_name
+
+    def initialize(dev_name)
+      @dev_name = dev_name
+      textdomain "bootloader"
+
+      # TRANSLATORS: %s is the device name
+      super _("Error reading the bootloader configuration files:\n" \
+        "Invalid device name %s\n" \
+        "\n" \
+        "This by-path device name may have changed after a reboot\n" \
+        "if the hardware or kernel parameters changed.\n" \
+        "\n" \
+        "Please use YaST2 bootloader to fix this.\n") % dev_name
+    end
+  end
+
   # Represent unsupported value in given option. Used mainly when value contain something that
   # bootloader does not understand yet.
   class UnsupportedOption < RuntimeError

src/modules/BootStorage.rb

@@ -72,8 +72,7 @@ def gpt_boot_disk?
     # @return [Array<String>] gpt disks only
     def gpt_disks(devices)
       targets = devices.map do |dev_name|
-        staging.find_by_any_name(dev_name) or
-          raise ::Bootloader::BrokenConfiguration, "Unknown device #{dev_name}"
+        staging.find_by_any_name(dev_name) or handle_unknown_device(dev_name)
       end
       boot_disks = targets.each_with_object([]) { |t, r| r.concat(stage1_disks_for(t)) }
 
@@ -140,7 +139,7 @@ def encrypted_boot?
     # @return [Array<Y2Storage::Device>] list of suitable devices
     def stage1_devices_for_name(dev_name)
       device = staging.find_by_any_name(dev_name)
-      raise ::Bootloader::BrokenConfiguration, "unknown device #{dev_name}" unless device
+      handle_unknown_device(dev_name) unless device
 
       if device.is?(:partition) || device.is?(:filesystem)
         stage1_partitions_for(device)
@@ -275,6 +274,25 @@ def select_ancestors(device, &predicate)
       end
       results
     end
+
+    # Handle an "unknown device" error: Raise an appropriate exception.
+    # @param dev_name [String]
+    def handle_unknown_device(dev_name)
+      # rubocop:disable Style/GuardClause
+      #
+      # I flatly refuse to make my code LESS readable because of a third-rate
+      # check tool. This is the CLASSIC use case for if...else, even if this
+      # mindless rubocop thinks otherwise.
+      #
+      # 2019-02-06 shundhammer
+
+      if dev_name =~ %r{/by-path/} # bsc#1122008, bsc#1116305
+        raise ::Bootloader::BrokenByPathDeviceName, dev_name
+      else
+        raise ::Bootloader::BrokenConfiguration, "Unknown device #{dev_name}"
+      end
+      # rubocop:enable Style/GuardClause
+    end
   end
 
   BootStorage = BootStorageClass.new

test/boot_storage_test.rb

@@ -54,14 +54,25 @@
   end
 
   describe ".stage1_devices_for_name" do
-    it "raises BrokenConfiguration exception if gets unknown name" do
-      # mock staging graph as graph does not return proper value when run as non-root
+    before do
       allow(subject.staging).to receive(:find_by_any_name).and_return(nil)
+    end
+
+    it "raises a BrokenConfiguration exception if gets an unknown name" do
+      # mock staging graph as graph does not return proper value when run as non-root
 
       expect { subject.stage1_devices_for_name("/dev/non-existing") }.to(
         raise_error(::Bootloader::BrokenConfiguration)
       )
     end
+
+    it "raises a BrokenByPathDeviceName exception if gets an unknown by-path device name" do
+      # mock staging graph as graph does not return proper value when run as non-root
+
+      expect { subject.stage1_devices_for_name("/dev/disk/by-path/non-existing") }.to(
+        raise_error(::Bootloader::BrokenByPathDeviceName)
+      )
+    end
   end
 
   describe ".boot_disks" do