-
Notifications
You must be signed in to change notification settings - Fork 43
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
Automatic generation of physical volumes #1655
Conversation
a333c2c
to
7ffd031
Compare
Pull Request Test Coverage Report for Build 11252648625Details
💛 - Coveralls |
3760f08
to
9bbe14a
Compare
def disks_for_clean | ||
return drives_names if config.boot_device.nil? || drives_names.include?(config.boot_device) | ||
|
||
drives_names + [config.boot_device] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NP: maybe more direct with [drive_names, config.boot_device].flatten.compact.uniq
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed into something similar to your suggestion
|
||
planned.pvs_encryption_method = enc.method | ||
planned.pvs_encryption_password = enc.password | ||
planned.pvs_encryption_pbkdf = enc.pbkd_function |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are the rest of encryption attributes ignored (#label
, #cipher
and #key_size
)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure whether label makes sense. We are going to generate several PVs and labels should be unique.
For the other two, I overlooked them because PlannedVg
doesn't currently support them. So it would need changes both here and at yast2-storage-ng. What about making a separate card/PBI out of it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added as another point to https://trello.com/c/XAaoYdCL/578-storage-profile-improve-support-for-generating-lvm-physical-volumes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
#1652 adds support for configuring the automatic generation of LVM physical volumes. Read there the details about how the feature works.
This pull request implements a first version of the functionality described there, including a couple of unit tests.
Depends on yast/yast-storage-ng#1392