WIP: SpaceMaker: support calculating PVs for several VGs

src/lib/y2storage/planned/lvm_vg.rb

@@ -83,6 +83,11 @@ class LvmVg < Device
       # @return [Symbol]
       attr_accessor :size_strategy
 
+      # Disks where the proposal can create extra physical volumes to honor {#size_strategy}
+      #
+      # @return [Array<String>] names of partitionable devices
+      attr_accessor :pvs_candidate_devices
+
       # Builds a new instance based on a real VG
       #
       # The new instance represents the intention to reuse the real VG, so the
@@ -111,6 +116,7 @@ def initialize(volume_group_name: nil, lvs: [], pvs: [])
         @pvs = pvs
         @pvs_encryption_password = nil
         @make_space_policy = :needed
+        @pvs_candidate_devices = []
       end
 
       # Initializes the object taking the values from a real volume group
@@ -258,13 +264,15 @@ def self.to_string_attrs
       end
 
       # Device name of the disk-like device in which the volume group has to be
-      # physically located. If nil, the volume group can spread freely over any
-      # set of disks.
+      # physically located. If nil, the volume group can spread over a set of
+      # several disks (maybe even unlimited).
       #
       # @return [String, nil]
       def forced_disk_name
         forced_lv = lvs.find(&:disk)
-        forced_lv ? forced_lv.disk : nil
+        return forced_lv.disk if forced_lv
+
+        pvs_candidate_devices.size == 1 ? pvs_candidate_devices.first : nil
       end
 
       protected

src/lib/y2storage/proposal/devicegraph_generator.rb

@@ -195,7 +195,7 @@ def provide_space_lvm(planned_partitions, devicegraph, lvm_helper)
           space_maker.protected_sids += lvm_sids
 
           result = space_maker.provide_space(
-            devicegraph, planned_partitions, lvm_helper.volume_group
+            devicegraph, planned_partitions, [lvm_helper.volume_group]
           )
           log.info "Found enough space including LVM, reusing #{vg}"
           return result
@@ -206,7 +206,9 @@ def provide_space_lvm(planned_partitions, devicegraph, lvm_helper)
         end
 
         lvm_helper.reused_volume_group = nil
-        result = space_maker.provide_space(devicegraph, planned_partitions, lvm_helper.volume_group)
+        result = space_maker.provide_space(
+          devicegraph, planned_partitions, [lvm_helper.volume_group]
+        )
         log.info "Found enough space including LVM"
 
         result

src/lib/y2storage/proposal/lvm_helper.rb

@@ -106,6 +106,9 @@ def reused_volume_group=(vg)
         @reused_volume_group = Y2Storage::Planned::LvmVg.from_real_vg(vg)
         @reused_volume_group.lvs = planned_lvs
         @reused_volume_group.size_strategy = vg_strategy
+        if settings.candidate_devices
+          @reused_volume_group.pvs_candidate_devices = settings.candidate_devices
+        end
         @reused_volume_group.pvs_encryption_password = settings.encryption_password
         @reused_volume_group.pvs_encryption_method = settings.encryption_method
         @reused_volume_group.pvs_encryption_pbkdf = settings.encryption_pbkdf
@@ -143,6 +146,7 @@ def new_volume_group
         vg.pvs_encryption_password = settings.encryption_password
         vg.pvs_encryption_method = settings.encryption_method
         vg.pvs_encryption_pbkdf = settings.encryption_pbkdf
+        vg.pvs_candidate_devices = settings.candidate_devices if settings.candidate_devices
         vg.size_strategy = vg_strategy
         vg
       end

src/lib/y2storage/proposal/partitions_distribution_calculator.rb

@@ -30,8 +30,11 @@ module Proposal
     class PartitionsDistributionCalculator
       include Yast::Logger
 
-      def initialize(planned_vg = nil)
-        @planned_vg = planned_vg
+      # Constructor
+      #
+      # @param planned_vgs [Array<Planned::LvmVg>] see {#planned_vgs}
+      def initialize(planned_vgs = [])
+        @planned_vgs = planned_vgs
       end
 
       # Best possible distribution, nil if the planned partitions don't fit
@@ -43,7 +46,7 @@ def initialize(planned_vg = nil)
       #
       # @param partitions [Array<Planned::Partition>]
       # @param spaces [Array<FreeDiskSpace>] spaces that can be used to allocate partitions targeted
-      #   to the corresponding disk but also partitions with no specific disk and partitions for LVM
+      #   to the corresponding disk but also partitions with no specific disk
       # @param extra_spaces [Array<FreeDiskSpace>] spaces that can only be used to allocate
       #   partitions explicitly targeted to the corresponding disk
       #
