ceph-disk: fix luks dmcrypt mapping during activation #5519
Conversation
|
Ping, any thoughts on this patch set? These changes address some pretty serious bugs in master ceph-disk (for activate --dmcrypt). |
ghost
self-assigned this
|
Looks like I now need to rebase here. |
2943194 added a call to dmcrypt_map() during disk activation. The change is not suitable for use alongside the recently added dmcrypt LUKS support, because: - The callers don't correctly provide cryptsetup_parameters or luks arguments. - dmcrypt_map() calls LuksFormat, which should never be performed during disk activation. - The key file paths don't carry the luks suffix when required. This commit addresses these issues. Corresponding tests and a udev file update will follow. Signed-off-by: David Disseldorp <ddiss@suse.de>
Signed-off-by: David Disseldorp <ddiss@suse.de>
Test plain and luks based disk activation. Signed-off-by: David Disseldorp <ddiss@suse.de>
ddiss
force-pushed
the
ceph_disk_activate_fix_luks_dmcrypt_map
branch
from
a8f82f4
to
6b40043
Compare
|
Rebased. It was just the restorecon change that was conflicting (fix is now upstream via 3aab146). Please pull. |
|
@ddiss Looks great, sorry for the lag. Now that infernalis was forked this should be against the infernalis branch. It looks good to me. Could you please confirm that you ran the tests and they succeeded ? |
|
Thanks for the feedback Loïc. The output of the tests before and after the fixes can be found at: http://paste.opensuse.org/0af11aa2 (without fixes) Regarding the infernalis merge: can I wait for the changes to hit master, then cherry-pick -x and submit, or should I raise a bug to track the backport? [edit: with/without links were around the wrong way] |
|
master + backporting to infernalis with cherry-pick -x would be what we do after infernalis is released. Right now infernalis is the branch where all bug fixes such as this one should go. And infernalis is merged into master on a regular (daily or so) basis. That's why I'm suggesting you target infernalis instead of master. |
|
Okay, will resubmit against the infernalis - thanks. |
|
@ddiss it would also be great if you could create an issue in http://tracker.ceph.com/ to track this and include a Fixes: #XXX where appropriate. |
|
@ddiss note that the method for testing ceph-disk in infernalis changed significantly. I'm willing to translate your pull request to use them so you don't have to go into the trouble of re-writing the tests you already wrote. |
This patchset fixes some issues with the recently merged ceph-disk activate --dmcrypt support, and additionally adds unit test coverage.