Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New strategy for SpaceMaker #1337

Merged
merged 6 commits into from
May 17, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/lib/y2storage/proposal/space_maker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ def prepare_devicegraph(original_graph)
log.info "BEGIN SpaceMaker#prepare_devicegraph"

result = original_graph.dup
actions = SpaceMakerActions::List.new(settings, disk_analyzer)
actions = SpaceMakerActions::List.new(settings.space_settings, disk_analyzer)
disks_for(result).each { |d| actions.add_mandatory_actions(d) }

while (action = actions.next)
Expand Down Expand Up @@ -265,7 +265,7 @@ def resize_and_delete(planned_partitions, keep, lvm_helper, disk_name: nil)
@distribution = nil
force_ptables(planned_partitions)

actions = SpaceMakerActions::List.new(settings, disk_analyzer)
actions = SpaceMakerActions::List.new(settings.space_settings, disk_analyzer)
disks_for(new_graph, disk_name).each do |disk|
actions.add_optional_actions(disk, keep, lvm_helper)
end
Expand Down
2 changes: 1 addition & 1 deletion src/lib/y2storage/proposal/space_maker_actions/list.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ module SpaceMakerActions
class List
# Initialize.
#
# @param settings [ProposalSettings] proposal settings
# @param settings [ProposalSpaceSettings] proposal settings
# @param disk_analyzer [DiskAnalyzer] information about existing partitions
def initialize(settings, disk_analyzer)
@settings = settings
Expand Down
109 changes: 43 additions & 66 deletions src/lib/y2storage/proposal_settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
# find current contact information at www.suse.com.

require "yast"
require "forwardable"
require "y2storage/disk_size"
require "y2storage/secret_attributes"
require "y2storage/volume_specification"
Expand All @@ -27,6 +28,7 @@
require "y2storage/volume_specifications_set"
require "y2storage/encryption_method"
require "y2storage/equal_by_instance_variables"
require "y2storage/proposal_space_settings"

module Y2Storage
# Class to manage settings used by the proposal (typically read from control.xml)
Expand All @@ -37,6 +39,7 @@ class ProposalSettings
include SecretAttributes
include PartitioningFeatures
include EqualByInstanceVariables
extend Forwardable

# @return [Boolean] whether to use LVM
attr_accessor :use_lvm
Expand Down Expand Up @@ -169,39 +172,6 @@ def candidate_devices=(devices)
# @return [PbkdFunction, nil] nil to use the default
attr_accessor :encryption_pbkdf

# @return [Boolean] whether to resize Windows systems if needed
attr_accessor :resize_windows

# What to do regarding removal of existing partitions hosting a Windows system.
#
# Options:
#
# * :none Never delete a Windows partition.
# * :ondemand Delete Windows partitions as needed by the proposal.
# * :all Delete all Windows partitions, even if not needed.
#
# @raise ArgumentError if any other value is assigned
#
# @return [Symbol]
attr_reader :windows_delete_mode

# @return [Symbol] what to do regarding removal of existing Linux
# partitions. See {DiskAnalyzer} for the definition of "Linux partitions".
# @see #windows_delete_mode for the possible values and exceptions
attr_reader :linux_delete_mode

# @return [Symbol] what to do regarding removal of existing partitions that
# don't fit in #windows_delete_mode or #linux_delete_mode.
# @see #windows_delete_mode for the possible values and exceptions
attr_reader :other_delete_mode

# Whether the delete mode of the partitions and the resize option for windows can be
# configured. When this option is set to `false`, the {#windows_delete_mode}, {#linux_delete_mode},
# {#other_delete_mode} and {#resize_windows} options cannot be modified by the user.
#
# @return [Boolean]
attr_accessor :delete_resize_configurable

# When the user decides to use LVM, strategy to decide the size of the volume
# group (and, thus, the number and size of created physical volumes).
#
Expand Down Expand Up @@ -233,6 +203,14 @@ def candidate_devices=(devices)
alias_method :lvm, :use_lvm
alias_method :lvm=, :use_lvm=

# @return [ProposalSpaceSettings]
attr_reader :space_settings

# Constructor
def initialize
@space_settings = ProposalSpaceSettings.new
end

# Volumes grouped by their location in the disks.
#
# This method is only useful when #allocate_volume_mode is set to
Expand Down Expand Up @@ -273,44 +251,48 @@ def use_encryption
!encryption_password.nil?
end

# Whether the settings disable deletion of a given type of partitions
#
# @see #windows_delete_mode
# @see #linux_delete_mode
# @see #other_delete_mode
#
# @param type [#to_s] :linux, :windows or :other
# @return [Boolean]
def delete_forbidden(type)
send(:"#{type}_delete_mode") == :none
end
def_delegators :@space_settings,
:windows_delete_mode, :linux_delete_mode, :other_delete_mode, :resize_windows,
:resize_windows=, :delete_resize_configurable, :delete_resize_configurable=,
:delete_forbidden, :delete_forbidden?, :delete_forced, :delete_forced?

alias_method :delete_forbidden?, :delete_forbidden
# @!attribute windows_delete_mode
# @see ProposalSpaceSettings

