Allow to customize how the proposal must handle pre-existing LVs in a reused VG

package/yast2-storage-ng.changes

@@ -1,3 +1,12 @@
+-------------------------------------------------------------------
+Fri Mar  6 10:50:26 UTC 2026 - Ancor Gonzalez Sosa <ancor@suse.com>
+
+- Added the needed infrastructure to specify what to do with the
+  existing volumes in a reused LVM volume group (related to
+  jsc#PED-15104, bsc#1254718 and gh#agama-project/agama#3171).
+- Fixed creation of thin pools within pre-existing thin pools.
+- 5.0.41
+
 -------------------------------------------------------------------
 Thu Jan 29 09:00:40 UTC 2026 - Ancor Gonzalez Sosa <ancor@suse.com>
 

package/yast2-storage-ng.spec

@@ -16,7 +16,7 @@
 #
 
 Name:           yast2-storage-ng
-Version:        5.0.40
+Version:        5.0.41
 Release:        0
 Summary:        YaST2 - Storage Configuration
 License:        GPL-2.0-only OR GPL-3.0-only

src/lib/y2storage/planned/lvm_vg.rb

@@ -135,6 +135,20 @@ def initialize_from_real_vg(real_vg)
         self.reuse_name = real_vg.vg_name
       end
 
+      # Redefines the corresponding method from the base class
+      #
+      # @see Device#assign_reuse
+      #
+      # For some reason (maybe just a historical mistake), the usage of #reuse_name is inconsistent
+      # in this class compared to the rest. Instead of using the device name, it uses the volume
+      # group name.
+      #
+      # @param device [Y2Storage::LvmVg]
+      def assign_reuse(device)
+        super(device)
+        @reuse_name = device.vg_name
+      end
+
       # Min size that a partition (or any other block device) must have to be useful as PV
       #
       # @return [DiskSize]

src/lib/y2storage/proposal/lvm_creator.rb

@@ -20,6 +20,7 @@
 require "y2storage/planned"
 require "y2storage/disk_size"
 require "y2storage/proposal/creator_result"
+require "y2storage/proposal/lvm_space_maker"
 
 module Y2Storage
   module Proposal
@@ -39,8 +40,12 @@ class LvmCreator
       # Constructor
       #
       # @param original_devicegraph [Devicegraph] Initial devicegraph
-      def initialize(original_devicegraph)
+      # @param space_settings [ProposalSpaceSettings, nil] Optional settings to customize what to do
+      #   with every existing logical volume. If omitted, the traditional YaST approach is used
+      #   (ie. the strategy "auto" is used, see {LvmSpaceStrategies::Auto}).
+      def initialize(original_devicegraph, space_settings = nil)
         @original_devicegraph = original_devicegraph
+        @space_settings = space_settings
       end
 
       # Returns a copy of the original devicegraph in which the volume
@@ -129,61 +134,13 @@ def assign_physical_volumes(volume_group, part_names, devicegraph)
 
       # Makes space for planned logical volumes
       #
-      # When making free space, three different policies can be followed:
-      #
-      # * :needed: remove logical volumes until there's enough space for
-      #            planned ones.
-      # * :remove: remove all logical volumes.
-      # * :keep:   keep all logical volumes.
-      #
       # This method modifies the volume group received as first argument.
       #
       # @param volume_group [LvmVg] volume group to clean-up
       # @param planned_vg   [Planned::LvmVg] planned logical volume
       def make_space(volume_group, planned_vg)
-        return if planned_vg.make_space_policy == :keep
-
-        case planned_vg.make_space_policy
-        when :needed
-          make_space_until_fit(volume_group, planned_vg.lvs)
-        when :remove
-          lvs_to_keep = planned_vg.all_lvs.select(&:reuse?).map(&:reuse_name)
-          remove_logical_volumes(volume_group, lvs_to_keep)
-        end
-      end
-
-      # Makes sure the given volume group has enough free extends to allocate
-      # all the planned volumes, by deleting the existing logical volumes.
-      #
-      # This method modifies the volume group received as first argument.
-      #
-      # FIXME: the current implementation does not guarantee than the freed
-      # space is the minimum valid one.
-      #
-      # @param volume_group [LvmVg] volume group to modify
-      def make_space_until_fit(volume_group, planned_lvs)
-        space_size = DiskSize.sum(planned_lvs.map(&:min_size))
-        missing = missing_vg_space(volume_group, space_size)
-        while missing > DiskSize.zero
-          lv_to_delete = delete_candidate(volume_group, missing)
-          if lv_to_delete.nil?
-            error_msg = "The volume group #{volume_group.vg_name} is not big enough"
-            raise NoDiskSpaceError, error_msg
-          end
-          volume_group.delete_lvm_lv(lv_to_delete)
-          missing = missing_vg_space(volume_group, space_size)
-        end
-      end
-
-      # Remove all logical volumes from a volume group
-      #
-      # This method modifies the volume group received as a first argument.
-      #
-      # @param volume_group [LvmVg]         volume group to remove logical volumes from
-      # @param lvs_to_keep  [Array<String>] name of logical volumes to keep
-      def remove_logical_volumes(volume_group, lvs_to_keep)
-        lvs_to_remove = volume_group.all_lvm_lvs.reject { |v| lvs_to_keep.include?(v.name) }
-        lvs_to_remove.each { |v| volume_group.delete_lvm_lv(v) }
+        space_maker = LvmSpaceMaker.new(volume_group, planned_vg, @space_settings)
+        space_maker.provide_space
       end
 
       # Creates a logical volume for each planned volume.
@@ -196,14 +153,17 @@ def remove_logical_volumes(volume_group, lvs_to_keep)
       # @return [Hash{String => Planned::LvmLv}] planned LVs indexed by the
       #   device name of the real LV devices that were created
       def create_logical_volumes(volume_group, planned_lvs)
-        adjusted_lvs = planned_lvs_in_vg(planned_lvs, volume_group)
+        adjusted_lvs = planned_lvs_in_vg(planned_lvs, volume_group).reject(&:reuse?)
         vg_size = volume_group.available_space
         lvs = Planned::LvmLv.distribute_space(adjusted_lvs, vg_size, rounding: volume_group.extent_size)
-        all_lvs = lvs + lvs.map(&:thin_lvs).flatten
+        all_lvs = lvs + lvs.map(&:thin_lvs).flatten + thin_lvs_from_reused_pools(planned_lvs)
         all_lvs.reject(&:reuse?).each_with_object({}) do |planned_lv, devices_map|
           new_lv = create_logical_volume(volume_group, planned_lv)
           devices_map[new_lv.name] = planned_lv
         end
+      rescue RuntimeError => e
+        log.info "The logical volumes do not fit into the volume group: #{e}"
+        raise NoDiskSpaceError
       end
 
       # Creates a logical volume in a volume group
@@ -229,32 +189,6 @@ def create_logical_volume(volume_group, planned_lv)
         new_lv
       end
 
-      # Best logical volume to delete next while trying to make space for the
-      # planned volumes. It returns the smallest logical volume that would
-      # fulfill the goal. If no LV is big enough, it returns the biggest one.
-      def delete_candidate(volume_group, target_space)
-        lvs = volume_group.lvm_lvs
-        big_lvs = lvs.select { |lv| lv.size >= target_space }
-        if big_lvs.empty?
-          lvs.max_by(&:size)
-        else
-          big_lvs.min_by(&:size)
-        end
-      end
-
-      # Missing space in the volume group to fullfil a target
-      #
-      # @param volume_group [LvmVg]    Volume group
-      # @param target_space [DiskSize] Required space
-      def missing_vg_space(volume_group, target_space)
-        available = volume_group.available_space
-        if available > target_space
-          DiskSize.zero
-        else
-          target_space - available
-        end
-      end
-
       # Returns the name that is available taking original_name as a base. If
       # the name is already taken, the returned name will have a number
       # appended.
@@ -310,6 +244,15 @@ def planned_lvs_in_vg(lvs, vg)
         end
       end
 
+      # Returns a list of planned logical thin volumes that should be created in thin pools
+      # that already exist.
+      #
+      # @param lvs [Array<Planned::LvmLv>] List of planned logical volumes
+      # @return [Array<Planned::LvmLv]
+      def thin_lvs_from_reused_pools(lvs)
+        lvs.select(&:reuse?).flat_map(&:thin_lvs)
+      end
+
       # Helper method to set stripes attributes
       #
       # @param lv         [LvmLv] Logical volume

src/lib/y2storage/proposal/lvm_space_maker.rb

@@ -0,0 +1,77 @@
+# Copyright (c) [2026] 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.
+
+require "y2storage/proposal/lvm_space_strategies"
+
+module Y2Storage
+  module Proposal
+    # Class to provide free space in a volume group by resizing and deleting pre-existing logical
+    # volumes. Analogous to what SpaceMaker does with partitions.
+    class LvmSpaceMaker
+      include Yast::Logger
+
+      # Strategies to use to find space. Equivalent to the corresponding SpaceMaker strategies.
+      STRATEGIES = {
+        auto:          LvmSpaceStrategies::Auto,
+        bigger_resize: LvmSpaceStrategies::BiggerResize
+      }
+      private_constant :STRATEGIES
+
+      # Constructor
+      #
+      # @param volume_group [LvmVg] volume group to clean-up
+      # @param planned_vg   [Planned::LvmVg] planned logical volume
+      # @param space_settings [ProposalSpaceSettings, nil] Optional settings. See
+      #   {LvmCreator#initialize}.
+      def initialize(volume_group, planned_vg, space_settings)
+        @volume_group = volume_group
+        @planned_vg = planned_vg
+        @space_settings = space_settings
+
+        if STRATEGIES[strategy]
+          @strategy_class = STRATEGIES[strategy]
+        else
+          err_msg = "Unsupported LVM strategy to make space: #{strategy}"
+          log.error err_msg
+          raise ArgumentError, err_msg
+        end
+      end
+
+      # Makes space for planned logical volumes
+      #
+      # This method modifies the volume group received as first argument.
+      def provide_space
+        log.info "Making space at LVM volume group with strategy #{strategy}"
+        log.info "vg: #{@volume_group.name}, planned: #{@planned_vg.inspect}"
+        @strategy_class.new(@volume_group, @planned_vg, @space_settings).provide_space
+      end
+
+      private
+
+      # Id of the strategy to be used
+      #
+      # @return [Symbol]
+      def strategy
+        return :auto unless @space_settings
+
+        @space_settings.strategy
+      end
+    end
+  end
+end

src/lib/y2storage/proposal/lvm_space_strategies.rb

@@ -0,0 +1,31 @@
+# Copyright (c) [2026] 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 Y2Storage
+  module Proposal
+    # Namespace to group all the strategies used by LvmSpaceMaker.
+    #
+    # There is a strategy to mimic each strategy at SpaceMakerActions.
+    module LvmSpaceStrategy
+    end
+  end
+end
+
+require "y2storage/proposal/lvm_space_strategies/auto"
+require "y2storage/proposal/lvm_space_strategies/bigger_resize"

src/lib/y2storage/proposal/lvm_space_strategies/auto.rb

@@ -0,0 +1,84 @@
+# Copyright (c) [2017-2026] 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.
+
+require "y2storage/proposal/lvm_space_strategies/base"
+
+module Y2Storage
+  module Proposal
+    module LvmSpaceStrategies
+      # Traditional strategy used by the YaST and AutoYaST proposals to make space in a existing LVM
+      # volume group
+      #
+      # The behavior depends on the value of {Planned::LvmVg#make_space_policy}.
+      class Auto < Base
+        # Makes space for planned logical volumes
+        #
+        # When making free space, three different policies can be followed:
+        #
+        # * :needed: remove logical volumes until there's enough space for
+        #            planned ones.
+        # * :remove: remove all logical volumes.
+        # * :keep:   keep all logical volumes.
+        #
+        # @see Base#provide_space
+        def provide_space
+          return if planned_vg.make_space_policy == :keep
+
+          case planned_vg.make_space_policy
+          when :needed
+            make_space_until_fit
+          when :remove
+            lvs_to_keep = planned_vg.all_lvs.select(&:reuse?).map(&:reuse_name)
+            remove_logical_volumes(lvs_to_keep)
+          end
+        end
+
+        private
+
+        # Makes sure the given volume group has enough free extends to allocate
+        # all the planned volumes, by deleting the existing logical volumes.
+        #
+        # This method modifies the volume group received as first argument.
+        #
+        # FIXME: the current implementation does not guarantee that the freed
+        # space is the minimum valid one.
+        def make_space_until_fit
+          while missing_vg_space > DiskSize.zero
+            lv_to_delete = delete_candidate(volume_group.lvm_lvs)
+            if lv_to_delete.nil?
+              error_msg = "The volume group #{volume_group.vg_name} is not big enough"
+              raise NoDiskSpaceError, error_msg
+            end
+            volume_group.delete_lvm_lv(lv_to_delete)
+          end
+        end
+
+        # Remove all logical volumes from a volume group
+        #
+        # This method modifies the volume group received as a first argument.
+        #
+        # @param lvs_to_keep  [Array<String>] name of logical volumes to keep
+        def remove_logical_volumes(lvs_to_keep)
+          lvs_to_remove = volume_group.all_lvm_lvs.reject { |v| lvs_to_keep.include?(v.name) }
+          lvs_to_remove.each { |v| volume_group.delete_lvm_lv(v) }
+        end
+      end
+    end
+  end
+end