Skip to content

Commit

Permalink
Improvements from review
Browse files Browse the repository at this point in the history
  • Loading branch information
joseivanlopez committed Aug 28, 2023
1 parent d0b9db3 commit 67d9a72
Show file tree
Hide file tree
Showing 15 changed files with 60 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@
TargetDevice s
TargetVG s
FsType s
MinSize t
MaxSize t (optinal, max size is considered as unlimited if omitted)
MinSize t (bytes)
MaxSize t (bytes. Optinal, max size is considered as unlimited if omitted)
AutoSize b
Snapshots b
Outline a{sv}
Expand Down
4 changes: 2 additions & 2 deletions doc/dbus/org.opensuse.Agama.Storage1.Proposal.doc.xml
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
TargetDevice s
TargetVG s
FsType s
MinSize t
MaxSize t (optinal, max size is considered as unlimited if omitted)
MinSize t (bytes)
MaxSize t (bytes. Optinal, max size is considered as unlimited if omitted)
AutoSize b
Snapshots b
Outline a{sv}
Expand Down
4 changes: 2 additions & 2 deletions service/etc/agama.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ ALP-Dolomite:
- package: device-mapper # Apparently needed if devices at /dev/mapper are used at boot (eg. FDE)
- package: fde-tools # Needed for FDE with TPM, hardcoded here temporarily
archs: aarch64, x86_64
- package: libtss2-tcti-device0 # Same than fde-tools
- package: libtss2-tcti-device0
- package: ppc64-diag # Needed for hardware-based installations
archs: ppc64
optional_packages: null
Expand Down Expand Up @@ -255,7 +255,7 @@ Leap16:
- package: device-mapper # Apparently needed if devices at /dev/mapper are used at boot (eg. FDE)
- package: fde-tools # Needed for FDE with TPM, hardcoded here temporarily
archs: aarch64, x86_64
- package: libtss2-tcti-device0 # Same than fde-tools
- package: libtss2-tcti-device0
optional_packages: null
base_product: Leap16

Expand Down
8 changes: 6 additions & 2 deletions service/lib/agama/dbus/storage/manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,13 @@ def default_volume(mount_path)
# @param dbus_settings [Hash]
# @return [Integer] 0 success; 1 error
def calculate_proposal(dbus_settings)
logger.info("Calculating storage proposal from D-Bus settings: #{dbus_settings}")

settings = ProposalSettingsConversion.from_dbus(dbus_settings, config: config)
logger.info(
"Calculating storage proposal from D-Bus.\n " \
"D-Bus settings: #{dbus_settings}\n" \
"Agama settings: #{settings}"
)

success = proposal.calculate(settings)

success ? 0 : 1
Expand Down
2 changes: 1 addition & 1 deletion service/lib/agama/dbus/storage/proposal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def initialize(backend, logger)

