On Sun, May 03, 2020 at 09:43:10PM +0300, Maxim Levitsky wrote: > Hi! > Here is the updated series of my patches, incorporating all the feedback I > received. > > This implements the API interface that we agreed upon except that I merged the > LUKSKeyslotActive/LUKSKeyslotInactive union into a struct because otherwise > I need nested unions which are not supported currently by QAPI parser. > This didn't change the API and thus once support for nested unions is there, > it can always be implemented in backward compatible way. > > I hope that this series will finally be considered for merging, since I am > somewhat running > out of time to finish this task. > > Patches are strictly divided by topic to 3 groups, and each group depends on > former groups. > > * Patches 1,2 implement qcrypto generic amend interface, including definition > of structs used in crypto.json and implement this in luks crypto driver > Nothing is exposed to the user at this stage > > * Patches 3-9 use the code from patches 1,2 to implement qemu-img amend based > encryption slot management > for luks and for qcow2, and add a bunch of iotests to cover that. > > * Patches 10-13 add x-blockdev-amend (I'll drop the -x prefix if you like), > and wire it > to luks and qcow2 driver to implement qmp based encryption slot management > also using > the code from patches 1,2, and also add a bunch of iotests to cover this. > > Tested with -raw,-qcow2 and -luks iotests and 'make check' > > V3: rebased, addressed most of the review feedback. > For now I kept the slot bitmap code since I am not sure that replacing it > will be better.
I'm still of the opinion that the bitmaps code should be replaced. With this current design we are iterating over the slot of keys slots 4 times, doing different checks/logic in each iteration. This is really not nice for understanding how the code works. IMHO we should be iterating at most twice - once to validate the requested configuration, and once to apply the requested configuration. Even if there si duplication of logic in between the delete/add key codepaths, I think it will be better as the logic for each operation will be in one place. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|