New device and boot selection dialogs

doc/dbus/org.opensuse.Agama.Storage1.Proposal.doc.xml

@@ -11,6 +11,7 @@
         TargetPVDevices        as (optional: only makes sense if Target is "newLvmVg")
         ConfigureBoot          b
         BootDevice             s
+        DefaultBootDevice      s
         EncryptionPassword     s
         EncryptionMethod       s
         EncryptionPBKDFunction s

service/Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    agama (7)
+    agama-yast (7.devel737)
       cfa (~> 1.0.2)
       cfa_grub2 (~> 2.0.0)
       cheetah (~> 1.0.0)
@@ -26,7 +26,9 @@ GEM
     docile (1.4.0)
     eventmachine (1.2.7)
     fast_gettext (2.3.0)
-    nokogiri (1.15.5-x86_64-linux)
+    mini_portile2 (2.8.5)
+    nokogiri (1.15.5)
+      mini_portile2 (~> 2.8.2)
       racc (~> 1.4)
     packaging_rake_tasks (1.5.4)
       rake
@@ -63,7 +65,7 @@ PLATFORMS
   x86_64-linux-gnu
 
 DEPENDENCIES
-  agama!
+  agama-yast!
   byebug
   packaging_rake_tasks (~> 1.5.1)
   rake (~> 13.0.6)

service/lib/agama/dbus/storage/interfaces/device/drive.rb

@@ -85,7 +85,10 @@ def drive_model
             #
             # @return [String]
             def drive_bus
-              storage_device.bus || ""
+              # FIXME: not sure whether checking for "none" is robust enough
+              return "" if storage_device.bus.nil? || storage_device.bus.casecmp?("none")
+
+              storage_device.bus
             end
 
             # Bus Id for DASD
@@ -110,7 +113,15 @@ def drive_driver
             def drive_transport
               return "" unless storage_device.respond_to?(:transport)
 
-              storage_device.transport.to_s
+              transport = storage_device.transport
+              return "" if transport.nil? || transport.is?(:unknown)
+
+              # FIXME: transport does not have proper i18n support at yast2-storage-ng, so we are
+              # just duplicating some logic from yast2-storage-ng here
+              return "USB" if transport.is?(:usb)
+              return "IEEE 1394" if transport.is?(:sbp)
+
+              transport.to_s
             end
 
             # More info about the device

service/lib/agama/dbus/storage/proposal_settings_conversion/to_dbus.rb

@@ -41,6 +41,7 @@ def initialize(settings)
           #   * "CandidatePVDevices" [Array<String>] Optional
           #   * "ConfigureBoot" [Boolean]
           #   * "BootDevice" [String]
+          #   * "DefaultBootDevice" [String]
           #   * "EncryptionPassword" [String]
           #   * "EncryptionMethod" [String]
           #   * "EncryptionPBKDFunction" [String]
