More flexible configuration for the actions of bigger_resize

src/lib/y2storage/proposal/space_maker.rb

@@ -361,14 +361,17 @@ def execute_action(action, graph, skip, planned_partitions = nil, disk_name = ni
       def execute_shrink(action, devicegraph, planned_partitions, disk_name)
         log.info "SpaceMaker#execute_shrink - #{action}"
 
-        if action.shrink_size.nil?
+        if action.target_size.nil?
+          part = devicegraph.find_device(action.sid)
           if planned_partitions
-            part = devicegraph.find_device(action.sid)
-            action.shrink_size = resizing_size(part, planned_partitions, disk_name)
+            resizing = resizing_size(part, planned_partitions, disk_name)
+            action.target_size = resizing > part.size ? DiskSize.zero : part.size - resizing
           else
-            action.shrink_size = DiskSize.Unlimited
+            # Mandatory resize
+            action.target_size = part.size
           end
         end
+
         action.shrink(devicegraph)
       end
 

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

@@ -35,111 +35,167 @@ def initialize(settings, _disk_analyzer)
           @to_delete_mandatory = []
           @to_delete_optional = []
           @to_wipe = []
-          @to_resize = []
+          @to_shrink_mandatory = []
+          @to_shrink_optional = []
         end
 
         # @param disk [Disk] see {List}
         def add_mandatory_actions(disk)
           return unless disk.partition_table?
 
-          devices = partitions(disk).select { |p| configured?(p, :force_delete) }
-          to_delete_mandatory.concat(devices)
+          add_mandatory_delete(disk)
+          add_mandatory_shrink(disk)
         end
 
         # @param disk [Disk] see {List}
         def add_optional_actions(disk, _lvm_helper)
           add_wipe(disk)
-          add_resize(disk)
+          add_optional_shrink(disk)
           add_optional_delete(disk)
         end
 
         # @return [Action, nil] nil if there are no more actions in the list
         def next
           source = source_for_next
-          dev = send(source).first
-          return unless dev
-
-          return Shrink.new(dev) if source == :to_resize
-          return Wipe.new(dev) if source == :to_wipe
-
-          Delete.new(dev, related_partitions: false)
+          send(source).first
         end
 
         # @param deleted_sids [Array<Integer>] see {List}
         def done(deleted_sids)
           send(source_for_next).shift
           cleanup(to_delete_mandatory, deleted_sids)
           cleanup(to_delete_optional, deleted_sids)
-          cleanup(to_resize, deleted_sids)
+          cleanup(to_shrink_mandatory, deleted_sids)
+          cleanup(to_shrink_optional, deleted_sids)
         end
 
         private
 
         # @return [ProposalSpaceSettings] proposal settings for making space
         attr_reader :settings
 
-        # @return [Array<BlkDevice>] list of devices to be deleted (mandatory)
+        # @return [Array<Base>] list of mandatory delete actions
         attr_reader :to_delete_mandatory
 
-        # @return [Array<BlkDevice>] list of devices to be deleted (optionally)
+        # @return [Array<Base>] list of optional delete actions
         attr_reader :to_delete_optional
 
-        # @return [Array<Partition>] list of partitions to be shrunk
-        attr_reader :to_resize
+        # @return [Array<Base>] list of mandatory shrink actions
+        attr_reader :to_shrink_mandatory
 
-        # @return [Array<BlkDevice>] list of disks to be emptied if needed
+        # @return [Array<Base>] list of optional shrink actions
+        attr_reader :to_shrink_optional
+
+        # @return [Array<Base>] list of actions to wipe disks if needed
         attr_reader :to_wipe
 
         # @see #add_optional_actions
         # @param disk [Disk]
         def add_wipe(disk)
           return if disk.partition_table?
 
-          to_wipe << disk
+          to_wipe << Wipe.new(disk)
         end
 
         # @see #add_optional_actions
         # @param disk [Disk]
-        def add_resize(disk)
+        def add_optional_shrink(disk)
           return unless disk.partition_table?
 
-          partitions = partitions(disk).select { |p| configured?(p, :resize) }
-          return if partitions.empty?
+          actions = optional_shrinks(disk)
+          return if actions.empty?
+
+          @to_shrink_optional = (to_shrink_optional + actions).sort do |a, b|
+            preferred_resize(a, b, disk.devicegraph)
+          end
+        end
+
+        # @see #add_optional_shrink
+        # @param disk [Disk]
+        def optional_shrinks(disk)
+          partitions(disk).map do |part|
+            resize = resize_actions.find { |a| a.device == part.name }
+            next unless resize
+            next if resize.min_size && resize.min_size > part.size
+            next if resize.max_size && resize.max_size < part.size
 
-          @to_resize = (to_resize + partitions).sort { |a, b| preferred_resize(a, b) }
+            resize_to_shrink(part, resize)
+          end.compact
         end
 
-        # Compares two partitions to decide which one should be resized first
+        # Compares two shrinking operations to decide which one should be executed first
         #
-        # @param part1 [Partition]
-        # @param part2 [Partition]
-        def preferred_resize(part1, part2)
-          result = part2.recoverable_size <=> part1.recoverable_size
+        # @param resize1 [Shrink]
+        # @param resize2 [Shrink]
+        def preferred_resize(resize1, resize2, devicegraph)
+          part1 = devicegraph.find_device(resize1.sid)
+          part2 = devicegraph.find_device(resize2.sid)
+          result = recoverable_size(part2, resize2) <=> recoverable_size(part1, resize1)
           return result unless result.zero?
 
           # Just to ensure stable sorting between different executions in case of draw
           part1.name <=> part2.name
         end
 
+        # Max space that can be recovered from the given partition, having into account the
+        # restrictions imposed by the its Resize action
+        #
+        # @see #preferred_resize
+        def recoverable_size(partition, resize)
+          if resize.min_size.nil? || resize.min_size > partition.size
+            return partition.recoverable_size
+          end
+
+          [partition.recoverable_size, partition.size - resize.min_size].min
+        end
+
         # @see #add_optional_actions
         #
         # @param disk [Disk]
         def add_optional_delete(disk)
           return unless disk.partition_table?
 
           partitions = partitions(disk).select { |p| configured?(p, :delete) }
-          to_delete_optional.concat(partitions.sort { |a, b| preferred_delete(a, b) })
+          partitions.sort! { |a, b| preferred_delete(a, b) }
+          actions = partitions.map { |p| Delete.new(p, related_partitions: false) }
+          to_delete_optional.concat(actions)
         end
 
         # Compares two partitions to decide which one should be deleted first
         #
         # @param part1 [Partition]
         # @param part2 [Partition]
         def preferred_delete(part1, part2)
-          # Mimic order from the auto strategy. We might consider other approaches in the future.
+          # FIXME: Currently this mimics the order from the auto strategy.
+          # We might consider other approaches in the future, like deleting partitions that are
+          # next to another partition that needs to grow. That circumstance is maybe not so easy
+          # to evaluate at the start and needs to be reconsidered after every action.
           part2.region.start <=> part1.region.start
         end
 
+        # @see #add_optional_actions
+        # @param disk [Disk]
+        def add_mandatory_shrink(disk)
+          shrink_actions = partitions(disk).map do |part|
+            resize = resize_actions.find { |a| a.device == part.name }
+            next unless resize
+            next unless resize.max_size
+            next if part.size <= resize.max_size
+
+            resize_to_shrink(part, resize)
+          end.compact
+
+          to_shrink_mandatory.concat(shrink_actions)
+        end
+
+        # @see #add_mandatory_actions
+        # @param disk [Disk]
+        def add_mandatory_delete(disk)
+          devices = partitions(disk).select { |p| configured?(p, :force_delete) }
+          actions = devices.map { |d| Delete.new(d, related_partitions: false) }
+          to_delete_mandatory.concat(actions)
+        end
+
         # Whether the given action is configured for the given device at the proposal settings
         #
         # @see ProposalSpaceSettings#actions
@@ -148,12 +204,17 @@ def preferred_delete(part1, part2)
         # @param action [Symbol] :force_delete, :delete or :resize
         # @return [Boolean]
         def configured?(device, action)
-          settings.actions[device.name]&.to_sym == action
+          case action
+          when :force_delete
+            delete_actions.select(&:mandatory).any? { |a| a.device == device.name }
+          when :delete
+            delete_actions.reject(&:mandatory).any? { |a| a.device == device.name }
+          end
         end
 
         # Removes devices with the given sids from a collection
         #
-        # @param collection [Array<BlkDevice>]
+        # @param collection [Array<Action>]
         # @param deleted_sids [Array<Integer>]
         def cleanup(collection, deleted_sids)
           collection.delete_if { |d| deleted_sids.include?(d.sid) }
@@ -167,8 +228,10 @@ def source_for_next
             :to_delete_mandatory
           elsif to_wipe.any?
             :to_wipe
-          elsif to_resize.any?
-            :to_resize
+          elsif to_shrink_mandatory.any?
+            :to_shrink_mandatory
+          elsif to_shrink_optional.any?
+            :to_shrink_optional
           else
             :to_delete_optional
           end
@@ -178,6 +241,24 @@ def source_for_next
         def partitions(disk)
           disk.partitions.reject { |part| part.type.is?(:extended) }
         end
+
+        # Trivial conversion
+        def resize_to_shrink(partition, resize)
+          Shrink.new(partition).tap do |shrink|
+            shrink.min_size = resize.min_size
+            shrink.max_size = resize.max_size
+          end
+        end
+
+        # All delete actions from the settings
+        def delete_actions
+          settings.actions.select { |a| a.is?(:delete) }
+        end
+
+        # All resize actions from the settings
+        def resize_actions
+          settings.actions.select { |a| a.is?(:resize) }
+        end
       end
     end
   end

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

@@ -26,12 +26,25 @@ module SpaceMakerActions
       #
       # @see Base
       class Shrink < Base
-        # @return [DiskSize] size of the space to substract ideally
-        attr_accessor :shrink_size
+        # Minimal size of the device set by the configuration, regardless of the ResizeInfo
+        #
+        # If set to nil, there is no artificial limit
+        # @return [DiskSize, nil]
+        attr_accessor :min_size
+
+        # Max size of the device set by the configuration, regardless of the ResizeInfo
+        #
+        # If set to nil, there is no artificial limit
+        # @return [DiskSize, nil]
+        attr_accessor :max_size
+
+        # Ideal final size of the device to satisfy the SpaceMaker algorithm
+        # @return [DiskSize]
+        attr_accessor :target_size
 
         # Reduces the size of the target partition
         #
-        # If possible, it reduces the size of the partition by {#shrink_size}.
+        # If possible, it reduces the size of the partition to {#adjusted_target_size}.
         # Otherwise, it reduces the size as much as possible.
         #
         # This method does not take alignment into account.
@@ -48,9 +61,21 @@ def shrink(devicegraph)
 
         # @param partition [Partition]
         def shrink_partition(partition)
-          target = shrink_size.unlimited? ? DiskSize.zero : partition.size - shrink_size
           # Explicitly avoid alignment to keep current behavior (to be reconsidered)
-          partition.resize(target, align_type: nil)
+          partition.resize(adjusted_target_size, align_type: nil)
+        end
+
+        # Real target size taking into account the max and min limits
+        #
+        # @return [DiskSize]
+        def adjusted_target_size
+          if min_size && min_size > target_size
+            min_size
+          elsif max_size && max_size < target_size
+            max_size
+          else
+            target_size
+          end
         end
       end
     end

src/lib/y2storage/proposal_space_settings.rb

@@ -19,6 +19,7 @@
 
 require "yast"
 require "y2storage/equal_by_instance_variables"
+require "y2storage/space_actions"
 
 module Y2Storage
   # Class to encapsulate all the GuidedProposal settings related to the process of making space
@@ -84,27 +85,20 @@ def self.delete_modes
 
     # What to do with existing partitions if they are involved in the process of making space.
     #
-    # Keys are device names (like in BlkDevice#name, no alternative names) that correspond to a
-    # partition.
-    #
-    # The value for each key specifies what to do with the corresponding partition if the storage
-    # proposal needs to process the corresponding disk. If the device is not explicitly mentioned,
-    # nothing will be done. Possible values are :resize, :delete and :force_delete.
-    #
     # Entries for devices that are not involved in the proposal are ignored. For example, if all
     # the volumes are configured to be placed at /dev/sda but there is an entry like
-    # `{"/dev/sdb1" => :force_delete}`, the corresponding /dev/sdb1 partition will NOT be deleted
-    # because there is no reason for the proposal to process the disk /dev/sdb.
+    # Delete<device: "/dev/sdb1", mandatory: true>, the corresponding /dev/sdb1 partition will NOT
+    # be deleted because there is no reason for the proposal to process the disk /dev/sdb.
     #
     # Device names corresponding to extended partitions are also ignored. The storage proposal only
     # considers actions for primary and logical partitions.
     #
-    # @return [Hash{String => Symbol}]
+    # @return [Array<SpaceActions::Base>]
     attr_accessor :actions
 
     def initialize
       @strategy = :auto
-      @actions = {}
+      @actions = []
     end
 
     # Whether the settings disable deletion of a given type of partitions

src/lib/y2storage/space_actions.rb

@@ -0,0 +1,27 @@
+# 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 Y2Storage
+  # Namespace for the objects representing the actions of the bigger_resize SpaceMaker strategy
+  module SpaceActions
+  end
+end
+
+require "y2storage/space_actions/delete"
+require "y2storage/space_actions/resize"