On 2018-03-09 22:46, Kevin Wolf wrote: > This adds the .bdrv_co_create driver callback to vhdx, which > enables image creation over QMP. > > Signed-off-by: Kevin Wolf <kw...@redhat.com> > --- > qapi/block-core.json | 37 ++++++++++- > block/vhdx.c | 174 > ++++++++++++++++++++++++++++++++++++++------------- > 2 files changed, 167 insertions(+), 44 deletions(-) > > diff --git a/qapi/block-core.json b/qapi/block-core.json > index 2eba0eef7e..3a65909c47 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -3699,6 +3699,41 @@ > '*static': 'bool' } } > > ## > +# @BlockdevVhdxSubformat: > +# > +# @dynamic: Growing image file > +# @fixed: Preallocated fixed-size imge file > +# > +# Since: 2.12 > +## > +{ 'enum': 'BlockdevVhdxSubformat', > + 'data': [ 'dynamic', 'fixed' ] } > + > +## > +# @BlockdevCreateOptionsVhdx: > +# > +# Driver specific image creation options for vhdx. > +# > +# @file Node to create the image format on > +# @size Size of the virtual disk in bytes > +# @log-size Log size in bytes (default: 1 MB) > +# @block-size Block size in bytes (default: 1 MB)
Is it? Currently, the default size is actually 0 and you keep that by a simple "block_size = vhdx_opts->block_size;" assignment. But the old help text also states: "0 means auto-calculate based on image size." This is reflected in the code, even after this patch. 0 can mean 8, 16, 32, or 64 MB. > +# @subformat vhdx subformat (default: dynamic) > +# @block-state-zero Force use of payload blocks of type 'ZERO'. Non-standard, > +# but default. Do not set to 'off' when using 'qemu-img > +# convert' with subformat=dynamic. > +# > +# Since: 2.12 > +## > +{ 'struct': 'BlockdevCreateOptionsVhdx', > + 'data': { 'file': 'BlockdevRef', > + 'size': 'size', > + '*log-size': 'size', > + '*block-size': 'size', > + '*subformat': 'BlockdevVhdxSubformat', > + '*block-state-zero': 'bool' } } > + > +## > # @BlockdevCreateNotSupported: > # > # This is used for all drivers that don't support creating images. [...] > index d82350d07c..0ce972381f 100644 > --- a/block/vhdx.c > +++ b/block/vhdx.c [...] > @@ -1792,54 +1797,63 @@ exit: > * .---- ~ ----------- ~ ------------ ~ ---------------- ~ -----------. > * 1MB > */ > -static int coroutine_fn vhdx_co_create_opts(const char *filename, QemuOpts > *opts, > - Error **errp) > +static int coroutine_fn vhdx_co_create(BlockdevCreateOptions *opts, > + Error **errp) > { [...] > > - if (!strcmp(type, "dynamic")) { > + switch (vhdx_opts->subformat) { > + case BLOCKDEV_VHDX_SUBFORMAT_DYNAMIC: > image_type = VHDX_TYPE_DYNAMIC; > - } else if (!strcmp(type, "fixed")) { > + break; > + case BLOCKDEV_VHDX_SUBFORMAT_FIXED: > image_type = VHDX_TYPE_FIXED; > - } else if (!strcmp(type, "differencing")) { > - error_setg_errno(errp, ENOTSUP, > - "Differencing files not yet supported"); > - ret = -ENOTSUP; > - goto exit; > - } else { > - error_setg(errp, "Invalid subformat '%s'", type); > - ret = -EINVAL; > - goto exit; > + break; > + default: > + g_assert_not_reached(); > } As a follow-up, it might be reasonable to replace VHDXImageType by BlockdevVhdxSubformat. > > /* These are pretty arbitrary, and mainly designed to keep the BAT > @@ -1865,21 +1879,17 @@ static int coroutine_fn vhdx_co_create_opts(const > char *filename, QemuOpts *opts There is some code not shown here that silently rounds the log_size and the block_size to 1 MB, and... > block_size = block_size > VHDX_BLOCK_SIZE_MAX ? VHDX_BLOCK_SIZE_MAX : > block_size; ...this. In other drivers you seemed to follow the approach of not doing this kind of rounding in the blockdev-create path but only in the legacy interface. Is there a reason for doing it differently here? Max > > - ret = bdrv_create_file(filename, opts, &local_err); > - if (ret < 0) { > - error_propagate(errp, local_err); > - goto exit; > + /* Create BlockBackend to write to the image */ > + bs = bdrv_open_blockdev_ref(vhdx_opts->file, errp); > + if (bs == NULL) { > + return -EIO; > } > > - blk = blk_new_open(filename, NULL, NULL, > - BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, > - &local_err); > - if (blk == NULL) { > - error_propagate(errp, local_err); > - ret = -EIO; > - goto exit; > + blk = blk_new(BLK_PERM_WRITE | BLK_PERM_RESIZE, BLK_PERM_ALL); > + ret = blk_insert_bs(blk, bs, errp); > + if (ret < 0) { > + goto delete_and_exit; > } > - > blk_set_allow_write_beyond_eof(blk, true); > > /* Create (A) */
signature.asc
Description: OpenPGP digital signature