@@ -65,6 +66,7 @@ def convert
           DBUS_PROPERTIES = {
             "ConfigureBoot"          => :configure_boot_conversion,
             "BootDevice"             => :boot_device_conversion,
+            "DefaultBootDevice"      => :default_boot_device_conversion,
             "EncryptionPassword"     => :encryption_password_conversion,
             "EncryptionMethod"       => :encryption_method_conversion,
             "EncryptionPBKDFunction" => :encryption_pbkd_function_conversion,
@@ -126,6 +128,11 @@ def boot_device_conversion
             settings.boot.device || ""
           end
 
+          # @return [String]
+          def default_boot_device_conversion
+            settings.default_boot_device || ""
+          end
+
           # @return [String]
           def encryption_password_conversion
             settings.encryption.password.to_s

service/lib/agama/storage/proposal_settings.rb

@@ -71,6 +71,20 @@ def installation_devices
           .uniq
       end
 
+      # Default device to use for configuring boot.
+      #
+      # @return [String, nil]
+      def default_boot_device
+        case device
+        when DeviceSettings::Disk
+          device.name
+        when DeviceSettings::NewLvmVg
+          device.candidate_pv_devices.min
+        when DeviceSettings::ReusedLvmVg
+          # TODO: Decide what device to use.
+        end
+      end
+
     private
 
       # Device used for booting.

service/lib/agama/storage/proposal_settings_conversion/to_y2storage.rb

@@ -77,7 +77,7 @@ def device_conversion(target)
         # @param target [Y2Storage::ProposalSettings]
         def disk_device_conversion(target)
           target.lvm = false
-          target.candidate_devices = [settings.boot.device].compact
+          target.candidate_devices = [boot_device].compact
         end
 
         # @param target [Y2Storage::ProposalSettings]
@@ -109,8 +109,8 @@ def enable_lvm(target)
 
         # @param target [Y2Storage::ProposalSettings]
         def boot_conversion(target)
-          target.root_device = settings.boot.device
           target.boot = settings.boot.configure?
+          target.root_device = boot_device
         end
 
         # @param target [Y2Storage::ProposalSettings]
@@ -196,6 +196,13 @@ def find_max_size_fallback(mount_path)
           volume&.mount_path
         end
 
+        # Device used for booting.
+        #
+        # @return [String, nil]
+        def boot_device
+          settings.boot.device || settings.default_boot_device
+        end
+
         # All block devices affected by the space policy.
         #
         # @see ProposalSettings#installation_devices

service/test/agama/dbus/storage/interfaces/device/drive_examples.rb

@@ -192,13 +192,29 @@
         allow(device).to receive(:transport).and_return(transport)
       end
 
-      let(:transport) { instance_double(Y2Storage::DataTransport, to_s: "usb") }
+      let(:transport) { Y2Storage::DataTransport::FCOE }
 
       it "returns the transport" do
-        expect(subject.drive_transport).to eq("usb")
+        expect(subject.drive_transport).to eq("fcoe")
       end
 
-      context "if transport is unknown" do
+      context "if transport is Unknown" do
+        let(:transport) { Y2Storage::DataTransport::UNKNOWN }
+
+        it "returns an empty string" do
+          expect(subject.drive_transport).to eq("")
+        end
+      end
+
+      context "if transport is USB" do
+        let(:transport) { Y2Storage::DataTransport::USB }
+
+        it "returns the corresponding string" do
+          expect(subject.drive_transport).to eq("USB")
+        end
+      end
+
+      context "if transport is missing" do
         let(:transport) { nil }
 
         it "returns an empty string" do

service/test/agama/dbus/storage/proposal_settings_conversion/to_dbus_test.rb

@@ -50,6 +50,7 @@
         "TargetDevice"           => "",
         "ConfigureBoot"          => true,
         "BootDevice"             => "",
+        "DefaultBootDevice"      => "",
         "EncryptionPassword"     => "",
         "EncryptionMethod"       => "luks2",
         "EncryptionPBKDFunction" => "pbkdf2",
@@ -63,6 +64,7 @@
         "TargetDevice"           => "/dev/sda",
         "ConfigureBoot"          => true,
         "BootDevice"             => "/dev/sdb",
+        "DefaultBootDevice"      => "/dev/sda",
         "EncryptionPassword"     => "notsecret",
         "EncryptionMethod"       => "luks2",
         "EncryptionPBKDFunction" => "argon2id",

service/test/agama/storage/proposal_settings_conversion/to_y2storage_test.rb

@@ -99,6 +99,7 @@
       context "when the device settings is set to use a disk" do
         before do
           settings.device = Agama::Storage::DeviceSettings::Disk.new("/dev/sda")
+          settings.boot.device = "/dev/sdb"
         end
 
         it "sets lvm to false" do
@@ -107,12 +108,6 @@
           expect(y2storage_settings.lvm).to eq(false)
         end
 
-        it "sets the boot device as candidate device" do
-          y2storage_settings = subject.convert
-
-          expect(y2storage_settings.candidate_devices).to contain_exactly("/dev/sda")
-        end
-
         it "sets the target device as device for the volumes with missing device" do
           y2storage_settings = subject.convert
 
@@ -122,6 +117,24 @@
             an_object_having_attributes(mount_point: "/test3", device: "/dev/sdb")
           )
         end
