Add swap/mount backend supports where needed and correct minor repeating spelling issue #9
Conversation
lczerner
left a comment
lczerner
left a comment
There was a problem hiding this comment.
Clearly Jan completely forgot to get the mounts data and it needs to be fixed, so thank you for that.
Could you also add some tests for this ? Unittests at least since bashtests seems to be completely broken and useless for the mutlipath unfortunatelly.
|
I am about to add more comments and review rest of the changes. I am sorry but I am not used to work with github web UI, I usually review email patches only so this is something new and I am trying to give it a chance :) -Lukas |
Just as volumes can be mounted, they can also be used as swap volumes. Add support for swap reporting to each available backend. Signed-off-by: John Pittman <jpittman@redhat.com>
|
Thanks again Lukas. I've updated the current commits with your requested changes. I'll add an additional commit with the requested test additions. Will let you know once I'm done. |
Currently the multipath backend is not configured to show the mount point. Add that support. Signed-off-by: John Pittman <jpittman@redhat.com>
In several places additional is incorrectly spelled "aditional". Correct these misspellings. Signed-off-by: John Pittman <jpittman@redhat.com>
Add test verifying mount check routine within get_volume_data() method from multipath backend. Also add real_dev listing to mount_data as it is needed in get_volume_data() check and fix a newline error. Signed-off-by: John Pittman <jpittman@redhat.com>
Add a basic volume swap test to the two available unittests, lvm and multipath. Signed-off-by: John Pittman <jpittman@redhat.com>
The check for type was hard coded to LUKS1. Add support for LUKS2 by adding a helper function to grab the version from luksDump and implement the function across the individual commands. Signed-off-by: John Pittman <jpittman@redhat.com>
Add test for crypt volume initialized as swap. Also add the swap commands to the dependency check in test.py. Signed-off-by: John Pittman <jpittman@redhat.com>
Since we're not sure when the multipath devices are fully created along with sub-paths, we need to ensure that udev is settled prior to exiting mpath_setup(). Otherwise the rest of the script can become racy. Also add udevadm to binary check as we need this command. Signed-off-by: John Pittman <jpittman@redhat.com>
If the multipath bashtests are run on a system that already has multipath devices present, the tests will grab the wrong devices. Ensure the tests only look at the multipath device we have created. Also, as the multipath tests require targetcli and iscsiadm, add those to the system dependencies list and fix a newline error as well. Signed-off-by: John Pittman <jpittman@redhat.com>
lczerner
left a comment
lczerner
left a comment
There was a problem hiding this comment.
Thanks for the work, it looks good. When the small problem I commented on in the first commit is fixed I'll merge it.
Thanks!
-Lukas
| else: | ||
| for swap in self.swaps: | ||
| if swap[0] == dm['real_dev']: | ||
| dm['mount'] = "SWAP" |
There was a problem hiding this comment.
There needs to be a break after the assignment. There is no need to iterate over the rest of the entries in swap. This applies to all places in this patch except the one in DeviceInfo of course.
There was a problem hiding this comment.
Thanks Lukas and sorry for the wait. Good catch. I've made the requested correction. Please feel free to review at your convenience.
As currently written, when attempting to match swaps, after the match, the loop will continue to iterate. This is unneeded. Correct by breaking from loop once a match is found. Signed-off-by: John Pittman <jpittman@redhat.com>
2 participants
These commits add swap printing support to all available backends, add mount print support to the multipath backend, and correct a repeating typo in the code.