On Mon, Mar 12, 2018 at 04:02:14PM +0100, Kevin Wolf wrote: > Everything that refers to the protocol layer or QemuOpts is moved out of > block_crypto_create_generic(), so that the remaining function is > suitable to be called by a .bdrv_co_create implementation. > > LUKS is the only driver that actually implements the old interface, and > we don't intend to use it in any new drivers, so put the moved out code > directly into a LUKS function rather than creating a generic > intermediate one. > > Signed-off-by: Kevin Wolf <kw...@redhat.com> > Reviewed-by: Daniel P. Berrangé <berra...@redhat.com> > Reviewed-by: Eric Blake <ebl...@redhat.com> > --- > block/crypto.c | 95 > +++++++++++++++++++++++++++++++++++++--------------------- > 1 file changed, 61 insertions(+), 34 deletions(-)
Reviving a year old commit... The LUKS driver doesn't implement preallocation during create. Before this commit this would be reported $ qemu-img create -f luks --object secret,id=sec0,data=base -o key-secret=sec0 base.luks 1G -o preallocation=full Formatting 'base.luks', fmt=luks size=1073741824 key-secret=sec0 preallocation=full qemu-img: base.luks: Parameter 'preallocation' is unexpected After this commit, there is no error reported - it just silently ignores the preallocation=full option. I'm a bit lost in block layer understanding where is the right place to fix the error reporting in this case. > > diff --git a/block/crypto.c b/block/crypto.c > index 77871640cc..b0a4cb3388 100644 > --- a/block/crypto.c > +++ b/block/crypto.c > @@ -306,43 +306,29 @@ static int block_crypto_open_generic(QCryptoBlockFormat > format, > } > > > -static int block_crypto_create_generic(QCryptoBlockFormat format, > - const char *filename, > - QemuOpts *opts, > - Error **errp) > +static int block_crypto_co_create_generic(BlockDriverState *bs, > + int64_t size, > + QCryptoBlockCreateOptions *opts, > + Error **errp) > { > - int ret = -EINVAL; > - QCryptoBlockCreateOptions *create_opts = NULL; > + int ret; > + BlockBackend *blk; > QCryptoBlock *crypto = NULL; > - struct BlockCryptoCreateData data = { > - .size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0), > - BDRV_SECTOR_SIZE), > - }; > - QDict *cryptoopts; > - > - /* Parse options */ > - cryptoopts = qemu_opts_to_qdict(opts, NULL); > + struct BlockCryptoCreateData data; > > - create_opts = block_crypto_create_opts_init(format, cryptoopts, errp); > - if (!create_opts) { > - return -1; > - } > + blk = blk_new(BLK_PERM_WRITE | BLK_PERM_RESIZE, BLK_PERM_ALL); > > - /* Create protocol layer */ > - ret = bdrv_create_file(filename, opts, errp); > + ret = blk_insert_bs(blk, bs, errp); > if (ret < 0) { > - return ret; > + goto cleanup; > } > > - data.blk = blk_new_open(filename, NULL, NULL, > - BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, > - errp); > - if (!data.blk) { > - return -EINVAL; > - } > + data = (struct BlockCryptoCreateData) { > + .blk = blk, > + .size = size, > + }; > > - /* Create format layer */ > - crypto = qcrypto_block_create(create_opts, NULL, > + crypto = qcrypto_block_create(opts, NULL, > block_crypto_init_func, > block_crypto_write_func, > &data, > @@ -355,10 +341,8 @@ static int > block_crypto_create_generic(QCryptoBlockFormat format, > > ret = 0; > cleanup: > - QDECREF(cryptoopts); > qcrypto_block_free(crypto); > - blk_unref(data.blk); > - qapi_free_QCryptoBlockCreateOptions(create_opts); > + blk_unref(blk); > return ret; > } > > @@ -563,8 +547,51 @@ static int coroutine_fn > block_crypto_co_create_opts_luks(const char *filename, > QemuOpts *opts, > Error **errp) > { > - return block_crypto_create_generic(Q_CRYPTO_BLOCK_FORMAT_LUKS, > - filename, opts, errp); > + QCryptoBlockCreateOptions *create_opts = NULL; > + BlockDriverState *bs = NULL; > + QDict *cryptoopts; > + int64_t size; > + int ret; > + > + /* Parse options */ > + size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0); > + > + cryptoopts = qemu_opts_to_qdict_filtered(opts, NULL, > + &block_crypto_create_opts_luks, > + true); > + > + create_opts = block_crypto_create_opts_init(Q_CRYPTO_BLOCK_FORMAT_LUKS, > + cryptoopts, errp); > + if (!create_opts) { > + ret = -EINVAL; > + goto fail; > + } > + > + /* Create protocol layer */ > + ret = bdrv_create_file(filename, opts, errp); > + if (ret < 0) { > + return ret; > + } > + > + bs = bdrv_open(filename, NULL, NULL, > + BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, errp); > + if (!bs) { > + ret = -EINVAL; > + goto fail; > + } > + > + /* Create format layer */ > + ret = block_crypto_co_create_generic(bs, size, create_opts, errp); > + if (ret < 0) { > + goto fail; > + } > + > + ret = 0; > +fail: > + bdrv_unref(bs); > + qapi_free_QCryptoBlockCreateOptions(create_opts); > + QDECREF(cryptoopts); > + return ret; > } > > static int block_crypto_get_info_luks(BlockDriverState *bs, > -- > 2.13.6 > 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 :|