+
+        it "sets the boot device as candidate device" do
+          y2storage_settings = subject.convert
+
+          expect(y2storage_settings.candidate_devices).to contain_exactly("/dev/sdb")
+        end
+
+        context "and no boot device is selected" do
+          before do
+            settings.boot.device = nil
+          end
+
+          it "sets the target device as candidate device" do
+            y2storage_settings = subject.convert
+
+            expect(y2storage_settings.candidate_devices).to contain_exactly("/dev/sda")
+          end
+        end
       end
 
       context "when the device settings is set to create a new LVM volume group" do
@@ -172,16 +185,6 @@
     end
 
     context "boot conversion" do
-      before do
-        settings.boot.device = "/dev/sda"
-      end
-
-      it "sets the boot device as root device" do
-        y2storage_settings = subject.convert
-
-        expect(y2storage_settings.root_device).to eq("/dev/sda")
-      end
-
       context "if boot configuration is enabled" do
         before do
           settings.boot.configure = true
@@ -205,6 +208,48 @@
           expect(y2storage_settings.boot).to eq(false)
         end
       end
+
+      context "if a boot device is selected" do
+        before do
+          settings.boot.device = "/dev/sda"
+        end
+
+        it "sets the boot device as root device" do
+          y2storage_settings = subject.convert
+
+          expect(y2storage_settings.root_device).to eq("/dev/sda")
+        end
+      end
+
+      context "if no boot device is selected" do
+        before do
+          settings.boot.device = nil
+        end
+
+        context "and the device settings is set to use a disk" do
+          before do
+            settings.device = Agama::Storage::DeviceSettings::Disk.new("/dev/sda")
+          end
+
+          it "sets the target device as root device" do
+            y2storage_settings = subject.convert
+
+            expect(y2storage_settings.root_device).to eq("/dev/sda")
+          end
+        end
+
+        context "and the device settings is set to create a new LVM volume group" do
+          before do
+            settings.device = Agama::Storage::DeviceSettings::NewLvmVg.new(["/dev/sda", "/dev/sdb"])
+          end
+
+          it "sets the first candidate device as root device" do
+            y2storage_settings = subject.convert
+
+            expect(y2storage_settings.root_device).to eq("/dev/sda")
+          end
+        end
+      end
     end
 
     context "space policy conversion" do

service/test/agama/storage/proposal_settings_test.rb

@@ -25,6 +25,60 @@
 require "agama/storage/volume"
 
 describe Agama::Storage::ProposalSettings do
+  describe "#default_boot_device" do
+    context "when the device is configured to use a disk" do
+      before do
+        subject.device = Agama::Storage::DeviceSettings::Disk.new
+      end
+
+      context "and no device is selected yet" do
+        before do
+          subject.device.name = nil
+        end
+
+        it "returns nil" do
+          expect(subject.default_boot_device).to be_nil
+        end
+      end
+
+      context "and a device is selected" do
+        before do
+          subject.device.name = "/dev/sda"
+        end
+
+        it "returns the target device" do
+          expect(subject.default_boot_device).to eq("/dev/sda")
+        end
+      end
+    end
+
+    context "when the device is configured to create a new LVM volume group" do
+      before do
+        subject.device = Agama::Storage::DeviceSettings::NewLvmVg.new
+      end
+
+      context "and no device is selected yet" do
+        before do
+          subject.device.candidate_pv_devices = []
+        end
+
+        it "returns nil" do
+          expect(subject.default_boot_device).to be_nil
+        end
+      end
+
+      context "and some candidate devices for creating the LVM physical volumes are selected" do
+        before do
+          subject.device.candidate_pv_devices = ["/dev/sdc", "/dev/sda", "/dev/sdb"]
+        end
+
+        it "returns the first candidate device in alphabetical order" do
+          expect(subject.default_boot_device).to eq("/dev/sda")
+        end
+      end
+    end
+  end
+
   describe "#installation_devices" do
     shared_examples "boot device" do
       context "when boot is set to be configured" do