@@ -62,8 +65,7 @@ def best_distribution(partitions, spaces, extra_spaces = [])
 
         if lvm?
           log.info "Calculate LVM posibilities for the #{candidates.size} candidate distributions"
-          pv_calculator = PhysVolCalculator.new(spaces, planned_vg)
-          candidates.map! { |dist| pv_calculator.add_physical_volumes(dist) }
+          add_physical_volumes(candidates, spaces + extra_spaces)
         end
         candidates.compact!
 
@@ -123,18 +125,19 @@ def resizing_size(partition, planned_partitions, free_spaces)
       #
       # @return [Boolean]
       def lvm?
-        !!(planned_vg && planned_vg.missing_space > DiskSize.zero)
+        planned_vgs.any? { |vg| vg.missing_space > DiskSize.zero }
       end
 
       protected
 
-      # When calculating an LVM proposal, this represents the projected "system"
-      # volume group to accommodate root and other volumes.
+      # When calculating an LVM proposal, this represents the projected volume groups for
+      # which is necessary to automatically allocate physical volumes (based on their respective
+      # values for {Planned::LvmVg#pvs_candidate_devices}.
       #
-      # Nil if LVM is not involved (partition-based proposal)
+      # Empty if LVM is not involved (partition-based proposal)
       #
-      # @return [Planned::LvmVg, nil]
-      attr_reader :planned_vg
+      # @return [Array<Planned::LvmVg>]
+      attr_reader :planned_vgs
 
       # Checks whether there is any chance of producing a valid
       # PartitionsDistribution to accomodate the planned partitions and the
@@ -145,7 +148,8 @@ def lvm?
       def impossible?(planned_partitions, free_spaces)
         if lvm?
           # Let's assume the best possible case - if we need to create a PV it will be only one
-          planned_partitions += [planned_vg.single_pv_partition]
+          # per volume group
+          planned_partitions += planned_vgs.map(&:single_pv_partition)
         end
 
         # First, do the simplest calculation - checking total sizes
@@ -197,6 +201,19 @@ def candidate_disk_spaces(planned_partitions, free_spaces, extra_spaces = [])
         end
       end
 
+      # @see #best_distribution
+      #
+      # @param candidates [Array<Planned::PartitionsDistribution>]
+      # @param all_spaces [Array<FreeDiskSpace>]
+      def add_physical_volumes(candidates, spaces)
+        candidates.map! do |dist|
+          planned_vgs.inject(dist) do |res, planned_vg|
+            pv_calculator = PhysVolCalculator.new(spaces, planned_vg)
+            pv_calculator.add_physical_volumes(res)
+          end
+        end
+      end
+
       # All possible combinations of spaces and planned partitions.
       #
       # The result is an array in which each entry represents a potential
@@ -247,7 +264,20 @@ def partition_candidate_spaces(partition, candidate_spaces, extra_spaces)
       #
       # @return [Boolean]
       def compatible_disk?(partition, space)
-        return true unless partition.disk && partition.disk != space.disk_name
+        return false if partition.disk && partition.disk != space.disk_name
+
+        vg = planned_vg_for(partition)
+        return true unless vg
+
+        vg.pvs_candidate_devices.include?(space.disk_name)
+      end
+
+      # Planned volume group associated to the given partition, if any
+      #
+      # @param planned_partition [Planned::Partition]
+      # @return [Planned::LvmVg, nil] nil if the partition is not meant as an LVM PV
+      def planned_vg_for(planned_partition)
+        planned_vgs.find { |vg| vg.volume_group_name == planned_partition.lvm_volume_group_name }
       end
 
       # @param partition [Planned::Partition]
@@ -441,8 +471,8 @@ def all_planned_partitions(planned_partitions)
         return planned_partitions unless lvm?
 
         # In the LVM case, assume the worst case - that there will be only
-        # one big PV and we have to make room for it as well.
-        planned_partitions + [planned_vg.single_pv_partition]
+        # one big PV per volume group and we have to make room for them as well.
+        planned_partitions + planned_vgs.map(&:single_pv_partition)
       end
 
       # Size that is missing in the space marked as "growing" in order to

src/lib/y2storage/proposal/phys_vol_calculator.rb

@@ -78,9 +78,12 @@ def add_physical_volumes(distribution)
       #   physical volume
       def spaces_in_valid_disks(all_spaces)
         disk_name = @planned_vg.forced_disk_name
-        return all_spaces unless disk_name
+        return all_spaces.select { |i| i.disk_name == disk_name } if disk_name
 
-        all_spaces.select { |i| i.disk_name == disk_name }
+        disk_names = @planned_vg.pvs_candidate_devices
+        return all_spaces if disk_names.empty?
+
+        all_spaces.select { |s| disk_names.include?(s.disk_name) }
       end
     end
   end

