New strategy for SpaceMaker

src/lib/y2storage/guided_proposal.rb

@@ -249,7 +249,7 @@ def clean_graph
       # the end of the process for those devices that were not used (as soon as libstorage-ng
       # allows us to copy sub-graphs).
       remove_empty_partition_tables(new_devicegraph)
-      @clean_graph = space_maker.delete_unwanted_partitions(new_devicegraph)
+      @clean_graph = space_maker.prepare_devicegraph(new_devicegraph)
     end
 
     # Removes partition tables from candidate devices with empty partition table

src/lib/y2storage/proposal/space_maker.rb

@@ -1,4 +1,4 @@
-# Copyright (c) [2015] SUSE LLC
+# Copyright (c) [2015-2023] SUSE LLC
 #
 # All Rights Reserved.
 #
@@ -19,13 +19,11 @@
 
 require "fileutils"
 require "storage"
-require "y2storage/planned"
-require "y2storage/partition"
+require "y2storage/partitionable"
 require "y2storage/disk_size"
 require "y2storage/free_disk_space"
 require "y2storage/proposal/partitions_distribution_calculator"
-require "y2storage/proposal/partition_killer"
-require "y2storage/proposal/space_maker_prospects"
+require "y2storage/proposal/space_maker_actions"
 
 module Y2Storage
   module Proposal
@@ -98,32 +96,24 @@ def provide_space(original_graph, planned_partitions, lvm_helper)
         }
       end
 
-      # Deletes all partitions explicitly marked for removal in the proposal
-      # settings, i.e. all the partitions belonging to one of the types with
-      # delete_mode set to :all.
-      #
-      # @see #windows_delete_mode
-      # @see #linux_delete_mode
-      # @see #other_delete_mode
+      # Executes all the mandatory actions to make space (see #{SpaceMakerActions::List})
       #
       # @param original_graph [Devicegraph] initial devicegraph
-      # @return [Devicegraph] copy of #original_graph without the unwanted
-      #   partitions
-      def delete_unwanted_partitions(original_graph)
-        log.info "BEGIN SpaceMaker#delete_unwanted_partitions"
+      # @return [Devicegraph] copy of #original_graph after performing the actions
+      def prepare_devicegraph(original_graph)
+        log.info "BEGIN SpaceMaker#prepare_devicegraph"
 
         result = original_graph.dup
-        partition_killer = PartitionKiller.new(result, candidate_disk_names)
-        prospects = SpaceMakerProspects::List.new(settings, disk_analyzer)
+        actions = SpaceMakerActions::List.new(settings.space_settings, disk_analyzer)
+        disks_for(result).each { |d| actions.add_mandatory_actions(d) }
 
-        disks_for(result).each do |disk|
-          prospects.unwanted_partition_prospects(disk).each do |entry|
-            sids = partition_killer.delete_by_sid(entry.sid)
-            @all_deleted_sids.concat(sids)
-          end
+        while (action = actions.next)
+          sids = execute_action(action, result)
+          actions.done(sids)
+          @all_deleted_sids.concat(sids)
         end
 
-        log.info "END SpaceMaker#delete_unwanted_partitions"
+        log.info "END SpaceMaker#prepare_devicegraph"
         result
       end
 
@@ -140,10 +130,6 @@ def delete_unwanted_partitions(original_graph)
       # @return [Devicegraph]
       attr_reader :new_graph
 
-      # Auxiliary killer used to calculate {#new_graph}
-      # @return [PartitionKiller]
-      attr_reader :new_graph_part_killer
-
       # Sids of the partitions deleted while calculating {#new_graph}. In other
       # words, partitions that where in the original devicegraph passed to
       # {#provide_space} but that are not longer there in {#new_graph}.
@@ -167,7 +153,6 @@ def deleted_partitions
       #     to deal with the existing LVM volume groups
       def calculate_new_graph(partitions, keep, lvm_helper)
         @new_graph = original_graph.duplicate
-        @new_graph_part_killer = PartitionKiller.new(@new_graph, candidate_disk_names)
         @new_graph_deleted_sids = []
         @extra_disk_names = partitions.map(&:disk).compact.uniq - settings.candidate_devices
 
@@ -280,13 +265,13 @@ def resize_and_delete(planned_partitions, keep, lvm_helper, disk_name: nil)
         @distribution = nil
         force_ptables(planned_partitions)
 
-        prospects = SpaceMakerProspects::List.new(settings, disk_analyzer)
+        actions = SpaceMakerActions::List.new(settings.space_settings, disk_analyzer)
         disks_for(new_graph, disk_name).each do |disk|
-          prospects.add_prospects(disk, lvm_helper, keep)
+          actions.add_optional_actions(disk, keep, lvm_helper)
         end
 
         until success?(planned_partitions)
-          break unless execute_next_action(planned_partitions, prospects, disk_name)
+          break unless execute_next_action(actions, planned_partitions, disk_name)
         end
 
         raise Error unless @distribution
