[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH 6/7] vhdx: Support .bdrv_co_create
From: |
Max Reitz |
Subject: |
Re: [Qemu-block] [PATCH 6/7] vhdx: Support .bdrv_co_create |
Date: |
Mon, 12 Mar 2018 22:38:19 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 |
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 <address@hidden>
> ---
> 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
- [Qemu-block] [PATCH 4/7] qed: Support .bdrv_co_create, (continued)
- [Qemu-block] [PATCH 4/7] qed: Support .bdrv_co_create, Kevin Wolf, 2018/03/09
- [Qemu-block] [PATCH 1/7] parallels: Support .bdrv_co_create, Kevin Wolf, 2018/03/09
- [Qemu-block] [PATCH 5/7] vdi: Support .bdrv_co_create, Kevin Wolf, 2018/03/09
- [Qemu-block] [PATCH 6/7] vhdx: Support .bdrv_co_create, Kevin Wolf, 2018/03/09
- [Qemu-block] [PATCH 7/7] vpc: Support .bdrv_co_create, Kevin Wolf, 2018/03/09