Re: [Qemu-devel] [PATCH 08/10] block/crypto: implement blockdev-amend
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 > > --- > > block/crypto.c | 86 +--- > > qapi/block-core.json | 4 +-- > > 2 files changed, 68 insertions(+), 22 deletions(-) > > Reviewed-by: Daniel P. Berrangé > > > > 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
Re: [Qemu-devel] [PATCH 08/10] block/crypto: implement blockdev-amend
On Fri, Aug 30, 2019 at 11:56:06PM +0300, Maxim Levitsky wrote: > Signed-off-by: Maxim Levitsky > --- > block/crypto.c | 86 +--- > qapi/block-core.json | 4 +-- > 2 files changed, 68 insertions(+), 22 deletions(-) Reviewed-by: Daniel P. Berrangé > 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 '=' > > 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" > 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 > '*preallocation': 'PreallocMode' } } > > ## > -- > 2.17.2 > 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 :|
[Qemu-devel] [PATCH 08/10] block/crypto: implement blockdev-amend
Signed-off-by: Maxim Levitsky --- block/crypto.c | 86 +--- qapi/block-core.json | 4 +-- 2 files changed, 68 insertions(+), 22 deletions(-) diff --git a/block/crypto.c b/block/crypto.c index dbd95a99ba..9cb668ff0e 100644 --- a/block/crypto.c +++ b/block/crypto.c @@ -534,6 +534,17 @@ block_crypto_co_create_luks(BlockdevCreateOptions *create_options, Error **errp) assert(create_options->driver == BLOCKDEV_DRIVER_LUKS); luks_opts = &create_options->u.luks; +if (!luks_opts->has_size) { +error_setg(errp, "'size' is manadatory for image creation"); +return -EINVAL; +} + +if (!luks_opts->has_file) { +error_setg(errp, "'file' is manadatory for image creation"); +return -EINVAL; +} + + bs = bdrv_open_blockdev_ref(luks_opts->file, errp); if (bs == NULL) { return -EIO; @@ -667,6 +678,39 @@ block_crypto_get_specific_info_luks(BlockDriverState *bs, Error **errp) } +static int +block_crypto_amend_options_generic(BlockDriverState *bs, + QCryptoBlockCreateOptions *amend_options, + bool force, + Error **errp) +{ +BlockCrypto *crypto = bs->opaque; +int ret = -1; + +assert(crypto); +assert(crypto->block); + +/* apply for exclusive write permissions to the underlying file*/ +crypto->updating_keys = true; +ret = bdrv_child_refresh_perms(bs, bs->file, errp); +if (ret) { +goto cleanup; +} + +ret = qcrypto_block_amend_options(crypto->block, + block_crypto_read_func, + block_crypto_write_func, + bs, + amend_options, + force, + errp); +cleanup: +/* release exclusive write permissions to the underlying file*/ +crypto->updating_keys = false; +bdrv_child_refresh_perms(bs, bs->file, errp); +return ret; +} + 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; 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: 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':