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

Handling of YAST_REUSE_LVM linuxrc option #1365

Merged
merged 11 commits into from
Feb 15, 2024
2 changes: 0 additions & 2 deletions src/lib/y2storage/proposal/lvm_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,6 @@ def new_volume_group
# @return [Boolean]
def try_to_reuse?
# Setting introduced to completely avoid LVM reusing in D-Installer.
# It's a new playground, so no need to carry YaST behaviors that have
# proven to be confusing.
return false if settings.lvm_vg_reuse == false

# We introduced this when we still didn't have a mechanism to activate
Expand Down
10 changes: 10 additions & 0 deletions src/lib/y2storage/proposal_settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
require "y2storage/encryption_method"
require "y2storage/equal_by_instance_variables"
require "y2storage/proposal_space_settings"
require "y2storage/storage_env"

module Y2Storage
# Class to manage settings used by the proposal (typically read from control.xml)
Expand Down Expand Up @@ -236,6 +237,7 @@ def self.new_for_current_product
def for_current_product
apply_defaults
load_features
apply_user_enforced
end

# Produces a deep copy of settings
Expand Down Expand Up @@ -401,6 +403,14 @@ def apply_defaults
end
end

# Some values can be explicitly enforced by user
# This setting should have precendence over everything else
def apply_user_enforced
value = StorageEnv.instance.requested_lvm_reuse

send(:lvm_vg_reuse=, StorageEnv.instance.requested_lvm_reuse) if !value.nil?
end

# Overrides the settings with values read from the YaST product features
# (i.e. values in /control.xml).
#
Expand Down
27 changes: 24 additions & 3 deletions src/lib/y2storage/storage_env.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,11 @@ class StorageEnv

ENV_LIBSTORAGE_IGNORE_PROBE_ERRORS = "LIBSTORAGE_IGNORE_PROBE_ERRORS".freeze

ENV_REUSE_LVM = "YAST_REUSE_LVM".freeze

private_constant :ENV_MULTIPATH, :ENV_BIOS_RAID, :ENV_ACTIVATE_LUKS, :ENV_LUKS2_AVAILABLE
private_constant :ENV_LIBSTORAGE_IGNORE_PROBE_ERRORS
private_constant :ENV_REUSE_LVM

def initialize
reset_cache
Expand Down Expand Up @@ -89,6 +92,19 @@ def luks2_available?
active?(ENV_LUKS2_AVAILABLE, default: false)
end

# Whether YaST should reuse existing LVM
#
# see jsc#PED-6407 or jsc#IBM-1315
#
# @return [Boolean, nil] boolean as explicitly set by user, nil if user set nothing
def requested_lvm_reuse
value = read(ENV_REUSE_LVM)

return env_str_to_bool(value) if value

nil
Copy link
Contributor

@ancorgs ancorgs Feb 9, 2024

Choose a reason for hiding this comment

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

Nitpicking comment: I think this would do the same and it's nicer to my Ruby eyes

def requested_lvm_reuse
  value = read(ENV_REUSE_LVM)
  return unless value

  env_str_to_bool(value)
end

How could you easily check if both implementations are equivalent? Well, I you would have unit tests... 😉

end

# Whether errors during libstorage probing should be ignored.
#
# See bsc#1177332:
Expand All @@ -106,6 +122,13 @@ def ignore_probe_errors?

private

# Takes a string and translates it to bool in a similar way how linuxrc does
def env_str_to_bool(value)
# Similar to what linuxrc does, also consider the flag activated if the
# variable is used with no value or with "1"
value.casecmp?("on") || value.empty? || value == "1"
end

# Whether the env variable is active
#
# @param variable [String]
Expand All @@ -116,9 +139,7 @@ def active?(variable, default: false)

value = read(variable)
result = if value
# Similar to what linuxrc does, also consider the flag activated if the
# variable is used with no value or with "1"
value.casecmp?("on") || value.empty? || value == "1"
env_str_to_bool(value)
else
default
end
Expand Down
Loading