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. 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? [...] Best regards, Maxim Levitsky