@@ -306,95 +291,89 @@ def force_ptables(planned_partitions)
 
       # Performs the next action of {#resize_and_delete}
       #
+      # @param actions [SpaceMakerActions::List] set of actions that could be executed
       # @param planned_partitions [Array<Planned::Partition>] set of partitions to make space for
-      # @param prospects [SpaceMakerProspects::List] set of prospect actions
-      #   that could be executed
       # @param disk_name [String] optional disk name to restrict operations to
       # @return [Boolean] true if some operation was performed. False if nothing
       #   else could be done to reach the goal.
-      def execute_next_action(planned_partitions, prospects, disk_name = nil)
-        prospect_action = prospects.next_available_prospect
-        if !prospect_action
-          log.info "No more prospects for SpaceMaker (disk_name: #{disk_name})"
+      def execute_next_action(actions, planned_partitions, disk_name = nil)
+        action = actions.next
+        if !action
+          log.info "No more actions for SpaceMaker (disk_name: #{disk_name})"
           return false
         end
 
-        case prospect_action.to_sym
-        when :resize_partition
-          execute_resize(prospect_action, planned_partitions, disk_name)
-        when :delete_partition
-          execute_delete(prospect_action, prospects)
-        else
-          execute_wipe(prospect_action)
-        end
-
+        sids = execute_action(action, new_graph, planned_partitions, disk_name)
+        actions.done(sids)
+        new_graph_deleted_sids.concat(sids)
         true
       end
 
-      # Performs the action described by a {SpaceMakerProspects::ResizePartition}
+      # Executes the given SpaceMaker action
       #
-      # @param prospect [SpaceMakerProspects::ResizePartition] candidate action
-      #   to execute
-      # @param planned_partitions [Array<Planned::Partition>] set of partitions to make space for
+      # @param action [SpaceMakerActions::Base] action to execute
+      # @param graph [Devicegraph] devicegraph in which the action will be executed
+      # @param planned_partitions [Array<Planned::Partition>, nil] set of partitions to make space
+      #   for, if known. Nil for mandatory operations executed during {#prepare_devicegraph}.
       # @param disk_name [String, nil] optional disk name to restrict operations to
-      def execute_resize(prospect, planned_partitions, disk_name)
-        log.info "SpaceMaker#execute_resize - #{prospect}"
-
-        part = new_graph.find_device(prospect.sid)
-        target_shrink_size = resizing_size(part, planned_partitions, disk_name)
-        shrink_partition(part, target_shrink_size)
-        prospect.available = false
+      # @return [Array<Integer>] sids of the devices that has been deleted as a consequence
+      #   of the action
+      def execute_action(action, graph, planned_partitions = nil, disk_name = nil)
+        case action
+        when SpaceMakerActions::Shrink
+          execute_shrink(action, graph, planned_partitions, disk_name)
+        when SpaceMakerActions::Delete
+          execute_delete(action, graph)
+        else
+          execute_wipe(action, graph)
+        end
       end
 
-      # Performs the action described by a {SpaceMakerProspects::DeletePartition}
+      # Performs the action described by a {SpaceMakerActions::Shrink}
       #
-      # @param prospect [SpaceMakerProspects::DeletePartition] candidate action
-      #   to execute
-      # @param prospects [SpaceMakerProspects::List] full set of prospect actions
-      def execute_delete(prospect, prospects)
-        log.info "SpaceMaker#execute_delete - #{prospect}"
-
-        sids = new_graph_part_killer.delete_by_sid(prospect.sid)
-        new_graph_deleted_sids.concat(sids)
-        prospects.mark_deleted(sids)
-      end
-
-      # Performs the action described by a {SpaceMakerProspects::WipeDisk}
+      # @see #execute_action
       #
-      # @param prospect [SpaceMakerProspects::DeletePartition] candidate action
-      #   to execute
-      def execute_wipe(prospect)
-        log.info "SpaceMaker#execute_wipe - #{prospect}"
-
-        disk = new_graph.find_device(prospect.sid)
-        remove_content(disk)
-        prospect.available = false
+      # @param action [ProposalSpaceAction]
+      # @param devicegraph [Devicegraph]
+      # @param planned_partitions [Array<Planned::Partition>, nil]
+      # @param disk_name [String, nil]
+      # @return [Array<Integer>]
+      def execute_shrink(action, devicegraph, planned_partitions, disk_name)
+        log.info "SpaceMaker#execute_shrink - #{action}"
+
+        if action.shrink_size.nil?
+          if planned_partitions
+            part = devicegraph.find_device(action.sid)
+            action.shrink_size = resizing_size(part, planned_partitions, disk_name)
+          else
+            action.shrink_size = DiskSize.Unlimited
+          end
+        end
+        action.shrink(devicegraph)
       end
 