src/lib/y2storage/proposal/space_maker.rb

@@ -65,20 +65,20 @@ def initialize(disk_analyzer, settings)
       #
       # @param original_graph [Devicegraph] initial devicegraph
       # @param planned_partitions [Array<Planned::Partition>] set of partitions to make space for
-      # @param planned_vg [Planned::LvmVg, nil] planned system LVM volume group. Nil if there is no
-      #   need to create space for the system VG
+      # @param planned_vg [Array<Planned::LvmVg>] LVM volume groups for which we need to allocate
+      #   physical volumes (ie. they have some values at Planned::LvmVg#pvs_candidate_devices).
       # @return [Hash] a hash with three elements:
       #   devicegraph: [Devicegraph] resulting devicegraph
       #   deleted_partitions: [Array<Partition>] partitions that
       #     were in the original devicegraph but are not in the resulting one
       #   partitions_distribution: [Planned::PartitionsDistribution] proposed
       #     distribution of partitions, including new PVs if necessary
       #
-      def provide_space(original_graph, planned_partitions, planned_vg = nil)
+      def provide_space(original_graph, planned_partitions, planned_vgs = [])
         @original_graph = original_graph
-        @dist_calculator = PartitionsDistributionCalculator.new(planned_vg)
+        @dist_calculator = PartitionsDistributionCalculator.new(planned_vgs)
 
