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 <[email protected]> > --- > 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
