[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 1/4] vmdk: Refactor vmdk_create_extent
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v3 1/4] vmdk: Refactor vmdk_create_extent |
Date: |
Fri, 07 Dec 2018 07:40:50 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) |
Kevin Wolf <address@hidden> writes:
> From: Fam Zheng <address@hidden>
>
> The extracted vmdk_init_extent takes a BlockBackend object and
> initializes the format metadata. It is the common part between "qemu-img
> create" and "blockdev-create".
>
> Add a "BlockBackend *pbb" parameter to vmdk_create_extent, to return the
> opened BB to the caller in the next patch.
I'd do this part in the next patch, when it has a user. Matter of
taste.
>
> Signed-off-by: Fam Zheng <address@hidden>
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
> block/vmdk.c | 69 ++++++++++++++++++++++++++++++++--------------------
> 1 file changed, 43 insertions(+), 26 deletions(-)
>
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 2c9e86d98f..32fc2c84b3 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -1741,35 +1741,17 @@ static int coroutine_fn
> vmdk_co_pwrite_zeroes(BlockDriverState *bs,
> return ret;
> }
>
> -static int vmdk_create_extent(const char *filename, int64_t filesize,
> - bool flat, bool compress, bool zeroed_grain,
> - QemuOpts *opts, Error **errp)
> +static int vmdk_init_extent(BlockBackend *blk,
> + int64_t filesize, bool flat,
> + bool compress, bool zeroed_grain,
> + Error **errp)
> {
> int ret, i;
> - BlockBackend *blk = NULL;
> VMDK4Header header;
> - Error *local_err = NULL;
> uint32_t tmp, magic, grains, gd_sectors, gt_size, gt_count;
> uint32_t *gd_buf = NULL;
> int gd_buf_size;
>
> - ret = bdrv_create_file(filename, opts, &local_err);
> - if (ret < 0) {
> - error_propagate(errp, local_err);
> - goto exit;
> - }
> -
> - 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_set_allow_write_beyond_eof(blk, true);
> -
> if (flat) {
> ret = blk_truncate(blk, filesize, PREALLOC_MODE_OFF, errp);
> goto exit;
> @@ -1863,15 +1845,50 @@ static int vmdk_create_extent(const char *filename,
> int64_t filesize,
> gd_buf, gd_buf_size, 0);
> if (ret < 0) {
> error_setg(errp, QERR_IO_ERROR);
> - goto exit;
> }
>
> ret = 0;
> +exit:
> + g_free(gd_buf);
> + return ret;
> +}
> +
> +static int vmdk_create_extent(const char *filename, int64_t filesize,
> + bool flat, bool compress, bool zeroed_grain,
> + BlockBackend **pbb,
> + QemuOpts *opts, Error **errp)
> +{
> + int ret;
> + BlockBackend *blk = NULL;
> + Error *local_err = NULL;
> +
> + ret = bdrv_create_file(filename, opts, &local_err);
> + if (ret < 0) {
> + error_propagate(errp, local_err);
> + goto exit;
> + }
> +
> + 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_set_allow_write_beyond_eof(blk, true);
> +
> + ret = vmdk_init_extent(blk, filesize, flat, compress, zeroed_grain,
> errp);
> exit:
> if (blk) {
> - blk_unref(blk);
> + if (pbb) {
> + *pbb = blk;
> + } else {
> + blk_unref(blk);
> + blk = NULL;
Dead assignment. Matter of taste.
> + }
> }
> - g_free(gd_buf);
> return ret;
> }
>
> @@ -2094,7 +2111,7 @@ static int coroutine_fn vmdk_co_create_opts(const char
> *filename, QemuOpts *opts
> snprintf(ext_filename, PATH_MAX, "%s%s", path, desc_filename);
>
> if (vmdk_create_extent(ext_filename, size,
> - flat, compress, zeroed_grain, opts, errp)) {
> + flat, compress, zeroed_grain, NULL, opts,
> errp)) {
> ret = -EINVAL;
> goto exit;
> }
Reviewed-by: Markus Armbruster <address@hidden>
[Qemu-devel] [PATCH v3 4/4] iotests: Add VMDK tests for blockdev-create, Kevin Wolf, 2018/12/06
Re: [Qemu-devel] [PATCH v3 0/4] vmdk: Implement blockdev-create, no-reply, 2018/12/06