-      # Reduces the size of a partition
-      #
-      # If possible, it reduces the size of the partition by shrink_size.
-      # Otherwise, it reduces the size as much as possible.
+      # Performs the action described by a {SpaceMakerActions::Delete}
       #
-      # This method does not take alignment into account.
+      # @see #execute_action
       #
-      # @param partition [Partition]
-      # @param shrink_size [DiskSize] size of the space to substract ideally
-      def shrink_partition(partition, shrink_size)
-        log.info "Shrinking #{partition.name}"
-        # Explicitly avoid alignment to keep current behavior (to be reconsidered)
-        partition.resize(partition.size - shrink_size, align_type: nil)
+      # @param action [ProposalSpaceAction]
+      # @param devicegraph [Devicegraph]
+      # @return [Array<Integer>]
+      def execute_delete(action, devicegraph)
+        log.info "SpaceMaker#execute_delete - #{action}"
+        action.delete(devicegraph, candidate_disk_names)
       end
 
-      # Remove descendants of a disk and also partitions from other disks that
-      # are not longer useful afterwards
+      # Performs the action described by a {SpaceMakerActions::Wipe}
       #
-      # TODO: delete partitions that were part of the removed VG and/or RAID
+      # @see #execute_action
       #
-      # @param disk [Partitionable] disk-like device to cleanup. It must not be
-      #   part of a multipath device or a BIOS RAID.
-      def remove_content(disk)
-        disk.remove_descendants
+      # @param action [ProposalSpaceAction]
+      # @param devicegraph [Devicegraph]
+      # @return [Array<Integer>]
+      def execute_wipe(action, devicegraph)
+        log.info "SpaceMaker#execute_wipe - #{action}"
+        action.wipe(devicegraph)
       end
 
       # Additional space that needs to be freed while resizing a partition in

src/lib/y2storage/proposal/space_maker_actions.rb

@@ -0,0 +1,31 @@
+# Copyright (c) [2023] 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 for the classes used to represent the actions performed by SpaceMaker
+    module SpaceMakerProspects
+    end
+  end
+end
+
+require "y2storage/proposal/space_maker_actions/delete"
+require "y2storage/proposal/space_maker_actions/wipe"
+require "y2storage/proposal/space_maker_actions/shrink"
+require "y2storage/proposal/space_maker_actions/list"

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

@@ -0,0 +1,95 @@
+# Copyright (c) [2023] 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/space_maker_prospects"
+
+module Y2Storage
+  module Proposal
+    module SpaceMakerActions
+      # Classical YaST strategy for {SpaceMakerActions::List}, used by default and if the strategy
+      # is configured as :auto.
+      #
+      # This is basically a wrapper around {SpaceMakerProspects::List}, since the original logic
+      # was implemented there.
+      class AutoStrategy
+        # Constructor
+        #
+        # @param settings [ProposalSpaceSettings] proposal settings
+        # @param disk_analyzer [DiskAnalyzer] information about existing partitions
+        def initialize(settings, disk_analyzer)
+          @settings = settings
+          @disk_analyzer = disk_analyzer
+          @prospects = SpaceMakerProspects::List.new(settings, disk_analyzer)
+          @mandatory = []
+        end
+
+        # In the case of the traditional YaST strategy for making space, this corresponds to
+        # deleting all partitions explicitly marked for removal in the proposal settings, i.e.
+        # all the partitions belonging to one of the types with delete_mode set to :all.
+        #
+        # @see ProposalSettings#windows_delete_mode
+        # @see ProposalSettings#linux_delete_mode
+        # @see ProposalSettings#other_delete_mode
+        #
+        # @param disk [Disk] see {List}
+        def add_mandatory_actions(disk)
+          mandatory.concat(prospects.unwanted_partition_prospects(disk))
+        end
+
+        # @param disk [Disk] see {List}
+        # @param keep [Array<Integer>] see {List}
+        # @param lvm_helper [Proposal::LvmHelper] see {List}
+        def add_optional_actions(disk, keep, lvm_helper)
+          prospects.add_prospects(disk, lvm_helper, keep)
+        end
+
+        # @return [Action, nil] nil if there are no more actions in the list
+        def next
+          next_prospect&.action
+        end
+
+        # @param deleted_sids [Array<Integer>] see {List}
+        def done(deleted_sids)
+          if mandatory.any?
+            mandatory.shift
+            return
+          end
+
+          prospects.next_available_prospect.available = false
+          prospects.mark_deleted(deleted_sids)
+        end
+
+        private
+
+        # @return [Array<Action>] list of mandatory actions to be executed
+        attr_reader :mandatory
+
+        # @return [SpaceMakerProspects::List] optional actions to be executed if needed
+        attr_reader :prospects
+
+        # @see #next
+        def next_prospect
+          return @mandatory.first unless @mandatory.empty?
+
+          prospects.next_available_prospect
+        end
+      end
+    end
+  end
+end