On Fri, 2019-09-06 at 15:10 +0100, Daniel P. Berrangé wrote: > On Fri, Aug 30, 2019 at 11:56:06PM +0300, Maxim Levitsky wrote: > > Signed-off-by: Maxim Levitsky <mlevi...@redhat.com> > > --- > > block/crypto.c | 86 +++++++++++++++++++++++++++++++++----------- > > qapi/block-core.json | 4 +-- > > 2 files changed, 68 insertions(+), 22 deletions(-) > > Reviewed-by: Daniel P. Berrangé <berra...@redhat.com> > > > > static int > > block_crypto_amend_options(BlockDriverState *bs, > > QemuOpts *opts, > > @@ -678,44 +722,45 @@ block_crypto_amend_options(BlockDriverState *bs, > > BlockCrypto *crypto = bs->opaque; > > QDict *cryptoopts = NULL; > > QCryptoBlockCreateOptions *amend_options = NULL; > > - int ret; > > + int ret= -EINVAL; > > nitpick - space before '=' Done. This is one of the few errors that checkpatch.pl does catch, but apparently I forgot to run it on this patch. > > > > > assert(crypto); > > assert(crypto->block); > > > > - crypto->updating_keys = true; > > - > > - ret = bdrv_child_refresh_perms(bs, bs->file, errp); > > - if (ret) { > > - goto cleanup; > > - } > > - > > cryptoopts = qemu_opts_to_qdict_filtered(opts, NULL, > > > > &block_crypto_create_opts_luks, > > true); > > > > qdict_put_str(cryptoopts, "format", "luks"); > > amend_options = block_crypto_create_opts_init(cryptoopts, errp); > > + > > if (!amend_options) { > > - ret = -EINVAL; > > - goto cleanup; > > + goto out; > > } > > > > - ret = qcrypto_block_amend_options(crypto->block, > > - block_crypto_read_func, > > - block_crypto_write_func, > > - bs, > > - amend_options, > > - force, > > - errp); > > -cleanup: > > - crypto->updating_keys = false; > > - bdrv_child_refresh_perms(bs, bs->file, errp); > > + ret = block_crypto_amend_options_generic(bs, amend_options, force, > > errp); > > +out: > > No need to rename the "cleanup" label to "out" All right. > > > qapi_free_QCryptoBlockCreateOptions(amend_options); > > qobject_unref(cryptoopts); > > return ret; > > } > > > > +static int > > +coroutine_fn block_crypto_co_amend(BlockDriverState *bs, > > + BlockdevCreateOptions *opts, > > + bool force, > > + Error **errp) > > +{ > > + QCryptoBlockCreateOptions amend_opts; > > + > > + amend_opts = (QCryptoBlockCreateOptions) { > > + .format = Q_CRYPTO_BLOCK_FORMAT_LUKS, > > + .u.luks = *qapi_BlockdevCreateOptionsLUKS_base(&opts->u.luks), > > + }; > > + > > + return block_crypto_amend_options_generic(bs, &amend_opts, force, > > errp); > > +} > > + > > > > static void > > block_crypto_child_perms(BlockDriverState *bs, BdrvChild *c, > > @@ -774,6 +819,7 @@ static BlockDriver bdrv_crypto_luks = { > > .bdrv_get_info = block_crypto_get_info_luks, > > .bdrv_get_specific_info = block_crypto_get_specific_info_luks, > > .bdrv_amend_options = block_crypto_amend_options, > > + .bdrv_co_amend = block_crypto_co_amend, > > > > .strong_runtime_opts = block_crypto_strong_runtime_opts, > > }; > > diff --git a/qapi/block-core.json b/qapi/block-core.json > > index 7900914506..02375fb59a 100644 > > --- a/qapi/block-core.json > > +++ b/qapi/block-core.json > > @@ -4220,8 +4220,8 @@ > > ## > > { 'struct': 'BlockdevCreateOptionsLUKS', > > 'base': 'QCryptoBlockCreateOptionsLUKS', > > - 'data': { 'file': 'BlockdevRef', > > - 'size': 'size', > > + 'data': { '*file': 'BlockdevRef', > > + '*size': 'size', > > Docs comment to explain they are mandatory for create Done > > > '*preallocation': 'PreallocMode' } } > > > > ## > > -- > > 2.17.2 > > > > Regards, > Daniel
Best regards, Maxim Levitsky