Partitioner: button to activate crypt devices #849
Conversation
ancorgs
force-pushed
the
provide_password
branch
3 times, most recently
from
3b09276
to
f8f1596
Compare
| # | ||
| # @return [Boolean, nil] | ||
| def activate | ||
| id == :crypt ? true : nil |
There was a problem hiding this comment.
to be honest i start thinking that here it is a bit abusing generic class where it more fits by common class hierarchy. I see there many "child" specific actions and this is one of it ( others are e.g. label defined elsewhere, warning text and others). why not create child class and have factory that create child based on id? If there is reason please document it in Action class description. Because as it is now, reuse single or subset of actions elsewhere is problematic.
There was a problem hiding this comment.
I don't think there is a reason to avoid a hierarchy other than keeping things simple. These "actions" are not supposed to be reused elsewhere, they are just some very simplistic objects to represent the options of the "Configure" menu-button. They will never be used out of the scope of such button.
Anyway, I will take a look to the possibility of introducing a hierarchy and a factory class.
There was a problem hiding this comment.
@jreidinger I added a commit which changes this to use a hierarchy of actions. I don't think the factory class helps in this case, so I just reorganized the code a bit to use the new classes directly.
Now is more correct from the OOP point of view, but to be honest I'm not sure if the code has become more readable or simply bigger. :-)
There was a problem hiding this comment.
@ancorgs for me it looks much better at least from my POV. and I think it makes also easier to extend it in future as it means add it only to two places and not more ( previously also warning texts and possible activate ). Only potentially movable stuff is also label, but nothing critical, so LGTM.
|
Remember changelog, etc. |
ancorgs
force-pushed
the
provide_password
branch
2 times, most recently
from
61739ec
to
48f1f30
Compare
ancorgs
force-pushed
the
provide_password
branch
from
48f1f30
to
c1d6033
Compare
|
✔️ Internal Jenkins job #118 successfully finished |
|
✔️ Public Jenkins job #116 successfully finished |
Problem
When executed on an already installed system (i.e. not in installation), the Partitioner of storage-ng does not contain any way to activate pre-existing but inactive LUKs device.
The old partitioner (pre-ng) didn't executed an activation by default either, but it offered two ways of activating such device so it could be mounted or used in any other way.
The user can click on "Edit" for an encrypted partition and then, in the first screen of the Edit wizard, select to mount the existing file-system. In that case, the next step of the Edit wizard consists on asking the password and activating the existing LUKS.
There is also a general option "Provide Crypt Passwords" that is available in "Configure..." just alongside the similar "Configure Multipath" also used to activate inactive devices (Multipath devices instead of LUKS ones).
See more in the following links
Solution
Due to fundamental differences in how activation of devices works in storage-ng, solution 1 cannot be re-implemented.
This pull request re-implements solution 2. But due to the mentioned differences in approach, the "Provide Crypt Passwords" button added in this pull request:
Testing
Screenshots
None of the options of "Configure" and "Rescan Devices" buttons has never been explained in the help (not even in the old storage). Let's fix it.