Message ID | 20190830205608.18192-9-mlevitsk@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | RFC crypto/luks: encryption key managment using amend interface | expand |
On Fri, Aug 30, 2019 at 11:56:06PM +0300, Maxim Levitsky wrote: > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > --- > block/crypto.c | 86 +++++++++++++++++++++++++++++++++----------- > qapi/block-core.json | 4 +-- > 2 files changed, 68 insertions(+), 22 deletions(-) Reviewed-by: Daniel P. Berrangé <berrange@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 '=' > > 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
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 <mlevitsk@redhat.com> > > --- > > block/crypto.c | 86 +++++++++++++++++++++++++++++++++----------- > > qapi/block-core.json | 4 +-- > > 2 files changed, 68 insertions(+), 22 deletions(-) > > Reviewed-by: Daniel P. Berrangé <berrange@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
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': 'BlockdevRef', - 'size': 'size', + 'data': { '*file': 'BlockdevRef', + '*size': 'size', '*preallocation': 'PreallocMode' } } ##
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> --- block/crypto.c | 86 +++++++++++++++++++++++++++++++++----------- qapi/block-core.json | 4 +-- 2 files changed, 68 insertions(+), 22 deletions(-)