# Whether the settings enforce deletion of a given type of partitions
#
# @see #windows_delete_mode
# @see #linux_delete_mode
# @see #other_delete_mode
#
# @param type [#to_s] :linux, :windows or :other
# @return [Boolean]
def delete_forced(type)
send(:"#{type}_delete_mode") == :all
end
# @!attribute linux_delete_mode
# @see ProposalSpaceSettings

alias_method :delete_forced?, :delete_forced
# @!attribute other_delete_mode
# @see ProposalSpaceSettings

# @!attribute resize_windows
# @see ProposalSpaceSettings

# @!attribute delete_resize_configurable
# @see ProposalSpaceSettings

# @!method delete_forbidden
# @see ProposalSpaceSettings

# @!method delete_forbidden?
# @see ProposalSpaceSettings

# @!method delete_forced
# @see ProposalSpaceSettings

# @!method delete_forced?
# @see ProposalSpaceSettings

def windows_delete_mode=(mode)
@windows_delete_mode = validated_delete_mode(mode)
space_settings.windows_delete_mode = validated_delete_mode(mode)
end

def linux_delete_mode=(mode)
@linux_delete_mode = validated_delete_mode(mode)
space_settings.linux_delete_mode = validated_delete_mode(mode)
end

def other_delete_mode=(mode)
@other_delete_mode = validated_delete_mode(mode)
space_settings.other_delete_mode = validated_delete_mode(mode)
end

def lvm_vg_strategy=(strategy)
Expand Down Expand Up @@ -388,11 +370,6 @@ def root_volume
volumes.find(&:root?)
end

# List of possible delete strategies.
# TODO: enum?
DELETE_MODES = [:none, :all, :ondemand]
private_constant :DELETE_MODES

# List of possible VG strategies.
# TODO: enum?
LVM_VG_STRATEGIES = [:use_available, :use_needed, :use_vg_size]
Expand Down Expand Up @@ -444,7 +421,7 @@ def load_features
end

def validated_delete_mode(mode)
validated_feature_value(mode, DELETE_MODES)
validated_feature_value(mode, ProposalSpaceSettings.delete_modes)
end

def validated_lvm_vg_strategy(strategy)
Expand Down
100 changes: 100 additions & 0 deletions src/lib/y2storage/proposal_space_settings.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
# 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 "yast"
require "y2storage/equal_by_instance_variables"

module Y2Storage
# Class to encapsulate all the GuidedProposal settings related to the process of making space
# to allocate the new operating system
class ProposalSpaceSettings
include EqualByInstanceVariables
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just as note, we have the Equatable mixin for custom comparison: https://github.com/yast/yast-yast2/blob/master/library/general/src/lib/yast2/equatable.rb

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, EqualByInstanceVariables is used for ProposalSettings. And this class is basically a subset of those settings extracted to a separate object for clarity. So it makes sense to be consistent with the container class and use the same mechanism.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, only mentioning to promote it for other cases :)


# @see .delete_modes
# TODO: enum?
DELETE_MODES = [:none, :all, :ondemand].freeze
private_constant :DELETE_MODES

# @return [Array<String>] list of possible delete strategies
def self.delete_modes
DELETE_MODES
end

# @return [Boolean] whether to resize Windows systems if needed
attr_accessor :resize_windows

# What to do regarding removal of existing partitions hosting a Windows system.
#
# Options:
#
# * :none Never delete a Windows partition.
# * :ondemand Delete Windows partitions as needed by the proposal.
# * :all Delete all Windows partitions, even if not needed.
#
# @raise ArgumentError if any other value is assigned
#
# @return [Symbol]
attr_accessor :windows_delete_mode

# @return [Symbol] what to do regarding removal of existing Linux
# partitions. See {DiskAnalyzer} for the definition of "Linux partitions".
# @see #windows_delete_mode for the possible values and exceptions
attr_accessor :linux_delete_mode

# @return [Symbol] what to do regarding removal of existing partitions that
# don't fit in #windows_delete_mode or #linux_delete_mode.
# @see #windows_delete_mode for the possible values and exceptions
attr_accessor :other_delete_mode

# Whether the delete mode of the partitions and the resize option for windows can be
# configured. When this option is set to `false`, the {#windows_delete_mode}, {#linux_delete_mode},
# {#other_delete_mode} and {#resize_windows} options cannot be modified by the user.
#
# @return [Boolean]
attr_accessor :delete_resize_configurable

# Whether the settings disable deletion of a given type of partitions
#
# @see #windows_delete_mode
# @see #linux_delete_mode
# @see #other_delete_mode
#
# @param type [#to_s] :linux, :windows or :other
# @return [Boolean]
def delete_forbidden(type)
send(:"#{type}_delete_mode") == :none
end

alias_method :delete_forbidden?, :delete_forbidden

# Whether the settings enforce deletion of a given type of partitions
#
# @see #windows_delete_mode
# @see #linux_delete_mode
# @see #other_delete_mode
#
# @param type [#to_s] :linux, :windows or :other
# @return [Boolean]
def delete_forced(type)
send(:"#{type}_delete_mode") == :all
end

alias_method :delete_forced?, :delete_forced
end
end