# Device used as boot device by the storage proposal
#
# @return [String]
# @return [String] Empty string if no device has been selected yet.
def boot_device
dbus_settings.fetch("BootDevice", "")
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def initialize(settings)
def convert # rubocop:disable Metrics/AbcSize
{
"BootDevice" => settings.boot_device.to_s,
"LVM" => settings.lvm.enabled,
"LVM" => settings.lvm.enabled?,
"SystemVGDevices" => settings.lvm.system_vg_devices,
"EncryptionPassword" => settings.encryption.password.to_s,
"EncryptionMethod" => settings.encryption.method.id.to_s,
Expand Down
8 changes: 5 additions & 3 deletions service/lib/agama/dbus/storage/volume_conversion/from_dbus.rb
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,10 @@ def target_vg_conversion(target, value)
# @param target [Agama::Storage::Volume]
# @param value [String]
def fs_type_conversion(target, value)
downcase_value = value.downcase

fs_type = target.outline.filesystems.find do |type|
type.to_human_string.downcase == value.downcase
type.to_human_string.downcase == downcase_value
end

return unless fs_type
Expand All @@ -114,13 +116,13 @@ def fs_type_conversion(target, value)
end

# @param target [Agama::Storage::Volume]
# @param value [Integer]
# @param value [Integer] Size in bytes.
def min_size_conversion(target, value)
target.min_size = Y2Storage::DiskSize.new(value)
end

# @param target [Agama::Storage::Volume]
# @param value [Integer]
# @param value [Integer] Size in bytes.
def max_size_conversion(target, value)
target.max_size = Y2Storage::DiskSize.new(value)
end
Expand Down
4 changes: 3 additions & 1 deletion service/lib/agama/storage/encryption_settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ class EncryptionSettings
# @return [Y2Storage::EncryptionMethod::Base]
attr_accessor :method

# @return [Y2Storage::PbkdFunction, nil]
# PBKD function to use for LUKS2
#
# @return [Y2Storage::PbkdFunction, nil] Can be nil if using LUKS1.
attr_accessor :pbkd_function

def initialize
Expand Down
6 changes: 1 addition & 5 deletions service/lib/agama/storage/lvm_settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ class LvmSettings
#
# @return [Boolean]
attr_accessor :enabled
alias_method :enabled?, :enabled

# Devices to use for the system LVM volume group
#
Expand All @@ -37,11 +38,6 @@ def initialize
@enabled = false
@system_vg_devices = []
end

# @return [Boolean]
def enabled?
!!@enabled
end
end
end
end
3 changes: 2 additions & 1 deletion service/lib/agama/storage/proposal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ def calculate(settings)
# Note that this settings might differ from the {#original_settings}. For example, the sizes
# of some volumes could be adjusted if auto size is set.
#
# @return [ProposalSettings, nil]
# @return [ProposalSettings, nil] nil if no proposal has been calculated yet or the proposal
# was invalidated.
def settings
return nil unless calculated?

Expand Down
2 changes: 1 addition & 1 deletion service/lib/agama/storage/proposal_settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class ProposalSettings
# Device name of the disk that will be used for booting the system and also to allocate all
# the partitions, except those that have been explicitly assigned to other disk(s).
#
# @return [String, nil]
# @return [String, nil] nil if no device has been selected yet.
attr_accessor :boot_device

# Set of volumes to create
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,13 +136,13 @@ def fallbacks_conversion(target)
end
end

# @param mount_path [String]
# @param mount_path [String, nil] nil if not found
def find_min_size_fallback(mount_path)
volume = settings.volumes.find { |v| v.min_size_fallback_for.include?(mount_path) }
volume&.mount_path
end

# @param mount_path [String]
# @param mount_path [String, nil] nil if not found
def find_max_size_fallback(mount_path)
volume = settings.volumes.find { |v| v.max_size_fallback_for.include?(mount_path) }
volume&.mount_path
Expand All @@ -167,7 +167,7 @@ def all_devices
# @return [String, Array<String>]
def device_or_partitions(device)
partitions = partitions(device)
partitions.any? ? partitions : device
partitions.empty? ? device : partitions
end

# @param device [String]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,15 @@ def convert

# @param target [Agama::Storage::Volume]
def sizes_conversion(target)
target.auto_size = !(spec.ignore_fallback_sizes? && spec.ignore_snapshots_sizes?)
target.auto_size = !spec.ignore_fallback_sizes? || !spec.ignore_snapshots_sizes?

# The volume specification contains the min and max sizes for the volume. But the final
# range of sizes used by the Y2Storage proposal depends on the fallback sizes (if this
# volume is fallback for other volume) and the size for snapshots (if snapshots is
# active). The planned device contains the real range of sizes used by the proposal.
#
# From Agama point of view, this is the way of recovering the range of sizes used by
# Y2Storage when a volume is set to have auto size.
planned = planned_device_for(spec.mount_point)
target.min_size = planned&.min || spec.min_size
target.max_size = planned&.max || spec.max_size
Expand Down
5 changes: 5 additions & 0 deletions service/lib/agama/storage/volume_conversion/to_y2storage.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ def sizes_conversion(target)
target.ignore_fallback_sizes = !auto
target.ignore_snapshots_sizes = !auto

# The range of sizes is defined by the volume outline in case of auto size (mix and max
# sizes cannot be configured if auto size is set).
# And note that the inal range of sizes used by the Y2Storage proposal is calculated by
# Y2Storage according the range configured here and other sizes like fallback sizes or
# the size for snapshots.
target.min_size = auto ? volume.outline.base_min_size : volume.min_size
target.max_size = auto ? volume.outline.base_max_size : volume.max_size
end
Expand Down
24 changes: 18 additions & 6 deletions service/lib/agama/storage/volume_outline.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,13 @@ class VolumeOutline
attr_accessor :filesystems

# Base value to calculate the min size for the volume (if #auto_size is set to true
# for that final volume) or to use as default value (if #auto_size is false)
# for that final volume)
#
# @return [Y2Storage::DiskSize]
attr_accessor :base_min_size

# Base value to calculate the max size for the volume (if #auto_size is set to true
# for that final volume) or to use as default value (if #auto_size is false)
# for that final volume)
#
# @return [Y2Storage::DiskSize]
attr_accessor :base_max_size
Expand All @@ -56,10 +56,22 @@ class VolumeOutline
attr_accessor :adjust_by_ram
alias_method :adjust_by_ram?, :adjust_by_ram

# @return [Array<String>] mount paths of other volumes
# Lists the mount paths of the volumes for which this volume is a min size fallback.
#
# Being a min size fallback means that the min size of the volume would be increased by the
# min size of other volumes, if any of that other volumes is not used for the proposal.
#
# For example, let's say the root volume is a fallback for the min size of /home and /var
# (root.min_size_fallback_for => ["/home", "/var"]). And a proposal is calculated with only
# root and /home. In that case, the min size of /var is added to the min size of root. The
# same would happen for /home if the proposal does not include it.
#
# @return [Array<String>]
attr_accessor :min_size_fallback_for

# @return [Array<String>] mount paths of other volumes
# The same as {#min_size_fallback_for}, but for the max size of the volume.
#
# @return [Array<String>]
attr_accessor :max_size_fallback_for

# Whether snapshots option can be configured
Expand All @@ -70,12 +82,12 @@ class VolumeOutline

# Size required for snapshots
#
# @return [Y2Storage::DiskSize, nil]
# @return [Y2Storage::DiskSize, nil] nil if no extra size for snapshots.
attr_accessor :snapshots_size

# Percentage of space required for snapshots
#
# @return [Integer, nil]
# @return [Integer, nil] nil if no extra size for snapshots.
attr_accessor :snapshots_percentage

def initialize
Expand Down

0 comments on commit 67d9a72

Please sign in to comment.