-        calculate_new_graph(planned_partitions, planned_vg)
+        calculate_new_graph(planned_partitions, planned_vgs)
 
         {
           devicegraph:             new_graph,
@@ -123,6 +123,10 @@ def prepare_devicegraph(original_graph, planned_partitions = [])
       # @return [Array<String>]
       attr_reader :extra_disk_names
 
+      # Disks that are involved in the creation of the requested LVM volume groups
+      # @return [Array<String>]
+      attr_reader :lvm_disk_names
+
       # New devicegraph calculated by {#provide_space}
       # @return [Devicegraph]
       attr_reader :new_graph
@@ -155,11 +159,13 @@ def non_candidate_disks(planned_partitions)
       # @see #provide_space
       #
       # @param partitions [Array<Planned::Partition>] partitions to make space for
-      # @param volume_group [Planned::LvmVg, nil] planned system LVM volume group
-      def calculate_new_graph(partitions, volume_group)
+      # @param volume_groups [Array<Planned::LvmVg>] planned LVM volume groups needing some PVs
+      def calculate_new_graph(partitions, volume_groups)
         @new_graph = original_graph.duplicate
         @new_graph_deleted_sids = []
-        @extra_disk_names = partitions.map(&:disk).compact.uniq - settings.candidate_devices
+        @lvm_disk_names = volume_groups.flat_map(&:pvs_candidate_devices).uniq
+        part_disks = partitions.map(&:disk).compact.uniq
+        @extra_disk_names = part_disks + @lvm_disk_names - settings.candidate_devices
 
         # To make sure we are not freeing space in useless places first
         # restrict the operations to disks with particular disk
@@ -186,7 +192,7 @@ def calculate_new_graph(partitions, volume_group)
           # The result (if successful) is kept in @distribution.
           #
           parts_by_disk.each do |disk, parts|
-            resize_and_delete(parts, volume_group, disk_name: disk)
+            resize_and_delete(parts, volume_groups, disk_name: disk)
           rescue Error
             # If LVM was involved, maybe there is still hope if we don't abort on this error.
             raise unless dist_calculator.lvm?
@@ -204,7 +210,7 @@ def calculate_new_graph(partitions, volume_group)
         # Note that the result of the run above is not lost as already
         # assigned partitions are taken into account.
         #
-        resize_and_delete(partitions, volume_group)
+        resize_and_delete(partitions, volume_groups)
 
         @all_deleted_sids.concat(new_graph_deleted_sids)
       end
@@ -252,10 +258,10 @@ def find_distribution(planned_partitions, ignore_lvm: false)
       #
       # @param planned_partitions [Array<Planned::Partition>] partitions
       #     to make space for
-      # @param volume_group [Planned::LvmVg, nil] planned system LVM volume group, if any
+      # @param volume_groups [Array<Planned::LvmVg>] planned LVM volume groups needing some PVs
       # @param disk_name [String, nil] optional disk name to restrict operations to
       #
-      def resize_and_delete(planned_partitions, volume_group, disk_name: nil)
+      def resize_and_delete(planned_partitions, volume_groups, disk_name: nil)
         # Note that only the execution with disk_name == nil is the final one.
         # In other words, if disk_name contains something, it means there will
         # be at least a subsequent call to the method.
@@ -270,7 +276,7 @@ def resize_and_delete(planned_partitions, volume_group, disk_name: nil)
 
         actions = SpaceMakerActions::List.new(settings.space_settings, disk_analyzer)
         disks_for(new_graph, disk_name).each do |disk|
-          actions.add_optional_actions(disk, volume_group)
+          actions.add_optional_actions(disk, volume_groups)
         end
         skip = all_protected_sids(new_graph)
 
@@ -403,8 +409,8 @@ def execute_wipe(action, devicegraph)
       # @return [DiskSize]
       def resizing_size(partition, planned_partitions, disk_name)
         spaces = free_spaces(disk_name)
-        if disk_name && extra_disk_names.include?(disk_name)
-          # Operating in a disk that is not a candidate_device, no need to make extra space for LVM
+        if disk_name && !lvm_disk_names.include?(disk_name)
+          # Operating in a disk that will not allocate a VG, no need to make extra space for it
           return non_lvm_dist_calculator.resizing_size(partition, planned_partitions, spaces)
         end
 

src/lib/y2storage/proposal/space_maker_actions/auto_strategy.rb

@@ -53,9 +53,9 @@ def add_mandatory_actions(disk)
         end
 
         # @param disk [Disk] see {List}
-        # @param volume_group [Planned::LvmVg, nil] system LVM VG to be created or reused, if any
-        def add_optional_actions(disk, volume_group)
-          prospects.add_prospects(disk, volume_group)
+        # @param volume_groups [Array<Planned::LvmVg>] LVM VGs to be reused
+        def add_optional_actions(disk, volume_groups)
+          prospects.add_prospects(disk, volume_groups)
         end
 
         # @return [Action, nil] nil if there are no more actions in the list

src/lib/y2storage/proposal/space_maker_actions/list.rb

@@ -52,9 +52,9 @@ def add_mandatory_actions(disk)
         # @see SpaceMaker#provide_space
         #
         # @param disk [Disk] disk to act upon
-        # @param volume_group [Planned::LvmVg, nil] system LVM VG to be created or reused, if any
-        def add_optional_actions(disk, volume_group)
-          strategy.add_optional_actions(disk, volume_group)
+        # @param volume_groups [Array<Planned::LvmVg>] LVM VGs to be reused
+        def add_optional_actions(disk, volume_groups)
+          strategy.add_optional_actions(disk, volume_groups)
         end
 
         # Next action to be performed by SpaceMaker

src/lib/y2storage/proposal/space_maker_prospects/list.rb

@@ -207,31 +207,31 @@ def add_resize_prospects(disk)
         # content (i.e. prospects of type {SpaceMakerProspects::WipeDisk})
         #
         # @param disk [Disk] disk to act upon
-        # @param volume_group [Planned::LvmVg, nil] system LVM VG to be created or reused, if any
-        def add_wipe_prospects(disk, volume_group)
+        # @param volume_groups [Array<Planned::LvmVg>] LVM VGs to be reused
+        def add_wipe_prospects(disk, volume_groups)
           log.info "Checking if the disk #{disk.name} has a partition table"
 
           return unless disk.has_children? && disk.partition_table.nil?
 
           log.info "Found something that is not a partition table"
 
-          if disk.descendants.any? { |dev| vg_to_reuse?(dev, volume_group) }
+          if disk.descendants.any? { |dev| vg_to_reuse?(dev, volume_groups) }
             log.info "Not cleaning up #{disk.name} because its VG must be reused"
             return
           end
 
           @wipe_disk_prospects << SpaceMakerProspects::WipeDisk.new(disk)
         end
 
-        # Whether the given device corresponds to the volume group that must be reused by the
+        # Whether the given device corresponds to a volume group that must be reused by the
         # proposal
         #
         # @param device [Device]
-        # @param volume_group [Planned::LvmVg, nil]
-        def vg_to_reuse?(device, volume_group)
-          return false unless volume_group && device.is?(:lvm_vg)
+        # @param volume_group [Array<Planned::LvmVg>]
+        def vg_to_reuse?(device, volume_groups)
+          return false unless device.is?(:lvm_vg)
 
-          volume_group.reuse? && volume_group.volume_group_name == device.vg_name
+          volume_groups.any? { |vg| vg.reuse? && vg.volume_group_name == device.vg_name }
         end
 
         # @see #add_resize_prospects

test/y2storage/proposal/partitions_distribution_calculator_test.rb

@@ -38,7 +38,7 @@
     fake_scenario(scenario)
   end
 
-  subject(:calculator) { described_class.new(planned_vg) }
+  subject(:calculator) { described_class.new([planned_vg]) }
 
   describe "#best_distribution" do
     let(:vol1) { planned_vol(mount_point: "/1", type: :ext4, min: 1.GiB, max: 3.GiB, weight: 1) }