WIP: RFC

service/lib/agama/dbus/storage/volume_conversion/from_dbus.rb

@@ -20,6 +20,7 @@
 # find current contact information at www.suse.com.
 
 require "agama/storage/volume"
+require "agama/storage/volume_location"
 require "agama/storage/volume_templates_builder"
 require "y2storage/disk_size"
 require "y2storage/filesystems/type"
@@ -67,8 +68,8 @@ def convert
           CONVERSIONS = {
             "MountPath"    => :mount_path_conversion,
             "MountOptions" => :mount_options_conversion,
+            "Target"       => :target_conversion,
             "TargetDevice" => :target_device_conversion,
-            "TargetVG"     => :target_vg_conversion,
             "FsType"       => :fs_type_conversion,
             "MinSize"      => :min_size_conversion,
             "MaxSize"      => :max_size_conversion,
@@ -92,13 +93,16 @@ def mount_options_conversion(target, value)
           # @param target [Agama::Storage::Volume]
           # @param value [String]
           def target_device_conversion(target, value)
-            target.device = value
+            target.location.device = value
           end
 
           # @param target [Agama::Storage::Volume]
           # @param value [String]
-          def target_vg_conversion(target, value)
-            target.separate_vg_name = value
+          def target_conversion(target, value)
+            target_value = value.downcase.to_sym
+            return unless Agama::Storage::VolumeLocation.targets.include?(target_value)
+
+            target.location.target = target_value
           end
 
           # @param target [Agama::Storage::Volume]

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

@@ -37,8 +37,10 @@ def convert
             {
               "MountPath"     => volume.mount_path.to_s,
               "MountOptions"  => volume.mount_options,
-              "TargetDevice"  => volume.device.to_s,
-              "TargetVG"      => volume.separate_vg_name.to_s,
+              # Values are :default, :new_partition, :new_vg, :device and :filesystem
+              # Show we convert them to camel case strings like "NewPartition"?
+              "Target"        => volume.location.target.to_s,
+              "TargetDevice"  => volume.location.device.to_s,
               "FsType"        => volume.fs_type&.to_human_string || "",
               "MinSize"       => volume.min_size&.to_i,
               "AutoSize"      => volume.auto_size?,

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

@@ -83,7 +83,8 @@ def lvm_conversion(target)
           lvm = settings.lvm.enabled?
 
           target.lvm = lvm
-          target.separate_vgs = lvm
+          # Activate support for dedicated volume groups
+          target.separate_vgs = true
           # Prevent VG reuse
           target.lvm_vg_reuse = false
         end
@@ -170,7 +171,7 @@ def find_max_size_fallback(mount_path)
         def all_devices
           devices = [settings.boot_device] +
             settings.lvm.system_vg_devices +
-            settings.volumes.map(&:device)
+            settings.volumes.map(&:location).reject(&:reuse_device?).map(&:device)
 
           devices.compact.uniq.map { |d| device_or_partitions(d) }.flatten
         end

service/lib/agama/storage/volume.rb

@@ -23,6 +23,7 @@
 require "y2storage/disk_size"
 require "agama/storage/btrfs_settings"
 require "agama/storage/volume_outline"
+require "agama/storage/volume_location"
 
 module Agama
   module Storage
@@ -58,15 +59,10 @@ class Volume
       # @return [Array<String>]
       attr_accessor :mount_options
 
-      # Used to locate the volume in a separate disk
+      # Location of the volume
       #
-      # @return [String, nil]
-      attr_accessor :device
-
-      # Used to locate the volume in a separate VG
-      #
-      # @return [String, nil]
-      attr_accessor :separate_vg_name
+      # @return [VolumeLocation]
+      attr_accessor :location
 
       # Min size for the volume
       #
@@ -98,6 +94,7 @@ def initialize(mount_path)
         @max_size = Y2Storage::DiskSize.unlimited
         @btrfs = BtrfsSettings.new
         @outline = VolumeOutline.new
+        @location = VolumeLocation.new
       end
 
       def_delegators :outline,

service/lib/agama/storage/volume_conversion/from_y2storage.rb

@@ -41,13 +41,12 @@ def convert
           volume = VolumeTemplatesBuilder.new_from_config(config).for(spec.mount_point || "")
 
           volume.tap do |target|
-            target.device = spec.device
-            target.separate_vg_name = spec.separate_vg_name
             target.mount_options = spec.mount_options
             target.fs_type = spec.fs_type
 
             sizes_conversion(target)
             btrfs_conversion(target)
+            location_conversion(target)
           end
         end
 
@@ -83,6 +82,17 @@ def btrfs_conversion(target)
           target.btrfs.read_only = spec.btrfs_read_only
         end
 
+        # @param target [Agama::Storage::Volume]
+        def location_conversion(target)
+          if spec.reuse?
+            target.location.target = spec.reformat? ? :device : :filesystem
+            target.location.device = spec.reuse_name
+          elsif !!spec.device
+            target.location.target = spec.separate_vg? ? :new_vg : :new_partition
+            target.location.device = spec.device
+          end
+        end
+
         # Planned device for the given mount path.
 
         # @param mount_path [String]

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

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-# Copyright (c) [2023] SUSE LLC
+# Copyright (c) [2023-2024] SUSE LLC
 #
 # All Rights Reserved.
 #
@@ -37,8 +37,6 @@ def initialize(volume)
         # @return [Y2Storage::VolumeSpecification]
         def convert # rubocop:disable Metrics/AbcSize
           Y2Storage::VolumeSpecification.new({}).tap do |target|
-            target.device = volume.device
-            target.separate_vg_name = volume.separate_vg_name
             target.mount_point = volume.mount_path
             target.mount_options = volume.mount_options.join(",")
             target.proposed = true
@@ -50,6 +48,7 @@ def convert # rubocop:disable Metrics/AbcSize
 
             sizes_conversion(target)
             btrfs_conversion(target)
+            location_conversion(target)
           end
         end
 
@@ -88,6 +87,29 @@ def btrfs_conversion(target)
           target.btrfs_default_subvolume = volume.btrfs.default_subvolume
           target.btrfs_read_only = volume.btrfs.read_only?
         end
+
+        # @param target [Y2Storage::VolumeSpecification]
+        def location_conversion(target)
+          location = volume.location
+          return if location.default?
+
+          if [:new_partition, :new_vg].include?(location.target)
+            target.device = location.device
+            target.separate_vg_name = vg_name(target) if location.target == :new_vg
+            return
+          end
+
+          target.reuse_name = location.device
+          target.reformat = location.target == :device
+        end
+
+        # Name to be used as separate_vg_name for the given Y2Storage volume
+        #
+        # @param target [Y2Storage::VolumeSpecification]
+        def vg_name(target)
+          mount_point = target.root? ? "root" : target.mount_point
+          "vg-#{mount_point.tr("/", "_")}"
+        end
       end
     end
   end

service/lib/agama/storage/volume_location.rb

@@ -0,0 +1,58 @@
+# frozen_string_literal: true
+
+# Copyright (c) [2024] SUSE LLC
+#
+# All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or modify it
+# under the terms of version 2 of the GNU General Public License as published
+# by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+# FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+# more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program; if not, contact SUSE LLC.
+#
+# To contact SUSE LLC about this file by physical or electronic mail, you may
+# find current contact information at www.suse.com.
+
+module Agama
+  module Storage
+    # Settings specifying what device should be used for a given Volume and how
+    class VolumeLocation
+      # @see .targets
+      TARGETS = [:default, :new_partition, :new_vg, :device, :filesystem].freeze
+      private_constant :TARGETS
+
+      # @return [Symbol] see .targets
+      attr_accessor :target
+
+      # @return [String, nil]
+      attr_accessor :device
+
+      # All possible values for #target
+      #
+      # @return [Array<Symbol>]
+      def self.targets
+        TARGETS
+      end
+
+      # Constructor
+      def initialize
+        @target = :default
+      end
+
+      # return [Boolean]
+      def default?
+        target == :default
+      end
+
+      def reuse_device?
+        [:device, :filesystem].include?(target)
+      end
+    end
+  end
+end

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

@@ -44,7 +44,10 @@
         settings.encryption.pbkd_function = Y2Storage::PbkdFunction::ARGON2ID
         settings.space.policy = :custom
         settings.space.actions = { "/dev/sda" => :force_delete }
-        volume = Agama::Storage::Volume.new("/test").tap { |v| v.device = "/dev/sdc" }
+        volume = Agama::Storage::Volume.new("/test").tap do |vol|
+          vol.location.target = :new_partition
+          vol.location.device = "/dev/sdc"
+        end
         settings.volumes = [volume]
       end
     end

service/test/agama/storage/volume_conversion/from_y2storage_test.rb

@@ -57,14 +57,16 @@
 
       expect(volume).to be_a(Agama::Storage::Volume)
       expect(volume).to have_attributes(
-        mount_path:       "/",
-        device:           "/dev/sda",
-        separate_vg_name: "/dev/vg0",
-        mount_options:    contain_exactly("defaults", "ro"),
-        fs_type:          Y2Storage::Filesystems::Type::BTRFS,
-        min_size:         Y2Storage::DiskSize.GiB(5),
-        max_size:         Y2Storage::DiskSize.GiB(20),
-        btrfs:            an_object_having_attributes(
+        mount_path:    "/",
+        location:      an_object_having_attributes(
+          device: "/dev/sda",
+          target: :new_vg
+        ),
+        mount_options: contain_exactly("defaults", "ro"),
+        fs_type:       Y2Storage::Filesystems::Type::BTRFS,
+        min_size:      Y2Storage::DiskSize.GiB(5),
+        max_size:      Y2Storage::DiskSize.GiB(20),
+        btrfs:         an_object_having_attributes(
           snapshots?:        true,
           subvolumes:        contain_exactly("@/home", "@/var"),
           default_subvolume: "@",

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

@@ -29,8 +29,8 @@
   describe "#convert" do
     let(:volume) do
       Agama::Storage::Volume.new("/").tap do |volume|
-        volume.device = "/dev/sda"
-        volume.separate_vg_name = "/dev/vg0"
+        volume.location.device = "/dev/sda"
+        volume.location.target = :new_vg
         volume.mount_options = ["defaults"]
         volume.fs_type = btrfs
         volume.auto_size = false
@@ -59,7 +59,7 @@
       expect(spec).to be_a(Y2Storage::VolumeSpecification)
       expect(spec).to have_attributes(
         device:                  "/dev/sda",
-        separate_vg_name:        "/dev/vg0",
+        separate_vg_name:        "vg-root",
         mount_point:             "/",
         mount_options:           "defaults",
         proposed?:               true,