Re: [PATCH v3 00/14] LUKS: encryption slot management using amend interface

["Daniel P. Berrangé" <berrange@redhat.com>]

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 :|