[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/5] vmdk: Implement .bdrv_co_create callback
|
From: |
Markus Armbruster |
|
Subject: |
Re: [Qemu-devel] [PATCH 3/5] vmdk: Implement .bdrv_co_create callback |
|
Date: |
Wed, 09 May 2018 14:41:08 +0200 |
|
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux) |
Beware, I'm only looking at QAPI-related aspects.
Fam Zheng <address@hidden> writes:
> This makes VMDK support x-blockdev-create. The implementation reuses the
> image creation code in vmdk_co_create_opts which now acceptes a callback
> pointer to "retrieve" BlockBackend pointers from the caller. This way we
> separate the logic between file/extent acquisition and initialization.
>
> The QAPI command parameters are mostly the same as the old create_opts
> except the dropped legacy @compat6 switch, which is redundant with
> @hwversion.
>
> Signed-off-by: Fam Zheng <address@hidden>
> ---
> block/vmdk.c | 481
> +++++++++++++++++++++++++++++++++++++--------------
> qapi/block-core.json | 67 ++++++-
> 2 files changed, 418 insertions(+), 130 deletions(-)
>
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 083942f806..e16b04e26a 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -1905,38 +1905,87 @@ static int filename_decompose(const char *filename,
> char *path, char *prefix,
> return VMDK_OK;
> }
>
> -static int coroutine_fn vmdk_co_create_opts(const char *filename, QemuOpts
> *opts,
> - Error **errp)
> +/* Stringify BlockdevVmdkSubformat enum in the faimiliar camelCase. */
> +static const char *vmdk_subformat_str(BlockdevVmdkSubformat subformat)
> {
> - int idx = 0;
> - BlockBackend *new_blk = NULL;
> + switch (subformat) {
> + case BLOCKDEV_VMDK_SUBFORMAT_MONOLITHICSPARSE:
> + return "monolithicSparse";
> + case BLOCKDEV_VMDK_SUBFORMAT_MONOLITHICFLAT:
> + return "monolithicFlat";
> + case BLOCKDEV_VMDK_SUBFORMAT_TWOGBMAXEXTENTFLAT:
> + return "twoGbMaxExtentFlat";
> + case BLOCKDEV_VMDK_SUBFORMAT_TWOGBMAXEXTENTSPARSE:
> + return "twoGbMaxExtentSparse";
> + case BLOCKDEV_VMDK_SUBFORMAT_STREAMOPTIMIZED:
> + return "streamOptimized";
> + default:
> + abort();
> + }
> +}
I'd use an array instead of a switch.
The standard mapping from enum FOO to string is FOO_lookup[], commonly
used via qapi_enum_lookup().
vmdk_subformat_str() differs from BlockdevVmdkSubformat_lookup[] only in
case: the former is CamelCase ("monolithicSparse"), the latter is all
lower case (like "monolithicsparse"), because that's how it's spelled in
the QAPI schema. By the way, QAPI naming conventions ask for hyphens,
like "monolithic-sparse".
Why do you need CamelCase? Is it for an existing external interface?
If yes, should we use CamelCase in the schema?
Should we use hyphens, and have this function map hyphen followed by
lower case letter to upper case letter?
> +
> +/*
> + * idx == 0: get or create the descriptor file (also the image file if in a
> + * non-split format.
> + * idx >= 1: get the n-th extent if in a split subformat
> + */
> +typedef BlockBackend *(*vmdk_create_extent_fn)(int64_t size,
> + int idx,
> + bool flat,
> + bool split,
> + bool compress,
> + bool zeroed_grain,
> + void *opaque,
> + Error **errp);
> +
> +static void vmdk_desc_add_extent(GString *desc,
> + const char *extent_line_fmt,
> + int64_t size, const char *filename)
> +{
> + char *desc_line = g_malloc0(BUF_SIZE);
> + const char *basename = strrchr(filename, '/');
Blank line between declarations and statements, if you don't mind.
> + if (!basename) {
> + basename = filename;
> + } else {
> + basename += 1;
> + }
g_path_get_basename()?
> + snprintf(desc_line, BUF_SIZE, extent_line_fmt,
> + DIV_ROUND_UP(size, BDRV_SECTOR_SIZE),
> + basename);
> + g_string_append(desc, desc_line);
> + g_free(desc_line);
g_string_append_printf()?
> +}
> +
> +static int coroutine_fn vmdk_co_do_create(int64_t size,
> + BlockdevVmdkSubformat subformat,
> + BlockdevVmdkAdapterType
> adapter_type,
> + const char *backing_file,
> + const char *hw_version,
> + bool compat6,
> + bool zeroed_grain,
> + vmdk_create_extent_fn extent_fn,
> + void *opaque,
> + Error **errp)
> +{
> + int extent_idx;
> + BlockBackend *blk;
> Error *local_err = NULL;
> char *desc = NULL;
> - int64_t total_size = 0, filesize;
> - char *adapter_type = NULL;
> - char *backing_file = NULL;
> - char *hw_version = NULL;
> - char *fmt = NULL;
> int ret = 0;
> bool flat, split, compress;
> GString *ext_desc_lines;
> - char *path = g_malloc0(PATH_MAX);
> - char *prefix = g_malloc0(PATH_MAX);
> - char *postfix = g_malloc0(PATH_MAX);
> - char *desc_line = g_malloc0(BUF_SIZE);
> - char *ext_filename = g_malloc0(PATH_MAX);
> - char *desc_filename = g_malloc0(PATH_MAX);
> const int64_t split_size = 0x80000000; /* VMDK has constant split size
> */
> - const char *desc_extent_line;
> + int64_t extent_size;
> + int64_t created_size = 0;
> + const char *extent_line_fmt;
> char *parent_desc_line = g_malloc0(BUF_SIZE);
> uint32_t parent_cid = 0xffffffff;
> uint32_t number_heads = 16;
> - bool zeroed_grain = false;
> uint32_t desc_offset = 0, desc_len;
> const char desc_template[] =
> "# Disk DescriptorFile\n"
> "version=1\n"
> - "CID=%" PRIx32 "\n"
> + "CID=%08" PRIx32 "\n"
How is this change related to the patch's purpose?
> "parentCID=%" PRIx32 "\n"
> "createType=\"%s\"\n"
> "%s"
> @@ -1955,71 +2004,35 @@ static int coroutine_fn vmdk_co_create_opts(const
> char *filename, QemuOpts *opts
>
> ext_desc_lines = g_string_new(NULL);
>
> - if (filename_decompose(filename, path, prefix, postfix, PATH_MAX, errp))
> {
> - ret = -EINVAL;
> - goto exit;
> - }
> /* Read out options */
> - total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> - BDRV_SECTOR_SIZE);
> - adapter_type = qemu_opt_get_del(opts, BLOCK_OPT_ADAPTER_TYPE);
> - backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
> - hw_version = qemu_opt_get_del(opts, BLOCK_OPT_HWVERSION);
> - if (qemu_opt_get_bool_del(opts, BLOCK_OPT_COMPAT6, false)) {
> - if (strcmp(hw_version, "undefined")) {
> + if (compat6) {
> + if (hw_version) {
> error_setg(errp,
> "compat6 cannot be enabled with hwversion set");
> ret = -EINVAL;
> goto exit;
> }
> - g_free(hw_version);
> - hw_version = g_strdup("6");
> + hw_version = "6";
> }
> - if (strcmp(hw_version, "undefined") == 0) {
> - g_free(hw_version);
> - hw_version = g_strdup("4");
> - }
> - fmt = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT);
> - if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ZEROED_GRAIN, false)) {
> - zeroed_grain = true;
> + if (!hw_version) {
> + hw_version = "4";
> }
>
> - if (!adapter_type) {
> - adapter_type = g_strdup("ide");
> - } else if (strcmp(adapter_type, "ide") &&
> - strcmp(adapter_type, "buslogic") &&
> - strcmp(adapter_type, "lsilogic") &&
> - strcmp(adapter_type, "legacyESX")) {
> - error_setg(errp, "Unknown adapter type: '%s'", adapter_type);
> - ret = -EINVAL;
> - goto exit;
> - }
> - if (strcmp(adapter_type, "ide") != 0) {
> + if (adapter_type != BLOCKDEV_VMDK_ADAPTER_TYPE_IDE) {
> /* that's the number of heads with which vmware operates when
> creating, exporting, etc. vmdk files with a non-ide adapter type
> */
> number_heads = 255;
> }
> - if (!fmt) {
> - /* Default format to monolithicSparse */
> - fmt = g_strdup("monolithicSparse");
> - } else if (strcmp(fmt, "monolithicFlat") &&
> - strcmp(fmt, "monolithicSparse") &&
> - strcmp(fmt, "twoGbMaxExtentSparse") &&
> - strcmp(fmt, "twoGbMaxExtentFlat") &&
> - strcmp(fmt, "streamOptimized")) {
> - error_setg(errp, "Unknown subformat: '%s'", fmt);
> - ret = -EINVAL;
> - goto exit;
> - }
> - split = !(strcmp(fmt, "twoGbMaxExtentFlat") &&
> - strcmp(fmt, "twoGbMaxExtentSparse"));
> - flat = !(strcmp(fmt, "monolithicFlat") &&
> - strcmp(fmt, "twoGbMaxExtentFlat"));
> - compress = !strcmp(fmt, "streamOptimized");
> + split = (subformat == BLOCKDEV_VMDK_SUBFORMAT_TWOGBMAXEXTENTFLAT) ||
> + (subformat == BLOCKDEV_VMDK_SUBFORMAT_TWOGBMAXEXTENTSPARSE);
> + flat = (subformat == BLOCKDEV_VMDK_SUBFORMAT_MONOLITHICFLAT) ||
> + (subformat == BLOCKDEV_VMDK_SUBFORMAT_TWOGBMAXEXTENTFLAT);
> + compress = subformat == BLOCKDEV_VMDK_SUBFORMAT_STREAMOPTIMIZED;
> +
> if (flat) {
> - desc_extent_line = "RW %" PRId64 " FLAT \"%s\" 0\n";
> + extent_line_fmt = "RW %" PRId64 " FLAT \"%s\" 0\n";
> } else {
> - desc_extent_line = "RW %" PRId64 " SPARSE \"%s\"\n";
> + extent_line_fmt = "RW %" PRId64 " SPARSE \"%s\"\n";
> }
> if (flat && backing_file) {
> error_setg(errp, "Flat image can't have backing file");
> @@ -2031,10 +2044,34 @@ static int coroutine_fn vmdk_co_create_opts(const
> char *filename, QemuOpts *opts
> ret = -ENOTSUP;
> goto exit;
> }
> +
> + /* Create extents */
> + if (split) {
> + extent_size = split_size;
> + } else {
> + extent_size = size;
> + }
> + if (!split && !flat) {
> + created_size = extent_size;
> + } else {
> + created_size = 0;
> + }
> + /* Get the descriptor file BDS */
> + blk = extent_fn(created_size, 0, flat, split, compress, zeroed_grain,
> + opaque, errp);
> + if (!blk) {
> + ret = -EIO;
> + goto exit;
> + }
> + if (!split && !flat) {
> + vmdk_desc_add_extent(ext_desc_lines, extent_line_fmt, created_size,
> + blk_bs(blk)->filename);
> + }
> +
> if (backing_file) {
> - BlockBackend *blk;
> + BlockBackend *backing;
> char *full_backing = g_new0(char, PATH_MAX);
> - bdrv_get_full_backing_filename_from_filename(filename, backing_file,
> + bdrv_get_full_backing_filename_from_filename(blk_bs(blk)->filename,
> backing_file,
> full_backing, PATH_MAX,
> &local_err);
> if (local_err) {
> @@ -2044,93 +2081,65 @@ static int coroutine_fn vmdk_co_create_opts(const
> char *filename, QemuOpts *opts
> goto exit;
> }
>
> - blk = blk_new_open(full_backing, NULL, NULL,
> - BDRV_O_NO_BACKING, errp);
> + backing = blk_new_open(full_backing, NULL, NULL,
> + BDRV_O_NO_BACKING, errp);
> g_free(full_backing);
> - if (blk == NULL) {
> + if (backing == NULL) {
> ret = -EIO;
> goto exit;
> }
> - if (strcmp(blk_bs(blk)->drv->format_name, "vmdk")) {
> - blk_unref(blk);
> + if (strcmp(blk_bs(backing)->drv->format_name, "vmdk")) {
> + error_setg(errp, "Invalid backing file format: %s. Must be vmdk",
> + blk_bs(backing)->drv->format_name);
> + blk_unref(backing);
> ret = -EINVAL;
> goto exit;
> }
> - ret = vmdk_read_cid(blk_bs(blk), 0, &parent_cid);
> - blk_unref(blk);
> + ret = vmdk_read_cid(blk_bs(backing), 0, &parent_cid);
> + blk_unref(backing);
> if (ret) {
> + error_setg(errp, "Failed to read parent CID");
> goto exit;
> }
> snprintf(parent_desc_line, BUF_SIZE,
> "parentFileNameHint=\"%s\"", backing_file);
> }
> -
> - /* Create extents */
> - filesize = total_size;
> - while (filesize > 0) {
> - int64_t size = filesize;
> -
> - if (split && size > split_size) {
> - size = split_size;
> - }
> - if (split) {
> - snprintf(desc_filename, PATH_MAX, "%s-%c%03d%s",
> - prefix, flat ? 'f' : 's', ++idx, postfix);
> - } else if (flat) {
> - snprintf(desc_filename, PATH_MAX, "%s-flat%s", prefix, postfix);
> - } else {
> - snprintf(desc_filename, PATH_MAX, "%s%s", prefix, postfix);
> - }
> - snprintf(ext_filename, PATH_MAX, "%s%s", path, desc_filename);
> -
> - if (vmdk_create_extent(ext_filename, size,
> - flat, compress, zeroed_grain, NULL, opts,
> errp)) {
> + extent_idx = 1;
> + while (created_size < size) {
> + BlockBackend *extent_blk;
> + int64_t cur_size = MIN(size - created_size, extent_size);
> + extent_blk = extent_fn(cur_size, extent_idx, flat, split, compress,
> + zeroed_grain, opaque, errp);
> + if (!extent_blk) {
> ret = -EINVAL;
> goto exit;
> }
> - filesize -= size;
> -
> - /* Format description line */
> - snprintf(desc_line, BUF_SIZE,
> - desc_extent_line, size / BDRV_SECTOR_SIZE,
> desc_filename);
> - g_string_append(ext_desc_lines, desc_line);
> + vmdk_desc_add_extent(ext_desc_lines, extent_line_fmt, cur_size,
> + blk_bs(extent_blk)->filename);
> + created_size += cur_size;
> + extent_idx++;
> + blk_unref(extent_blk);
> }
> /* generate descriptor file */
> desc = g_strdup_printf(desc_template,
> g_random_int(),
> parent_cid,
> - fmt,
> + vmdk_subformat_str(subformat),
> parent_desc_line,
> ext_desc_lines->str,
> hw_version,
> - total_size /
> + size /
> (int64_t)(63 * number_heads *
> BDRV_SECTOR_SIZE),
> number_heads,
> - adapter_type);
> + qapi_enum_lookup(&BlockdevVmdkAdapterType_lookup,
> + adapter_type));
> desc_len = strlen(desc);
> /* the descriptor offset = 0x200 */
> if (!split && !flat) {
> desc_offset = 0x200;
> - } else {
> - ret = bdrv_create_file(filename, opts, &local_err);
> - if (ret < 0) {
> - error_propagate(errp, local_err);
> - goto exit;
> - }
> }
>
> - new_blk = blk_new_open(filename, NULL, NULL,
> - BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL,
> - &local_err);
> - if (new_blk == NULL) {
> - error_propagate(errp, local_err);
> - ret = -EIO;
> - goto exit;
> - }
> -
> - blk_set_allow_write_beyond_eof(new_blk, true);
> -
> - ret = blk_pwrite(new_blk, desc_offset, desc, desc_len, 0);
> + ret = blk_pwrite(blk, desc_offset, desc, desc_len, 0);
> if (ret < 0) {
> error_setg_errno(errp, -ret, "Could not write description");
> goto exit;
> @@ -2138,12 +2147,148 @@ static int coroutine_fn vmdk_co_create_opts(const
> char *filename, QemuOpts *opts
> /* bdrv_pwrite write padding zeros to align to sector, we don't need that
> * for description file */
> if (desc_offset == 0) {
> - ret = blk_truncate(new_blk, desc_len, PREALLOC_MODE_OFF, errp);
> + ret = blk_truncate(blk, desc_len, PREALLOC_MODE_OFF, errp);
> }
> exit:
> - if (new_blk) {
> - blk_unref(new_blk);
> + if (blk) {
> + blk_unref(blk);
> }
> + g_free(desc);
> + g_free(parent_desc_line);
> + g_string_free(ext_desc_lines, true);
> + return ret;
> +}
> +
> +typedef struct {
> + char *path;
> + char *prefix;
> + char *postfix;
> + QemuOpts *opts;
> +} VMDKCreateOptsData;
> +
> +static BlockBackend *vmdk_co_create_opts_cb(int64_t size, int idx,
> + bool flat, bool split, bool
> compress,
> + bool zeroed_grain, void *opaque,
> + Error **errp)
> +{
> + BlockBackend *blk = NULL;
> + BlockDriverState *bs = NULL;
> + VMDKCreateOptsData *data = opaque;
> + char *ext_filename = NULL;
> + char *rel_filename = NULL;
> +
> + if (idx == 0) {
> + rel_filename = g_strdup_printf("%s%s", data->prefix, data->postfix);
> + } else if (split) {
> + rel_filename = g_strdup_printf("%s-%c%03d%s",
> + data->prefix,
> + flat ? 'f' : 's', idx, data->postfix);
> + } else {
> + assert(idx == 1);
> + rel_filename = g_strdup_printf("%s-flat%s", data->prefix,
> data->postfix);
> + }
> +
> + ext_filename = g_strdup_printf("%s%s", data->path, rel_filename);
> + g_free(rel_filename);
> +
> + if (vmdk_create_extent(ext_filename, size,
> + flat, compress, zeroed_grain, &blk, data->opts,
> + errp)) {
> + goto exit;
> + }
> + bdrv_unref(bs);
> +exit:
> + g_free(ext_filename);
> + return blk;
> +}
> +
> +static int coroutine_fn vmdk_co_create_opts(const char *filename, QemuOpts
> *opts,
> + Error **errp)
> +{
> + Error *local_err = NULL;
> + char *desc = NULL;
> + int64_t total_size = 0;
> + char *adapter_type = NULL;
> + BlockdevVmdkAdapterType adapter_type_enum;
> + char *backing_file = NULL;
> + char *hw_version = NULL;
> + char *fmt = NULL;
> + BlockdevVmdkSubformat subformat;
> + int ret = 0;
> + char *path = g_malloc0(PATH_MAX);
> + char *prefix = g_malloc0(PATH_MAX);
> + char *postfix = g_malloc0(PATH_MAX);
> + char *desc_line = g_malloc0(BUF_SIZE);
> + char *ext_filename = g_malloc0(PATH_MAX);
> + char *desc_filename = g_malloc0(PATH_MAX);
> + char *parent_desc_line = g_malloc0(BUF_SIZE);
> + bool zeroed_grain;
> + bool compat6;
> + int i;
> + VMDKCreateOptsData data;
> +
> + if (filename_decompose(filename, path, prefix, postfix, PATH_MAX, errp))
> {
> + ret = -EINVAL;
> + goto exit;
> + }
> + /* Read out options */
> + total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> + BDRV_SECTOR_SIZE);
> + adapter_type = qemu_opt_get_del(opts, BLOCK_OPT_ADAPTER_TYPE);
> + backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
> + hw_version = qemu_opt_get_del(opts, BLOCK_OPT_HWVERSION);
> + compat6 = qemu_opt_get_bool_del(opts, BLOCK_OPT_COMPAT6, false);
> + if (strcmp(hw_version, "undefined") == 0) {
> + g_free(hw_version);
> + hw_version = g_strdup("4");
> + }
> + fmt = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT);
> + zeroed_grain = qemu_opt_get_bool_del(opts, BLOCK_OPT_ZEROED_GRAIN,
> false);
> +
> + if (adapter_type) {
> + for (i = 0; i < strlen(adapter_type); ++i) {
> + adapter_type[i] = qemu_tolower(adapter_type[i]);
> + }
First, you convert to lower cases, and then...
> + adapter_type_enum =
> qapi_enum_parse_full(&BlockdevVmdkAdapterType_lookup,
> + adapter_type,
> +
> BLOCKDEV_VMDK_ADAPTER_TYPE_IDE,
> + true,
> + &local_err);
... you parse case-insensitive. Huh?
Which spellings did the old code accept? As far as I can tell, exactly
"ide", "lsilogic", "buslogic", "legacyESX". Are you sure we should
ignore case going forward?
> + if (local_err) {
> + error_propagate(errp, local_err);
> + ret = -EINVAL;
> + goto exit;
> + }
> + } else {
> + adapter_type_enum = BLOCKDEV_VMDK_ADAPTER_TYPE_IDE;
> + }
> +
> + if (!fmt) {
> + /* Default format to monolithicSparse */
> + subformat = BLOCKDEV_VMDK_SUBFORMAT_MONOLITHICSPARSE;
> + } else {
> + subformat = qapi_enum_parse_full(&BlockdevVmdkSubformat_lookup,
> + fmt,
> +
> BLOCKDEV_VMDK_SUBFORMAT_MONOLITHICSPARSE,
> + true,
> + &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + ret = -EINVAL;
> + goto exit;
> + }
Likewise: should we ignore case going forward? The old code appears to
accept exactly "monolithicFlat", "monolithicSparse",
"twoGbMaxExtentSparse", "twoGbMaxExtentFlat", "streamOptimized".
> + }
> + data = (VMDKCreateOptsData){
> + .prefix = prefix,
> + .postfix = postfix,
> + .path = path,
> + .opts = opts,
> + };
> + ret = vmdk_co_do_create(total_size, subformat, adapter_type_enum,
> + backing_file, hw_version, compat6, zeroed_grain,
> + vmdk_co_create_opts_cb, &data, errp);
> +
> +exit:
> g_free(adapter_type);
> g_free(backing_file);
> g_free(hw_version);
> @@ -2156,7 +2301,84 @@ exit:
> g_free(ext_filename);
> g_free(desc_filename);
> g_free(parent_desc_line);
> - g_string_free(ext_desc_lines, true);
> + return ret;
> +}
> +
> +static BlockBackend *vmdk_co_create_cb(int64_t size, int idx,
> + bool flat, bool split, bool compress,
> + bool zeroed_grain, void *opaque,
> + Error **errp)
> +{
> + int ret;
> + BlockDriverState *bs;
> + BlockBackend *blk;
> + BlockdevCreateOptionsVmdk *opts = opaque;
> +
> + if (idx == 0) {
> + bs = bdrv_open_blockdev_ref(opts->file, errp);
> + } else {
> + int i;
> + BlockdevRefList *list = opts->extents;
> + for (i = 1; i < idx; i++) {
> + if (!list || !list->next) {
> + error_setg(errp, "Extent [%d] not specified", i);
> + return NULL;
> + }
> + list = list->next;
> + }
> + if (!list) {
> + error_setg(errp, "Extent [%d] not specified", idx - 1);
> + return NULL;
> + }
> + bs = bdrv_open_blockdev_ref(list->value, errp);
> + }
> + if (!bs) {
> + return NULL;
> + }
> + blk = blk_new(BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE |
> BLK_PERM_RESIZE,
> + BLK_PERM_ALL);
> + if (blk_insert_bs(blk, bs, errp)) {
> + bdrv_unref(bs);
> + return NULL;
> + }
> + blk_set_allow_write_beyond_eof(blk, true);
> + bdrv_unref(bs);
> +
> + ret = vmdk_init_extent(blk, size, flat, compress, zeroed_grain, errp);
> + if (ret) {
> + blk_unref(blk);
> + blk = NULL;
> + }
> + return blk;
> +}
> +
> +static int coroutine_fn vmdk_co_create(BlockdevCreateOptions *create_options,
> + Error **errp)
> +{
> + int ret;
> + BlockdevCreateOptionsVmdk *opts;
> +
> + opts = &create_options->u.vmdk;
> +
> + /* Validate options */
> + if (!QEMU_IS_ALIGNED(opts->size, BDRV_SECTOR_SIZE)) {
> + error_setg(errp, "Image size must be a multiple of 512 bytes");
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + ret = vmdk_co_do_create(opts->size,
> + opts->subformat,
> + opts->adapter_type,
> + opts->backing_file,
> + opts->hwversion,
> + false,
> + opts->zeroed_grain,
> + vmdk_co_create_cb,
> + opts, errp);
> + return ret;
> +
> +out:
> return ret;
> }
>
> @@ -2424,6 +2646,7 @@ static BlockDriver bdrv_vmdk = {
> .bdrv_co_pwrite_zeroes = vmdk_co_pwrite_zeroes,
> .bdrv_close = vmdk_close,
> .bdrv_co_create_opts = vmdk_co_create_opts,
> + .bdrv_co_create = vmdk_co_create,
> .bdrv_co_flush_to_disk = vmdk_co_flush,
> .bdrv_co_block_status = vmdk_co_block_status,
> .bdrv_get_allocated_file_size = vmdk_get_allocated_file_size,
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index c50517bff3..df3903b54d 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -3855,6 +3855,71 @@
> 'size': 'size',
> '*cluster-size' : 'size' } }
>
> +##
> +# @BlockdevVmdkSubformat:
> +#
> +# Subformat options for VMDK images
> +#
> +# @monolithicsparse: Single file image with sparse cluster allocation
> +# @monolithicflat: Single flat data image and a descriptor file
> +# @twogbmaxextentsparse: Data is split into 2GB (per virtual LBA) sparse
> extent
> +# files, in addition to a descriptor file
> +# @twogbmaxextentflat: Data is split into 2GB (per virtual LBA) flat extent
> +# files, in addition to a descriptor file
> +# @streamoptimized: Single file image sparse cluster allocation, optimized
> for
> +# streaming over network.
> +#
> +# Since: 2.13
> +##
> +{ 'enum': 'BlockdevVmdkSubformat',
> + 'data': [ 'monolithicsparse', 'monolithicflat', 'twogbmaxextentsparse',
> + 'twogbmaxextentflat', 'streamoptimized'] }
alllowercasewithoutspacesisevenlesslegiblethanCamelCase.
THERESULTINGCIDENTIFIERSAREALLCAPSWITHOUTSPACESWHICHISEVENWORSE.
QAPI conventions ask for monolithic-sparse, monolithic-flat,
two-gb-max-extent-sparse and so forth. Results in C enum identifiers
BLOCKDEV_VMDK_SUBFORMAT_MONOLITHIC_SPARSE,
BLOCKDEV_VMDK_SUBFORMAT_MONOLITHIC_FLAT,
BLOCKDEV_VMDK_SUBFORMAT_TWO_GB_MAX_EXTENT_SPARSE, ...
The existing external interface appears to ask for monolithicFlat,
monolithicSparse, twoGbMaxExtentSparse, ... What's the best way to map
between these guys and a QAPI enum?
> +
> +##
> +# @BlockdevVmdkAdapterType:
> +#
> +# Adapter type info for VMDK images
> +#
> +# Since: 2.13
> +##
> +{ 'enum': 'BlockdevVmdkAdapterType',
> + 'data': [ 'ide', 'buslogic', 'lsilogic', 'legacyesx'] }
> +
> +##
> +# @BlockdevCreateOptionsVmdk:
> +#
> +# Driver specific image creation options for VMDK.
> +#
> +# @file Where to store the new image file. This refers to the image
> +# file for monolithcSparse and streamOptimized format, or the
> +# descriptor file for other formats.
> +# @size Size of the virtual disk in bytes
> +# @extents Where to store the data extents. Required for monolithcflat,
> +# twoGbMaxExtentSparse and twoGbMaxExtentFlat formats. For
> +# monolithicflat, only one entry is required; for
> +# twoGbMaxExtent* formats, the number of entries required is
> +# calculated as extent_number = virtual_size / 2GB.
> +# @subformat The subformat of the VMDK image. Default: "monolithicsparse".
> +# @backing-file The path of backing file. Default: no backing file is used.
> +# @adapter-type The adapter type used to fill in the descriptor. Default:
> ide.
> +# @hwversion Hardware version. The meaningful options are "4" or "6".
Okay, these are the meaningfull options. What are the meaningless ones?
> +# Defaulted to "4".
More common phrasings are
Default is "4"
Defaults to "4"
Default: "4"
> +# @zeroed-grain Whether to enable zeroed-grain feature for sparse subformats.
> +# Default: false.
> +#
> +# Since: 2.13
> +##
> +{ 'struct': 'BlockdevCreateOptionsVmdk',
> + 'data': { 'file': 'BlockdevRef',
> + 'size': 'size',
> + '*extents': ['BlockdevRef'],
> + '*subformat': 'BlockdevVmdkSubformat',
> + '*backing-file': 'str',
> + '*adapter-type': 'BlockdevVmdkAdapterType',
> + '*hwversion': 'str',
> + '*zeroed-grain': 'bool' } }
> +
> +
> ##
> # @SheepdogRedundancyType:
> #
> @@ -4078,7 +4143,7 @@
> 'throttle': 'BlockdevCreateNotSupported',
> 'vdi': 'BlockdevCreateOptionsVdi',
> 'vhdx': 'BlockdevCreateOptionsVhdx',
> - 'vmdk': 'BlockdevCreateNotSupported',
> + 'vmdk': 'BlockdevCreateOptionsVmdk',
> 'vpc': 'BlockdevCreateOptionsVpc',
> 'vvfat': 'BlockdevCreateNotSupported',
> 'vxhs': 'BlockdevCreateNotSupported'