On Fri, Sep 06, 2019 at 04:57:22PM +0300, Maxim Levitsky wrote:
> On Fri, 2019-09-06 at 14:49 +0100, Daniel P. Berrangé wrote:
> > On Fri, Aug 30, 2019 at 11:56:00PM +0300, Maxim Levitsky wrote:
> > > Now you can specify which slot to put the encryption key to
> > > Plus add 'active' option which will let user erase the key secret
> > > instead of adding it.
> > > Check that it is true for creation
> > >
> > > Signed-off-by: Maxim Levitsky <mlevi...@redhat.com>
> > > ---
> > > block/crypto.c | 2 ++
> > > block/crypto.h | 16 +++++++++++
> > > block/qcow2.c | 2 ++
> > > crypto/block-luks.c | 26 +++++++++++++++---
> > > qapi/crypto.json | 19 ++++++++++++++
> > > tests/qemu-iotests/082.out | 54 ++++++++++++++++++++++++++++++++++++++
> > > 6 files changed, 115 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/block/crypto.c b/block/crypto.c
> > > index 6e822c6e50..a6a3e1f1d8 100644
> > > --- a/block/crypto.c
> > > +++ b/block/crypto.c
> > > @@ -144,6 +144,8 @@ static QemuOptsList block_crypto_create_opts_luks = {
> > > BLOCK_CRYPTO_OPT_DEF_LUKS_IVGEN_HASH_ALG(""),
> > > BLOCK_CRYPTO_OPT_DEF_LUKS_HASH_ALG(""),
> > > BLOCK_CRYPTO_OPT_DEF_LUKS_ITER_TIME(""),
> > > + BLOCK_CRYPTO_OPT_DEF_LUKS_SLOT(""),
> > > + BLOCK_CRYPTO_OPT_DEF_LUKS_ACTIVE(""),
> > > { /* end of list */ }
> > > },
> > > };
> > > diff --git a/block/crypto.h b/block/crypto.h
> > > index b935695e79..05cc43d9bc 100644
> > > --- a/block/crypto.h
> > > +++ b/block/crypto.h
> > > @@ -35,12 +35,14 @@
> > > "ID of the secret that provides the AES encryption key")
> > >
> > > #define BLOCK_CRYPTO_OPT_LUKS_KEY_SECRET "key-secret"
> > > +#define BLOCK_CRYPTO_OPT_LUKS_SLOT "slot"
> > > #define BLOCK_CRYPTO_OPT_LUKS_CIPHER_ALG "cipher-alg"
> > > #define BLOCK_CRYPTO_OPT_LUKS_CIPHER_MODE "cipher-mode"
> > > #define BLOCK_CRYPTO_OPT_LUKS_IVGEN_ALG "ivgen-alg"
> > > #define BLOCK_CRYPTO_OPT_LUKS_IVGEN_HASH_ALG "ivgen-hash-alg"
> > > #define BLOCK_CRYPTO_OPT_LUKS_HASH_ALG "hash-alg"
> > > #define BLOCK_CRYPTO_OPT_LUKS_ITER_TIME "iter-time"
> > > +#define BLOCK_CRYPTO_OPT_LUKS_ACTIVE "active"
> > >
> > > #define BLOCK_CRYPTO_OPT_DEF_LUKS_KEY_SECRET(prefix) \
> > > BLOCK_CRYPTO_OPT_DEF_KEY_SECRET(prefix, \
> > > @@ -88,6 +90,20 @@
> > > .help = "Time to spend in PBKDF in milliseconds", \
> > > }
> > >
> > > +#define BLOCK_CRYPTO_OPT_DEF_LUKS_SLOT(prefix) \
> > > + { \
> > > + .name = prefix BLOCK_CRYPTO_OPT_LUKS_SLOT, \
> > > + .type = QEMU_OPT_NUMBER, \
> > > + .help = "Controls the slot where the secret is added/erased",
> > > \
> > > + }
> > > +
> > > +#define BLOCK_CRYPTO_OPT_DEF_LUKS_ACTIVE(prefix) \
> > > + { \
> > > + .name = prefix BLOCK_CRYPTO_OPT_LUKS_ACTIVE, \
> > > + .type = QEMU_OPT_BOOL, \
> > > + .help = "Controls if the added secret is added or erased", \
> > > + }
> >
> > Do we actually need the "active" property for initial
> > creation. I think its only needed for amend, so perhaps
> > we shuold not register this at all ?
>
> Sadly we kind of do, since both amend and create use the same option list
> currently.
> I tried to duplicate it, and it is possible, but then you end up
> with significant code duplication in qcow2 with its huge create option list.
Ah, I see now.
> I am now thinking that we could have had , 'create only' option list, 'amend
> only' option list,
> and 'common' option list.
> What do you think?
I'm not too fussed - either way is fine with me.
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 :|
