On Mon, 2020-05-04 at 11:19 +0100, Daniel P. Berrangé wrote:
> 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
All right then, I also agree kind of that these iterations aren't
that nice. I just wanted to do this in the same way the spec defines it,
which is currently kind of independent of how key add/remove operation.
I'll send updated code soon.
Best regards,
Maxim Levitsky
