qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 0/8] block: .bdrv_co_create for format drivers
@ 2018-03-13 14:47 Kevin Wolf
  2018-03-13 14:47 ` [Qemu-devel] [PATCH v2 1/8] parallels: Support .bdrv_co_create Kevin Wolf
                   ` (8 more replies)
  0 siblings, 9 replies; 14+ messages in thread
From: Kevin Wolf @ 2018-03-13 14:47 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, mreitz, den, jcody, eblake, qemu-devel

This series adds a .bdrv_co_create implementation to almost all format
drivers that support creating images where its still missing. The only
exception is VMDK because its support for extents will make the QAPI
design a bit more complicated.

The other format driver not covered in this series are qcow2 (already
merged) and luks (already posted in a separate series).

v2:
- Rebased, the vdi patch consists just of some cosmetic cleanups now
- vhdx, vpc: Don't do any silent rounding in .bdrv_co_create, error out
  if the passed size isn't properly aligned yet. The legacy code paths
  compensate for this.

Kevin Wolf (8):
  parallels: Support .bdrv_co_create
  qemu-iotests: Enable write tests for parallels
  qcow: Support .bdrv_co_create
  qed: Support .bdrv_co_create
  vdi: Make comments consistent with other drivers
  vhdx: Support .bdrv_co_create
  vpc: Support .bdrv_co_create
  vpc: Require aligned size in .bdrv_co_create

 qapi/block-core.json     | 137 ++++++++++++++++++++++++++-
 block/parallels.c        | 199 ++++++++++++++++++++++++++++----------
 block/qcow.c             | 196 +++++++++++++++++++++++++-------------
 block/qed.c              | 204 ++++++++++++++++++++++++++-------------
 block/vdi.c              |  12 ++-
 block/vhdx.c             | 216 ++++++++++++++++++++++++++++++++----------
 block/vpc.c              | 241 ++++++++++++++++++++++++++++++++++++-----------
 tests/qemu-iotests/181   |   2 +-
 tests/qemu-iotests/check |   1 -
 9 files changed, 910 insertions(+), 298 deletions(-)

-- 
2.13.6

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v2 1/8] parallels: Support .bdrv_co_create
  2018-03-13 14:47 [Qemu-devel] [PATCH v2 0/8] block: .bdrv_co_create for format drivers Kevin Wolf
@ 2018-03-13 14:47 ` Kevin Wolf
  2018-03-13 14:47 ` [Qemu-devel] [PATCH v2 2/8] qemu-iotests: Enable write tests for parallels Kevin Wolf
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Kevin Wolf @ 2018-03-13 14:47 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, mreitz, den, jcody, eblake, qemu-devel

This adds the .bdrv_co_create driver callback to parallels, which
enables image creation over QMP.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
---
 qapi/block-core.json |  18 ++++-
 block/parallels.c    | 199 ++++++++++++++++++++++++++++++++++++++-------------
 2 files changed, 168 insertions(+), 49 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 6211b8222c..e0ab01d92d 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3625,6 +3625,22 @@
             'size':             'size' } }
 
 ##
+# @BlockdevCreateOptionsParallels:
+#
+# Driver specific image creation options for parallels.
+#
+# @file             Node to create the image format on
+# @size             Size of the virtual disk in bytes
+# @cluster-size     Cluster size in bytes (default: 1 MB)
+#
+# Since: 2.12
+##
+{ 'struct': 'BlockdevCreateOptionsParallels',
+  'data': { 'file':             'BlockdevRef',
+            'size':             'size',
+            '*cluster-size':    'size' } }
+
+##
 # @BlockdevQcow2Version:
 #
 # @v2:  The original QCOW2 format as introduced in qemu 0.10 (version 2)
@@ -3826,7 +3842,7 @@
       'null-aio':       'BlockdevCreateNotSupported',
       'null-co':        'BlockdevCreateNotSupported',
       'nvme':           'BlockdevCreateNotSupported',
-      'parallels':      'BlockdevCreateNotSupported',
+      'parallels':      'BlockdevCreateOptionsParallels',
       'qcow2':          'BlockdevCreateOptionsQcow2',
       'qcow':           'BlockdevCreateNotSupported',
       'qed':            'BlockdevCreateNotSupported',
diff --git a/block/parallels.c b/block/parallels.c
index c13cb619e6..2da5e56a9d 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -34,6 +34,9 @@
 #include "sysemu/block-backend.h"
 #include "qemu/module.h"
 #include "qemu/option.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qobject-input-visitor.h"
+#include "qapi/qapi-visit-block-core.h"
 #include "qemu/bswap.h"
 #include "qemu/bitmap.h"
 #include "migration/blocker.h"
@@ -79,6 +82,25 @@ static QemuOptsList parallels_runtime_opts = {
     },
 };
 
+static QemuOptsList parallels_create_opts = {
+    .name = "parallels-create-opts",
+    .head = QTAILQ_HEAD_INITIALIZER(parallels_create_opts.head),
+    .desc = {
+        {
+            .name = BLOCK_OPT_SIZE,
+            .type = QEMU_OPT_SIZE,
+            .help = "Virtual disk size",
+        },
+        {
+            .name = BLOCK_OPT_CLUSTER_SIZE,
+            .type = QEMU_OPT_SIZE,
+            .help = "Parallels image cluster size",
+            .def_value_str = stringify(DEFAULT_CLUSTER_SIZE),
+        },
+        { /* end of list */ }
+    }
+};
+
 
 static int64_t bat2sect(BDRVParallelsState *s, uint32_t idx)
 {
@@ -480,46 +502,62 @@ out:
 }
 
 
-static int coroutine_fn parallels_co_create_opts(const char *filename,
-                                                 QemuOpts *opts,
-                                                 Error **errp)
+static int coroutine_fn parallels_co_create(BlockdevCreateOptions* opts,
+                                            Error **errp)
 {
+    BlockdevCreateOptionsParallels *parallels_opts;
+    BlockDriverState *bs;
+    BlockBackend *blk;
     int64_t total_size, cl_size;
-    uint8_t tmp[BDRV_SECTOR_SIZE];
-    Error *local_err = NULL;
-    BlockBackend *file;
     uint32_t bat_entries, bat_sectors;
     ParallelsHeader header;
+    uint8_t tmp[BDRV_SECTOR_SIZE];
     int ret;
 
-    total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
-                          BDRV_SECTOR_SIZE);
-    cl_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_CLUSTER_SIZE,
-                          DEFAULT_CLUSTER_SIZE), BDRV_SECTOR_SIZE);
+    assert(opts->driver == BLOCKDEV_DRIVER_PARALLELS);
+    parallels_opts = &opts->u.parallels;
+
+    /* Sanity checks */
+    total_size = parallels_opts->size;
+
+    if (parallels_opts->has_cluster_size) {
+        cl_size = parallels_opts->cluster_size;
+    } else {
+        cl_size = DEFAULT_CLUSTER_SIZE;
+    }
+
     if (total_size >= MAX_PARALLELS_IMAGE_FACTOR * cl_size) {
-        error_propagate(errp, local_err);
+        error_setg(errp, "Image size is too large for this cluster size");
         return -E2BIG;
     }
 
-    ret = bdrv_create_file(filename, opts, &local_err);
-    if (ret < 0) {
-        error_propagate(errp, local_err);
-        return ret;
+    if (!QEMU_IS_ALIGNED(total_size, BDRV_SECTOR_SIZE)) {
+        error_setg(errp, "Image size must be a multiple of 512 bytes");
+        return -EINVAL;
     }
 
-    file = blk_new_open(filename, NULL, NULL,
-                        BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL,
-                        &local_err);
-    if (file == NULL) {
-        error_propagate(errp, local_err);
+    if (!QEMU_IS_ALIGNED(cl_size, BDRV_SECTOR_SIZE)) {
+        error_setg(errp, "Cluster size must be a multiple of 512 bytes");
+        return -EINVAL;
+    }
+
+    /* Create BlockBackend to write to the image */
+    bs = bdrv_open_blockdev_ref(parallels_opts->file, errp);
+    if (bs == NULL) {
         return -EIO;
     }
 
-    blk_set_allow_write_beyond_eof(file, true);
+    blk = blk_new(BLK_PERM_WRITE | BLK_PERM_RESIZE, BLK_PERM_ALL);
+    ret = blk_insert_bs(blk, bs, errp);
+    if (ret < 0) {
+        goto out;
+    }
+    blk_set_allow_write_beyond_eof(blk, true);
 
-    ret = blk_truncate(file, 0, PREALLOC_MODE_OFF, errp);
+    /* Create image format */
+    ret = blk_truncate(blk, 0, PREALLOC_MODE_OFF, errp);
     if (ret < 0) {
-        goto exit;
+        goto out;
     }
 
     bat_entries = DIV_ROUND_UP(total_size, cl_size);
@@ -542,24 +580,107 @@ static int coroutine_fn parallels_co_create_opts(const char *filename,
     memset(tmp, 0, sizeof(tmp));
     memcpy(tmp, &header, sizeof(header));
 
-    ret = blk_pwrite(file, 0, tmp, BDRV_SECTOR_SIZE, 0);
+    ret = blk_pwrite(blk, 0, tmp, BDRV_SECTOR_SIZE, 0);
     if (ret < 0) {
         goto exit;
     }
-    ret = blk_pwrite_zeroes(file, BDRV_SECTOR_SIZE,
+    ret = blk_pwrite_zeroes(blk, BDRV_SECTOR_SIZE,
                             (bat_sectors - 1) << BDRV_SECTOR_BITS, 0);
     if (ret < 0) {
         goto exit;
     }
-    ret = 0;
 
-done:
-    blk_unref(file);
+    ret = 0;
+out:
+    blk_unref(blk);
+    bdrv_unref(bs);
     return ret;
 
 exit:
     error_setg_errno(errp, -ret, "Failed to create Parallels image");
-    goto done;
+    goto out;
+}
+
+static int coroutine_fn parallels_co_create_opts(const char *filename,
+                                                 QemuOpts *opts,
+                                                 Error **errp)
+{
+    BlockdevCreateOptions *create_options = NULL;
+    Error *local_err = NULL;
+    BlockDriverState *bs = NULL;
+    QDict *qdict = NULL;
+    QObject *qobj;
+    Visitor *v;
+    int ret;
+
+    static const QDictRenames opt_renames[] = {
+        { BLOCK_OPT_CLUSTER_SIZE,       "cluster-size" },
+        { NULL, NULL },
+    };
+
+    /* Parse options and convert legacy syntax */
+    qdict = qemu_opts_to_qdict_filtered(opts, NULL, &parallels_create_opts,
+                                        true);
+
+    if (!qdict_rename_keys(qdict, opt_renames, errp)) {
+        ret = -EINVAL;
+        goto done;
+    }
+
+    /* Create and open the file (protocol layer) */
+    ret = bdrv_create_file(filename, opts, &local_err);
+    if (ret < 0) {
+        error_propagate(errp, local_err);
+        goto done;
+    }
+
+    bs = bdrv_open(filename, NULL, NULL,
+                   BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, errp);
+    if (bs == NULL) {
+        ret = -EIO;
+        goto done;
+    }
+
+    /* Now get the QAPI type BlockdevCreateOptions */
+    qdict_put_str(qdict, "driver", "parallels");
+    qdict_put_str(qdict, "file", bs->node_name);
+
+    qobj = qdict_crumple(qdict, errp);
+    QDECREF(qdict);
+    qdict = qobject_to_qdict(qobj);
+    if (qdict == NULL) {
+        ret = -EINVAL;
+        goto done;
+    }
+
+    v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
+    visit_type_BlockdevCreateOptions(v, NULL, &create_options, &local_err);
+    visit_free(v);
+
+    if (local_err) {
+        error_propagate(errp, local_err);
+        ret = -EINVAL;
+        goto done;
+    }
+
+    /* Silently round up sizes */
+    create_options->u.parallels.size =
+        ROUND_UP(create_options->u.parallels.size, BDRV_SECTOR_SIZE);
+    create_options->u.parallels.cluster_size =
+        ROUND_UP(create_options->u.parallels.cluster_size, BDRV_SECTOR_SIZE);
+
+    /* Create the Parallels image (format layer) */
+    ret = parallels_co_create(create_options, errp);
+    if (ret < 0) {
+        goto done;
+    }
+    ret = 0;
+
+done:
+    QDECREF(qdict);
+    bdrv_unref(bs);
+    qapi_free_BlockdevCreateOptions(create_options);
+    return ret;
 }
 
 
@@ -771,25 +892,6 @@ static void parallels_close(BlockDriverState *bs)
     error_free(s->migration_blocker);
 }
 
-static QemuOptsList parallels_create_opts = {
-    .name = "parallels-create-opts",
-    .head = QTAILQ_HEAD_INITIALIZER(parallels_create_opts.head),
-    .desc = {
-        {
-            .name = BLOCK_OPT_SIZE,
-            .type = QEMU_OPT_SIZE,
-            .help = "Virtual disk size",
-        },
-        {
-            .name = BLOCK_OPT_CLUSTER_SIZE,
-            .type = QEMU_OPT_SIZE,
-            .help = "Parallels image cluster size",
-            .def_value_str = stringify(DEFAULT_CLUSTER_SIZE),
-        },
-        { /* end of list */ }
-    }
-};
-
 static BlockDriver bdrv_parallels = {
     .format_name	= "parallels",
     .instance_size	= sizeof(BDRVParallelsState),
@@ -803,6 +905,7 @@ static BlockDriver bdrv_parallels = {
     .bdrv_co_readv  = parallels_co_readv,
     .bdrv_co_writev = parallels_co_writev,
     .supports_backing = true,
+    .bdrv_co_create      = parallels_co_create,
     .bdrv_co_create_opts = parallels_co_create_opts,
     .bdrv_co_check  = parallels_co_check,
     .create_opts    = &parallels_create_opts,
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v2 2/8] qemu-iotests: Enable write tests for parallels
  2018-03-13 14:47 [Qemu-devel] [PATCH v2 0/8] block: .bdrv_co_create for format drivers Kevin Wolf
  2018-03-13 14:47 ` [Qemu-devel] [PATCH v2 1/8] parallels: Support .bdrv_co_create Kevin Wolf
@ 2018-03-13 14:47 ` Kevin Wolf
  2018-03-13 14:47 ` [Qemu-devel] [PATCH v2 3/8] qcow: Support .bdrv_co_create Kevin Wolf
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Kevin Wolf @ 2018-03-13 14:47 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, mreitz, den, jcody, eblake, qemu-devel

Originally we added parallels as a read-only format to qemu-iotests
where we did just some tests with a binary image. Since then, write and
image creation support has been added to the driver, so we can now
enable it in _supported_fmt generic.

The driver doesn't support migration yet, though, so we need to add it
to the list of exceptions in 181.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
---
 tests/qemu-iotests/181   | 2 +-
 tests/qemu-iotests/check | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/181 b/tests/qemu-iotests/181
index 0c91e8f9de..5e767c6195 100755
--- a/tests/qemu-iotests/181
+++ b/tests/qemu-iotests/181
@@ -44,7 +44,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _supported_fmt generic
 # Formats that do not support live migration
-_unsupported_fmt qcow vdi vhdx vmdk vpc vvfat
+_unsupported_fmt qcow vdi vhdx vmdk vpc vvfat parallels
 _supported_proto generic
 _supported_os Linux
 
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index e6b6ff7a04..469142cd58 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -284,7 +284,6 @@ testlist options
 
         -parallels)
             IMGFMT=parallels
-            IMGFMT_GENERIC=false
             xpand=false
             ;;
 
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v2 3/8] qcow: Support .bdrv_co_create
  2018-03-13 14:47 [Qemu-devel] [PATCH v2 0/8] block: .bdrv_co_create for format drivers Kevin Wolf
  2018-03-13 14:47 ` [Qemu-devel] [PATCH v2 1/8] parallels: Support .bdrv_co_create Kevin Wolf
  2018-03-13 14:47 ` [Qemu-devel] [PATCH v2 2/8] qemu-iotests: Enable write tests for parallels Kevin Wolf
@ 2018-03-13 14:47 ` Kevin Wolf
  2018-03-13 14:47 ` [Qemu-devel] [PATCH v2 4/8] qed: " Kevin Wolf
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Kevin Wolf @ 2018-03-13 14:47 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, mreitz, den, jcody, eblake, qemu-devel

This adds the .bdrv_co_create driver callback to qcow, which
enables image creation over QMP.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
---
 qapi/block-core.json |  21 +++++-
 block/qcow.c         | 196 ++++++++++++++++++++++++++++++++++-----------------
 2 files changed, 150 insertions(+), 67 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index e0ab01d92d..7b7d5a01fd 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3641,6 +3641,25 @@
             '*cluster-size':    'size' } }
 
 ##
+# @BlockdevCreateOptionsQcow:
+#
+# Driver specific image creation options for qcow.
+#
+# @file             Node to create the image format on
+# @size             Size of the virtual disk in bytes
+# @backing-file     File name of the backing file if a backing file
+#                   should be used
+# @encrypt          Encryption options if the image should be encrypted
+#
+# Since: 2.12
+##
+{ 'struct': 'BlockdevCreateOptionsQcow',
+  'data': { 'file':             'BlockdevRef',
+            'size':             'size',
+            '*backing-file':    'str',
+            '*encrypt':         'QCryptoBlockCreateOptions' } }
+
+##
 # @BlockdevQcow2Version:
 #
 # @v2:  The original QCOW2 format as introduced in qemu 0.10 (version 2)
@@ -3843,8 +3862,8 @@
       'null-co':        'BlockdevCreateNotSupported',
       'nvme':           'BlockdevCreateNotSupported',
       'parallels':      'BlockdevCreateOptionsParallels',
+      'qcow':           'BlockdevCreateOptionsQcow',
       'qcow2':          'BlockdevCreateOptionsQcow2',
-      'qcow':           'BlockdevCreateNotSupported',
       'qed':            'BlockdevCreateNotSupported',
       'quorum':         'BlockdevCreateNotSupported',
       'raw':            'BlockdevCreateNotSupported',
diff --git a/block/qcow.c b/block/qcow.c
index 47a18d9a3a..2e3770ca63 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -33,6 +33,8 @@
 #include <zlib.h>
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qstring.h"
+#include "qapi/qobject-input-visitor.h"
+#include "qapi/qapi-visit-block-core.h"
 #include "crypto/block.h"
 #include "migration/blocker.h"
 #include "block/crypto.h"
@@ -86,6 +88,8 @@ typedef struct BDRVQcowState {
     Error *migration_blocker;
 } BDRVQcowState;
 
+static QemuOptsList qcow_create_opts;
+
 static int decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset);
 
 static int qcow_probe(const uint8_t *buf, int buf_size, const char *filename)
@@ -810,62 +814,50 @@ static void qcow_close(BlockDriverState *bs)
     error_free(s->migration_blocker);
 }
 
-static int coroutine_fn qcow_co_create_opts(const char *filename, QemuOpts *opts,
-                                            Error **errp)
+static int coroutine_fn qcow_co_create(BlockdevCreateOptions *opts,
+                                       Error **errp)
 {
+    BlockdevCreateOptionsQcow *qcow_opts;
     int header_size, backing_filename_len, l1_size, shift, i;
     QCowHeader header;
     uint8_t *tmp;
     int64_t total_size = 0;
-    char *backing_file = NULL;
-    Error *local_err = NULL;
     int ret;
+    BlockDriverState *bs;
     BlockBackend *qcow_blk;
-    char *encryptfmt = NULL;
-    QDict *options;
-    QDict *encryptopts = NULL;
-    QCryptoBlockCreateOptions *crypto_opts = NULL;
     QCryptoBlock *crypto = NULL;
 
-    /* Read out options */
-    total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
-                          BDRV_SECTOR_SIZE);
+    assert(opts->driver == BLOCKDEV_DRIVER_QCOW);
+    qcow_opts = &opts->u.qcow;
+
+    /* Sanity checks */
+    total_size = qcow_opts->size;
     if (total_size == 0) {
         error_setg(errp, "Image size is too small, cannot be zero length");
-        ret = -EINVAL;
-        goto cleanup;
+        return -EINVAL;
     }
 
-    backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
-    encryptfmt = qemu_opt_get_del(opts, BLOCK_OPT_ENCRYPT_FORMAT);
-    if (encryptfmt) {
-        if (qemu_opt_get(opts, BLOCK_OPT_ENCRYPT)) {
-            error_setg(errp, "Options " BLOCK_OPT_ENCRYPT " and "
-                       BLOCK_OPT_ENCRYPT_FORMAT " are mutually exclusive");
-            ret = -EINVAL;
-            goto cleanup;
-        }
-    } else if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) {
-        encryptfmt = g_strdup("aes");
+    if (qcow_opts->has_encrypt &&
+        qcow_opts->encrypt->format != Q_CRYPTO_BLOCK_FORMAT_QCOW)
+    {
+        error_setg(errp, "Unsupported encryption format");
+        return -EINVAL;
     }
 
-    ret = bdrv_create_file(filename, opts, &local_err);
-    if (ret < 0) {
-        error_propagate(errp, local_err);
-        goto cleanup;
+    /* Create BlockBackend to write to the image */
+    bs = bdrv_open_blockdev_ref(qcow_opts->file, errp);
+    if (bs == NULL) {
+        return -EIO;
     }
 
-    qcow_blk = blk_new_open(filename, NULL, NULL,
-                            BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL,
-                            &local_err);
-    if (qcow_blk == NULL) {
-        error_propagate(errp, local_err);
-        ret = -EIO;
-        goto cleanup;
+    qcow_blk = blk_new(BLK_PERM_WRITE | BLK_PERM_RESIZE, BLK_PERM_ALL);
+    ret = blk_insert_bs(qcow_blk, bs, errp);
+    if (ret < 0) {
+        goto exit;
     }
-
     blk_set_allow_write_beyond_eof(qcow_blk, true);
 
+    /* Create image format */
     ret = blk_truncate(qcow_blk, 0, PREALLOC_MODE_OFF, errp);
     if (ret < 0) {
         goto exit;
@@ -877,16 +869,15 @@ static int coroutine_fn qcow_co_create_opts(const char *filename, QemuOpts *opts
     header.size = cpu_to_be64(total_size);
     header_size = sizeof(header);
     backing_filename_len = 0;
-    if (backing_file) {
-        if (strcmp(backing_file, "fat:")) {
+    if (qcow_opts->has_backing_file) {
+        if (strcmp(qcow_opts->backing_file, "fat:")) {
             header.backing_file_offset = cpu_to_be64(header_size);
-            backing_filename_len = strlen(backing_file);
+            backing_filename_len = strlen(qcow_opts->backing_file);
             header.backing_file_size = cpu_to_be32(backing_filename_len);
             header_size += backing_filename_len;
         } else {
             /* special backing file for vvfat */
-            g_free(backing_file);
-            backing_file = NULL;
+            qcow_opts->has_backing_file = false;
         }
         header.cluster_bits = 9; /* 512 byte cluster to avoid copying
                                     unmodified sectors */
@@ -901,26 +892,10 @@ static int coroutine_fn qcow_co_create_opts(const char *filename, QemuOpts *opts
 
     header.l1_table_offset = cpu_to_be64(header_size);
 
-    options = qemu_opts_to_qdict(opts, NULL);
-    qdict_extract_subqdict(options, &encryptopts, "encrypt.");
-    QDECREF(options);
-    if (encryptfmt) {
-        if (!g_str_equal(encryptfmt, "aes")) {
-            error_setg(errp, "Unknown encryption format '%s', expected 'aes'",
-                       encryptfmt);
-            ret = -EINVAL;
-            goto exit;
-        }
+    if (qcow_opts->has_encrypt) {
         header.crypt_method = cpu_to_be32(QCOW_CRYPT_AES);
 
-        crypto_opts = block_crypto_create_opts_init(
-            Q_CRYPTO_BLOCK_FORMAT_QCOW, encryptopts, errp);
-        if (!crypto_opts) {
-            ret = -EINVAL;
-            goto exit;
-        }
-
-        crypto = qcrypto_block_create(crypto_opts, "encrypt.",
+        crypto = qcrypto_block_create(qcow_opts->encrypt, "encrypt.",
                                       NULL, NULL, NULL, errp);
         if (!crypto) {
             ret = -EINVAL;
@@ -936,9 +911,9 @@ static int coroutine_fn qcow_co_create_opts(const char *filename, QemuOpts *opts
         goto exit;
     }
 
-    if (backing_file) {
+    if (qcow_opts->has_backing_file) {
         ret = blk_pwrite(qcow_blk, sizeof(header),
-                         backing_file, backing_filename_len, 0);
+                         qcow_opts->backing_file, backing_filename_len, 0);
         if (ret != backing_filename_len) {
             goto exit;
         }
@@ -959,12 +934,100 @@ static int coroutine_fn qcow_co_create_opts(const char *filename, QemuOpts *opts
     ret = 0;
 exit:
     blk_unref(qcow_blk);
-cleanup:
-    QDECREF(encryptopts);
-    g_free(encryptfmt);
     qcrypto_block_free(crypto);
-    qapi_free_QCryptoBlockCreateOptions(crypto_opts);
-    g_free(backing_file);
+    return ret;
+}
+
+static int coroutine_fn qcow_co_create_opts(const char *filename,
+                                            QemuOpts *opts, Error **errp)
+{
+    BlockdevCreateOptions *create_options = NULL;
+    BlockDriverState *bs = NULL;
+    QDict *qdict = NULL;
+    QObject *qobj;
+    Visitor *v;
+    const char *val;
+    Error *local_err = NULL;
+    int ret;
+
+    static const QDictRenames opt_renames[] = {
+        { BLOCK_OPT_BACKING_FILE,       "backing-file" },
+        { BLOCK_OPT_ENCRYPT,            BLOCK_OPT_ENCRYPT_FORMAT },
+        { NULL, NULL },
+    };
+
+    /* Parse options and convert legacy syntax */
+    qdict = qemu_opts_to_qdict_filtered(opts, NULL, &qcow_create_opts, true);
+
+    val = qdict_get_try_str(qdict, BLOCK_OPT_ENCRYPT);
+    if (val && !strcmp(val, "on")) {
+        qdict_put_str(qdict, BLOCK_OPT_ENCRYPT, "qcow");
+    } else if (val && !strcmp(val, "off")) {
+        qdict_del(qdict, BLOCK_OPT_ENCRYPT);
+    }
+
+    val = qdict_get_try_str(qdict, BLOCK_OPT_ENCRYPT_FORMAT);
+    if (val && !strcmp(val, "aes")) {
+        qdict_put_str(qdict, BLOCK_OPT_ENCRYPT_FORMAT, "qcow");
+    }
+
+    if (!qdict_rename_keys(qdict, opt_renames, errp)) {
+        ret = -EINVAL;
+        goto fail;
+    }
+
+    /* Create and open the file (protocol layer) */
+    ret = bdrv_create_file(filename, opts, &local_err);
+    if (ret < 0) {
+        error_propagate(errp, local_err);
+        goto fail;
+    }
+
+    bs = bdrv_open(filename, NULL, NULL,
+                   BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, errp);
+    if (bs == NULL) {
+        ret = -EIO;
+        goto fail;
+    }
+
+    /* Now get the QAPI type BlockdevCreateOptions */
+    qdict_put_str(qdict, "driver", "qcow");
+    qdict_put_str(qdict, "file", bs->node_name);
+
+    qobj = qdict_crumple(qdict, errp);
+    QDECREF(qdict);
+    qdict = qobject_to_qdict(qobj);
+    if (qdict == NULL) {
+        ret = -EINVAL;
+        goto fail;
+    }
+
+    v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
+    visit_type_BlockdevCreateOptions(v, NULL, &create_options, &local_err);
+    visit_free(v);
+
+    if (local_err) {
+        error_propagate(errp, local_err);
+        ret = -EINVAL;
+        goto fail;
+    }
+
+    /* Silently round up size */
+    assert(create_options->driver == BLOCKDEV_DRIVER_QCOW);
+    create_options->u.qcow.size =
+        ROUND_UP(create_options->u.qcow.size, BDRV_SECTOR_SIZE);
+
+    /* Create the qcow image (format layer) */
+    ret = qcow_co_create(create_options, errp);
+    if (ret < 0) {
+        goto fail;
+    }
+
+    ret = 0;
+fail:
+    QDECREF(qdict);
+    bdrv_unref(bs);
+    qapi_free_BlockdevCreateOptions(create_options);
     return ret;
 }
 
@@ -1128,6 +1191,7 @@ static BlockDriver bdrv_qcow = {
     .bdrv_close		= qcow_close,
     .bdrv_child_perm        = bdrv_format_default_perms,
     .bdrv_reopen_prepare    = qcow_reopen_prepare,
+    .bdrv_co_create         = qcow_co_create,
     .bdrv_co_create_opts    = qcow_co_create_opts,
     .bdrv_has_zero_init     = bdrv_has_zero_init_1,
     .supports_backing       = true,
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v2 4/8] qed: Support .bdrv_co_create
  2018-03-13 14:47 [Qemu-devel] [PATCH v2 0/8] block: .bdrv_co_create for format drivers Kevin Wolf
                   ` (2 preceding siblings ...)
  2018-03-13 14:47 ` [Qemu-devel] [PATCH v2 3/8] qcow: Support .bdrv_co_create Kevin Wolf
@ 2018-03-13 14:47 ` Kevin Wolf
  2018-03-13 14:47 ` [Qemu-devel] [PATCH v2 5/8] vdi: Make comments consistent with other drivers Kevin Wolf
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Kevin Wolf @ 2018-03-13 14:47 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, mreitz, den, jcody, eblake, qemu-devel

This adds the .bdrv_co_create driver callback to qed, which
enables image creation over QMP.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 qapi/block-core.json |  25 ++++++-
 block/qed.c          | 204 ++++++++++++++++++++++++++++++++++-----------------
 2 files changed, 162 insertions(+), 67 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 7b7d5a01fd..d091817855 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3703,6 +3703,29 @@
             '*refcount-bits':   'int' } }
 
 ##
+# @BlockdevCreateOptionsQed:
+#
+# Driver specific image creation options for qed.
+#
+# @file             Node to create the image format on
+# @size             Size of the virtual disk in bytes
+# @backing-file     File name of the backing file if a backing file
+#                   should be used
+# @backing-fmt      Name of the block driver to use for the backing file
+# @cluster-size     Cluster size in bytes (default: 65536)
+# @table-size       L1/L2 table size (in clusters)
+#
+# Since: 2.12
+##
+{ 'struct': 'BlockdevCreateOptionsQed',
+  'data': { 'file':             'BlockdevRef',
+            'size':             'size',
+            '*backing-file':    'str',
+            '*backing-fmt':     'BlockdevDriver',
+            '*cluster-size':    'size',
+            '*table-size':      'int' } }
+
+##
 # @BlockdevCreateOptionsRbd:
 #
 # Driver specific image creation options for rbd/Ceph.
@@ -3864,7 +3887,7 @@
       'parallels':      'BlockdevCreateOptionsParallels',
       'qcow':           'BlockdevCreateOptionsQcow',
       'qcow2':          'BlockdevCreateOptionsQcow2',
-      'qed':            'BlockdevCreateNotSupported',
+      'qed':            'BlockdevCreateOptionsQed',
       'quorum':         'BlockdevCreateNotSupported',
       'raw':            'BlockdevCreateNotSupported',
       'rbd':            'BlockdevCreateOptionsRbd',
diff --git a/block/qed.c b/block/qed.c
index 5e6a6bfaa0..46a84beeed 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -20,6 +20,11 @@
 #include "trace.h"
 #include "qed.h"
 #include "sysemu/block-backend.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qobject-input-visitor.h"
+#include "qapi/qapi-visit-block-core.h"
+
+static QemuOptsList qed_create_opts;
 
 static int bdrv_qed_probe(const uint8_t *buf, int buf_size,
                           const char *filename)
@@ -594,57 +599,95 @@ static void bdrv_qed_close(BlockDriverState *bs)
     qemu_vfree(s->l1_table);
 }
 
-static int qed_create(const char *filename, uint32_t cluster_size,
-                      uint64_t image_size, uint32_t table_size,
-                      const char *backing_file, const char *backing_fmt,
-                      QemuOpts *opts, Error **errp)
+static int coroutine_fn bdrv_qed_co_create(BlockdevCreateOptions *opts,
+                                           Error **errp)
 {
-    QEDHeader header = {
-        .magic = QED_MAGIC,
-        .cluster_size = cluster_size,
-        .table_size = table_size,
-        .header_size = 1,
-        .features = 0,
-        .compat_features = 0,
-        .l1_table_offset = cluster_size,
-        .image_size = image_size,
-    };
+    BlockdevCreateOptionsQed *qed_opts;
+    BlockBackend *blk = NULL;
+    BlockDriverState *bs = NULL;
+
+    QEDHeader header;
     QEDHeader le_header;
     uint8_t *l1_table = NULL;
-    size_t l1_size = header.cluster_size * header.table_size;
-    Error *local_err = NULL;
+    size_t l1_size;
     int ret = 0;
-    BlockBackend *blk;
 
-    ret = bdrv_create_file(filename, opts, &local_err);
-    if (ret < 0) {
-        error_propagate(errp, local_err);
-        return ret;
+    assert(opts->driver == BLOCKDEV_DRIVER_QED);
+    qed_opts = &opts->u.qed;
+
+    /* Validate options and set default values */
+    if (!qed_opts->has_cluster_size) {
+        qed_opts->cluster_size = QED_DEFAULT_CLUSTER_SIZE;
+    }
+    if (!qed_opts->has_table_size) {
+        qed_opts->table_size = QED_DEFAULT_TABLE_SIZE;
     }
 
-    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);
+    if (!qed_is_cluster_size_valid(qed_opts->cluster_size)) {
+        error_setg(errp, "QED cluster size must be within range [%u, %u] "
+                         "and power of 2",
+                   QED_MIN_CLUSTER_SIZE, QED_MAX_CLUSTER_SIZE);
+        return -EINVAL;
+    }
+    if (!qed_is_table_size_valid(qed_opts->table_size)) {
+        error_setg(errp, "QED table size must be within range [%u, %u] "
+                         "and power of 2",
+                   QED_MIN_TABLE_SIZE, QED_MAX_TABLE_SIZE);
+        return -EINVAL;
+    }
+    if (!qed_is_image_size_valid(qed_opts->size, qed_opts->cluster_size,
+                                 qed_opts->table_size))
+    {
+        error_setg(errp, "QED image size must be a non-zero multiple of "
+                         "cluster size and less than %" PRIu64 " bytes",
+                   qed_max_image_size(qed_opts->cluster_size,
+                                      qed_opts->table_size));
+        return -EINVAL;
+    }
+
+    /* Create BlockBackend to write to the image */
+    bs = bdrv_open_blockdev_ref(qed_opts->file, errp);
+    if (bs == NULL) {
         return -EIO;
     }
 
+    blk = blk_new(BLK_PERM_WRITE | BLK_PERM_RESIZE, BLK_PERM_ALL);
+    ret = blk_insert_bs(blk, bs, errp);
+    if (ret < 0) {
+        goto out;
+    }
     blk_set_allow_write_beyond_eof(blk, true);
 
+    /* Prepare image format */
+    header = (QEDHeader) {
+        .magic = QED_MAGIC,
+        .cluster_size = qed_opts->cluster_size,
+        .table_size = qed_opts->table_size,
+        .header_size = 1,
+        .features = 0,
+        .compat_features = 0,
+        .l1_table_offset = qed_opts->cluster_size,
+        .image_size = qed_opts->size,
+    };
+
+    l1_size = header.cluster_size * header.table_size;
+
     /* File must start empty and grow, check truncate is supported */
     ret = blk_truncate(blk, 0, PREALLOC_MODE_OFF, errp);
     if (ret < 0) {
         goto out;
     }
 
-    if (backing_file) {
+    if (qed_opts->has_backing_file) {
         header.features |= QED_F_BACKING_FILE;
         header.backing_filename_offset = sizeof(le_header);
-        header.backing_filename_size = strlen(backing_file);
+        header.backing_filename_size = strlen(qed_opts->backing_file);
 
-        if (qed_fmt_is_raw(backing_fmt)) {
-            header.features |= QED_F_BACKING_FORMAT_NO_PROBE;
+        if (qed_opts->has_backing_fmt) {
+            const char *backing_fmt = BlockdevDriver_str(qed_opts->backing_fmt);
+            if (qed_fmt_is_raw(backing_fmt)) {
+                header.features |= QED_F_BACKING_FORMAT_NO_PROBE;
+            }
         }
     }
 
@@ -653,7 +696,7 @@ static int qed_create(const char *filename, uint32_t cluster_size,
     if (ret < 0) {
         goto out;
     }
-    ret = blk_pwrite(blk, sizeof(le_header), backing_file,
+    ret = blk_pwrite(blk, sizeof(le_header), qed_opts->backing_file,
                      header.backing_filename_size, 0);
     if (ret < 0) {
         goto out;
@@ -669,6 +712,7 @@ static int qed_create(const char *filename, uint32_t cluster_size,
 out:
     g_free(l1_table);
     blk_unref(blk);
+    bdrv_unref(bs);
     return ret;
 }
 
@@ -676,51 +720,78 @@ static int coroutine_fn bdrv_qed_co_create_opts(const char *filename,
                                                 QemuOpts *opts,
                                                 Error **errp)
 {
-    uint64_t image_size = 0;
-    uint32_t cluster_size = QED_DEFAULT_CLUSTER_SIZE;
-    uint32_t table_size = QED_DEFAULT_TABLE_SIZE;
-    char *backing_file = NULL;
-    char *backing_fmt = NULL;
+    BlockdevCreateOptions *create_options = NULL;
+    QDict *qdict = NULL;
+    QObject *qobj;
+    Visitor *v;
+    BlockDriverState *bs = NULL;
+    Error *local_err = NULL;
     int ret;
 
-    image_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
-                          BDRV_SECTOR_SIZE);
-    backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
-    backing_fmt = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FMT);
-    cluster_size = qemu_opt_get_size_del(opts,
-                                         BLOCK_OPT_CLUSTER_SIZE,
-                                         QED_DEFAULT_CLUSTER_SIZE);
-    table_size = qemu_opt_get_size_del(opts, BLOCK_OPT_TABLE_SIZE,
-                                       QED_DEFAULT_TABLE_SIZE);
-
-    if (!qed_is_cluster_size_valid(cluster_size)) {
-        error_setg(errp, "QED cluster size must be within range [%u, %u] "
-                         "and power of 2",
-                   QED_MIN_CLUSTER_SIZE, QED_MAX_CLUSTER_SIZE);
+    static const QDictRenames opt_renames[] = {
+        { BLOCK_OPT_BACKING_FILE,       "backing-file" },
+        { BLOCK_OPT_BACKING_FMT,        "backing-fmt" },
+        { BLOCK_OPT_CLUSTER_SIZE,       "cluster-size" },
+        { BLOCK_OPT_TABLE_SIZE,         "table-size" },
+        { NULL, NULL },
+    };
+
+    /* Parse options and convert legacy syntax */
+    qdict = qemu_opts_to_qdict_filtered(opts, NULL, &qed_create_opts, true);
+
+    if (!qdict_rename_keys(qdict, opt_renames, errp)) {
         ret = -EINVAL;
-        goto finish;
+        goto fail;
     }
-    if (!qed_is_table_size_valid(table_size)) {
-        error_setg(errp, "QED table size must be within range [%u, %u] "
-                         "and power of 2",
-                   QED_MIN_TABLE_SIZE, QED_MAX_TABLE_SIZE);
+
+    /* Create and open the file (protocol layer) */
+    ret = bdrv_create_file(filename, opts, &local_err);
+    if (ret < 0) {
+        error_propagate(errp, local_err);
+        goto fail;
+    }
+
+    bs = bdrv_open(filename, NULL, NULL,
+                   BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, errp);
+    if (bs == NULL) {
+        ret = -EIO;
+        goto fail;
+    }
+
+    /* Now get the QAPI type BlockdevCreateOptions */
+    qdict_put_str(qdict, "driver", "qed");
+    qdict_put_str(qdict, "file", bs->node_name);
+
+    qobj = qdict_crumple(qdict, errp);
+    QDECREF(qdict);
+    qdict = qobject_to_qdict(qobj);
+    if (qdict == NULL) {
         ret = -EINVAL;
-        goto finish;
+        goto fail;
     }
-    if (!qed_is_image_size_valid(image_size, cluster_size, table_size)) {
-        error_setg(errp, "QED image size must be a non-zero multiple of "
-                         "cluster size and less than %" PRIu64 " bytes",
-                   qed_max_image_size(cluster_size, table_size));
+
+    v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
+    visit_type_BlockdevCreateOptions(v, NULL, &create_options, &local_err);
+    visit_free(v);
+
+    if (local_err) {
+        error_propagate(errp, local_err);
         ret = -EINVAL;
-        goto finish;
+        goto fail;
     }
 
-    ret = qed_create(filename, cluster_size, image_size, table_size,
-                     backing_file, backing_fmt, opts, errp);
+    /* Silently round up size */
+    assert(create_options->driver == BLOCKDEV_DRIVER_QED);
+    create_options->u.qed.size =
+        ROUND_UP(create_options->u.qed.size, BDRV_SECTOR_SIZE);
+
+    /* Create the qed image (format layer) */
+    ret = bdrv_qed_co_create(create_options, errp);
 
-finish:
-    g_free(backing_file);
-    g_free(backing_fmt);
+fail:
+    QDECREF(qdict);
+    bdrv_unref(bs);
+    qapi_free_BlockdevCreateOptions(create_options);
     return ret;
 }
 
@@ -1602,6 +1673,7 @@ static BlockDriver bdrv_qed = {
     .bdrv_close               = bdrv_qed_close,
     .bdrv_reopen_prepare      = bdrv_qed_reopen_prepare,
     .bdrv_child_perm          = bdrv_format_default_perms,
+    .bdrv_co_create           = bdrv_qed_co_create,
     .bdrv_co_create_opts      = bdrv_qed_co_create_opts,
     .bdrv_has_zero_init       = bdrv_has_zero_init_1,
     .bdrv_co_block_status     = bdrv_qed_co_block_status,
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v2 5/8] vdi: Make comments consistent with other drivers
  2018-03-13 14:47 [Qemu-devel] [PATCH v2 0/8] block: .bdrv_co_create for format drivers Kevin Wolf
                   ` (3 preceding siblings ...)
  2018-03-13 14:47 ` [Qemu-devel] [PATCH v2 4/8] qed: " Kevin Wolf
@ 2018-03-13 14:47 ` Kevin Wolf
  2018-03-13 14:55   ` Max Reitz
  2018-03-13 14:47 ` [Qemu-devel] [PATCH v2 6/8] vhdx: Support .bdrv_co_create Kevin Wolf
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 14+ messages in thread
From: Kevin Wolf @ 2018-03-13 14:47 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, mreitz, den, jcody, eblake, qemu-devel

This makes the .bdrv_co_create(_opts) implementation of vdi look more
like the other recently converted block drivers.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/vdi.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/block/vdi.c b/block/vdi.c
index 8132e3adfe..d939b034c4 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -742,7 +742,7 @@ static int coroutine_fn vdi_co_do_create(BlockdevCreateOptions *create_options,
 
     logout("\n");
 
-    /* Read out options. */
+    /* Validate options and set default values */
     bytes = vdi_opts->size;
     if (vdi_opts->q_static) {
         image_type = VDI_TYPE_STATIC;
@@ -772,6 +772,7 @@ static int coroutine_fn vdi_co_do_create(BlockdevCreateOptions *create_options,
         goto exit;
     }
 
+    /* Create BlockBackend to write to the image */
     bs_file = bdrv_open_blockdev_ref(vdi_opts->file, errp);
     if (!bs_file) {
         ret = -EIO;
@@ -877,7 +878,9 @@ static int coroutine_fn vdi_co_create_opts(const char *filename, QemuOpts *opts,
     Error *local_err = NULL;
     int ret;
 
-    /* Since CONFIG_VDI_BLOCK_SIZE is disabled by default,
+    /* Parse options and convert legacy syntax.
+     *
+     * Since CONFIG_VDI_BLOCK_SIZE is disabled by default,
      * cluster-size is not part of the QAPI schema; therefore we have
      * to parse it before creating the QAPI object. */
 #if defined(CONFIG_VDI_BLOCK_SIZE)
@@ -895,6 +898,7 @@ static int coroutine_fn vdi_co_create_opts(const char *filename, QemuOpts *opts,
 
     qdict = qemu_opts_to_qdict_filtered(opts, NULL, &vdi_create_opts, true);
 
+    /* Create and open the file (protocol layer) */
     ret = bdrv_create_file(filename, opts, errp);
     if (ret < 0) {
         goto done;
@@ -921,10 +925,12 @@ static int coroutine_fn vdi_co_create_opts(const char *filename, QemuOpts *opts,
         goto done;
     }
 
+    /* Silently round up size */
     assert(create_options->driver == BLOCKDEV_DRIVER_VDI);
     create_options->u.vdi.size = ROUND_UP(create_options->u.vdi.size,
                                           BDRV_SECTOR_SIZE);
 
+    /* Create the vdi image (format layer) */
     ret = vdi_co_do_create(create_options, block_size, errp);
 done:
     QDECREF(qdict);
@@ -981,8 +987,8 @@ static BlockDriver bdrv_vdi = {
     .bdrv_close = vdi_close,
     .bdrv_reopen_prepare = vdi_reopen_prepare,
     .bdrv_child_perm          = bdrv_format_default_perms,
-    .bdrv_co_create_opts = vdi_co_create_opts,
     .bdrv_co_create      = vdi_co_create,
+    .bdrv_co_create_opts = vdi_co_create_opts,
     .bdrv_has_zero_init = bdrv_has_zero_init_1,
     .bdrv_co_block_status = vdi_co_block_status,
     .bdrv_make_empty = vdi_make_empty,
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v2 6/8] vhdx: Support .bdrv_co_create
  2018-03-13 14:47 [Qemu-devel] [PATCH v2 0/8] block: .bdrv_co_create for format drivers Kevin Wolf
                   ` (4 preceding siblings ...)
  2018-03-13 14:47 ` [Qemu-devel] [PATCH v2 5/8] vdi: Make comments consistent with other drivers Kevin Wolf
@ 2018-03-13 14:47 ` Kevin Wolf
  2018-03-13 14:59   ` Max Reitz
  2018-03-13 14:47 ` [Qemu-devel] [PATCH v2 7/8] vpc: " Kevin Wolf
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 14+ messages in thread
From: Kevin Wolf @ 2018-03-13 14:47 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, mreitz, den, jcody, eblake, qemu-devel

This adds the .bdrv_co_create driver callback to vhdx, which
enables image creation over QMP.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qapi/block-core.json |  40 +++++++++-
 block/vhdx.c         | 216 ++++++++++++++++++++++++++++++++++++++-------------
 2 files changed, 203 insertions(+), 53 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index d091817855..350094f46a 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3842,6 +3842,44 @@
             '*static':          'bool' } }
 
 ##
+# @BlockdevVhdxSubformat:
+#
+# @dynamic: Growing image file
+# @fixed:   Preallocated fixed-size image 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, must be a multiple of 1 MB
+#                   (default: 1 MB)
+# @block-size       Block size in bytes, must be a multiple of 1 MB and not
+#                   larger than 256 MB (default: automatically choose a block
+#                   size depending on the image size)
+# @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.
@@ -3896,7 +3934,7 @@
       'ssh':            'BlockdevCreateOptionsSsh',
       'throttle':       'BlockdevCreateNotSupported',
       'vdi':            'BlockdevCreateOptionsVdi',
-      'vhdx':           'BlockdevCreateNotSupported',
+      'vhdx':           'BlockdevCreateOptionsVhdx',
       'vmdk':           'BlockdevCreateNotSupported',
       'vpc':            'BlockdevCreateNotSupported',
       'vvfat':          'BlockdevCreateNotSupported',
diff --git a/block/vhdx.c b/block/vhdx.c
index d82350d07c..f1b97f4b49 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -26,6 +26,9 @@
 #include "block/vhdx.h"
 #include "migration/blocker.h"
 #include "qemu/uuid.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qobject-input-visitor.h"
+#include "qapi/qapi-visit-block-core.h"
 
 /* Options for VHDX creation */
 
@@ -39,6 +42,8 @@ typedef enum VHDXImageType {
     VHDX_TYPE_DIFFERENCING,   /* Currently unsupported */
 } VHDXImageType;
 
+static QemuOptsList vhdx_create_opts;
+
 /* Several metadata and region table data entries are identified by
  * guids in  a MS-specific GUID format. */
 
@@ -1792,59 +1797,71 @@ 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)
 {
+    BlockdevCreateOptionsVhdx *vhdx_opts;
+    BlockBackend *blk = NULL;
+    BlockDriverState *bs = NULL;
+
     int ret = 0;
-    uint64_t image_size = (uint64_t) 2 * GiB;
-    uint32_t log_size   = 1 * MiB;
-    uint32_t block_size = 0;
+    uint64_t image_size;
+    uint32_t log_size;
+    uint32_t block_size;
     uint64_t signature;
     uint64_t metadata_offset;
     bool use_zero_blocks = false;
 
     gunichar2 *creator = NULL;
     glong creator_items;
-    BlockBackend *blk;
-    char *type = NULL;
     VHDXImageType image_type;
-    Error *local_err = NULL;
 
-    image_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
-                          BDRV_SECTOR_SIZE);
-    log_size = qemu_opt_get_size_del(opts, VHDX_BLOCK_OPT_LOG_SIZE, 0);
-    block_size = qemu_opt_get_size_del(opts, VHDX_BLOCK_OPT_BLOCK_SIZE, 0);
-    type = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT);
-    use_zero_blocks = qemu_opt_get_bool_del(opts, VHDX_BLOCK_OPT_ZERO, true);
+    assert(opts->driver == BLOCKDEV_DRIVER_VHDX);
+    vhdx_opts = &opts->u.vhdx;
 
+    /* Validate options and set default values */
+    image_size = vhdx_opts->size;
     if (image_size > VHDX_MAX_IMAGE_SIZE) {
         error_setg_errno(errp, EINVAL, "Image size too large; max of 64TB");
-        ret = -EINVAL;
-        goto exit;
+        return -EINVAL;
     }
 
-    if (type == NULL) {
-        type = g_strdup("dynamic");
+    if (!vhdx_opts->has_log_size) {
+        log_size = DEFAULT_LOG_SIZE;
+    } else {
+        log_size = vhdx_opts->log_size;
+    }
+    if (log_size < MiB || (log_size % MiB) != 0) {
+        error_setg_errno(errp, EINVAL, "Log size must be a multiple of 1 MB");
+        return -EINVAL;
     }
 
-    if (!strcmp(type, "dynamic")) {
+    if (!vhdx_opts->has_block_state_zero) {
+        use_zero_blocks = true;
+    } else {
+        use_zero_blocks = vhdx_opts->block_state_zero;
+    }
+
+    if (!vhdx_opts->has_subformat) {
+        vhdx_opts->subformat = BLOCKDEV_VHDX_SUBFORMAT_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();
     }
 
     /* These are pretty arbitrary, and mainly designed to keep the BAT
      * size reasonable to load into RAM */
-    if (block_size == 0) {
+    if (vhdx_opts->has_block_size) {
+        block_size = vhdx_opts->block_size;
+    } else {
         if (image_size > 32 * TiB) {
             block_size = 64 * MiB;
         } else if (image_size > (uint64_t) 100 * GiB) {
@@ -1856,30 +1873,27 @@ static int coroutine_fn vhdx_co_create_opts(const char *filename, QemuOpts *opts
         }
     }
 
-
-    /* make the log size close to what was specified, but must be
-     * min 1MB, and multiple of 1MB */
-    log_size = ROUND_UP(log_size, MiB);
-
-    block_size = ROUND_UP(block_size, MiB);
-    block_size = block_size > VHDX_BLOCK_SIZE_MAX ? VHDX_BLOCK_SIZE_MAX :
-                                                    block_size;
-
-    ret = bdrv_create_file(filename, opts, &local_err);
-    if (ret < 0) {
-        error_propagate(errp, local_err);
-        goto exit;
+    if (block_size < MiB || (block_size % MiB) != 0) {
+        error_setg_errno(errp, EINVAL, "Block size must be a multiple of 1 MB");
+        return -EINVAL;
+    }
+    if (block_size > VHDX_BLOCK_SIZE_MAX) {
+        error_setg_errno(errp, EINVAL, "Block size must not exceed %d",
+                         VHDX_BLOCK_SIZE_MAX);
+        return -EINVAL;
     }
 
-    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;
+    /* Create BlockBackend to write to the image */
+    bs = bdrv_open_blockdev_ref(vhdx_opts->file, errp);
+    if (bs == NULL) {
+        return -EIO;
     }
 
+    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) */
@@ -1931,12 +1945,109 @@ static int coroutine_fn vhdx_co_create_opts(const char *filename, QemuOpts *opts
 
 delete_and_exit:
     blk_unref(blk);
-exit:
-    g_free(type);
+    bdrv_unref(bs);
     g_free(creator);
     return ret;
 }
 
+static int coroutine_fn vhdx_co_create_opts(const char *filename,
+                                            QemuOpts *opts,
+                                            Error **errp)
+{
+    BlockdevCreateOptions *create_options = NULL;
+    QDict *qdict = NULL;
+    QObject *qobj;
+    Visitor *v;
+    BlockDriverState *bs = NULL;
+    Error *local_err = NULL;
+    int ret;
+
+    static const QDictRenames opt_renames[] = {
+        { VHDX_BLOCK_OPT_LOG_SIZE,      "log-size" },
+        { VHDX_BLOCK_OPT_BLOCK_SIZE,    "block-size" },
+        { VHDX_BLOCK_OPT_ZERO,          "block-state-zero" },
+        { NULL, NULL },
+    };
+
+    /* Parse options and convert legacy syntax */
+    qdict = qemu_opts_to_qdict_filtered(opts, NULL, &vhdx_create_opts, true);
+
+    if (!qdict_rename_keys(qdict, opt_renames, errp)) {
+        ret = -EINVAL;
+        goto fail;
+    }
+
+    /* Create and open the file (protocol layer) */
+    ret = bdrv_create_file(filename, opts, &local_err);
+    if (ret < 0) {
+        error_propagate(errp, local_err);
+        goto fail;
+    }
+
+    bs = bdrv_open(filename, NULL, NULL,
+                   BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, errp);
+    if (bs == NULL) {
+        ret = -EIO;
+        goto fail;
+    }
+
+    /* Now get the QAPI type BlockdevCreateOptions */
+    qdict_put_str(qdict, "driver", "vhdx");
+    qdict_put_str(qdict, "file", bs->node_name);
+
+    qobj = qdict_crumple(qdict, errp);
+    QDECREF(qdict);
+    qdict = qobject_to_qdict(qobj);
+    if (qdict == NULL) {
+        ret = -EINVAL;
+        goto fail;
+    }
+
+    v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
+    visit_type_BlockdevCreateOptions(v, NULL, &create_options, &local_err);
+    visit_free(v);
+
+    if (local_err) {
+        error_propagate(errp, local_err);
+        ret = -EINVAL;
+        goto fail;
+    }
+
+    /* Silently round up sizes:
+     * The image size is rounded to 512 bytes. Make the block and log size
+     * close to what was specified, but must be at least 1MB, and a multiple of
+     * 1 MB. Also respect VHDX_BLOCK_SIZE_MAX for block sizes. block_size = 0
+     * means auto, which is represented by a missing key in QAPI. */
+    assert(create_options->driver == BLOCKDEV_DRIVER_VHDX);
+    create_options->u.vhdx.size =
+        ROUND_UP(create_options->u.vhdx.size, BDRV_SECTOR_SIZE);
+
+    if (create_options->u.vhdx.has_log_size) {
+        create_options->u.vhdx.log_size =
+            ROUND_UP(create_options->u.vhdx.log_size, MiB);
+    }
+    if (create_options->u.vhdx.has_block_size) {
+        create_options->u.vhdx.block_size =
+            ROUND_UP(create_options->u.vhdx.block_size, MiB);
+
+        if (create_options->u.vhdx.block_size == 0) {
+            create_options->u.vhdx.has_block_size = false;
+        }
+        if (create_options->u.vhdx.block_size > VHDX_BLOCK_SIZE_MAX) {
+            create_options->u.vhdx.block_size = VHDX_BLOCK_SIZE_MAX;
+        }
+    }
+
+    /* Create the vhdx image (format layer) */
+    ret = vhdx_co_create(create_options, errp);
+
+fail:
+    QDECREF(qdict);
+    bdrv_unref(bs);
+    qapi_free_BlockdevCreateOptions(create_options);
+    return ret;
+}
+
 /* If opened r/w, the VHDX driver will automatically replay the log,
  * if one is present, inside the vhdx_open() call.
  *
@@ -2005,6 +2116,7 @@ static BlockDriver bdrv_vhdx = {
     .bdrv_child_perm        = bdrv_format_default_perms,
     .bdrv_co_readv          = vhdx_co_readv,
     .bdrv_co_writev         = vhdx_co_writev,
+    .bdrv_co_create         = vhdx_co_create,
     .bdrv_co_create_opts    = vhdx_co_create_opts,
     .bdrv_get_info          = vhdx_get_info,
     .bdrv_co_check          = vhdx_co_check,
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v2 7/8] vpc: Support .bdrv_co_create
  2018-03-13 14:47 [Qemu-devel] [PATCH v2 0/8] block: .bdrv_co_create for format drivers Kevin Wolf
                   ` (5 preceding siblings ...)
  2018-03-13 14:47 ` [Qemu-devel] [PATCH v2 6/8] vhdx: Support .bdrv_co_create Kevin Wolf
@ 2018-03-13 14:47 ` Kevin Wolf
  2018-03-13 15:00   ` Max Reitz
  2018-03-13 14:47 ` [Qemu-devel] [PATCH v2 8/8] vpc: Require aligned size in .bdrv_co_create Kevin Wolf
  2018-03-13 15:12 ` [Qemu-devel] [PATCH v2 0/8] block: .bdrv_co_create for format drivers Kevin Wolf
  8 siblings, 1 reply; 14+ messages in thread
From: Kevin Wolf @ 2018-03-13 14:47 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, mreitz, den, jcody, eblake, qemu-devel

This adds the .bdrv_co_create driver callback to vpc, which
enables image creation over QMP.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qapi/block-core.json |  33 ++++++++++-
 block/vpc.c          | 152 ++++++++++++++++++++++++++++++++++++++-------------
 2 files changed, 147 insertions(+), 38 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 350094f46a..47ff5f8ce5 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3880,6 +3880,37 @@
             '*block-state-zero':    'bool' } }
 
 ##
+# @BlockdevVpcSubformat:
+#
+# @dynamic: Growing image file
+# @fixed:   Preallocated fixed-size image file
+#
+# Since: 2.12
+##
+{ 'enum': 'BlockdevVpcSubformat',
+  'data': [ 'dynamic', 'fixed' ] }
+
+##
+# @BlockdevCreateOptionsVpc:
+#
+# Driver specific image creation options for vpc (VHD).
+#
+# @file             Node to create the image format on
+# @size             Size of the virtual disk in bytes
+# @subformat        vhdx subformat (default: dynamic)
+# @force-size       Force use of the exact byte size instead of rounding to the
+#                   next size that can be represented in CHS geometry
+#                   (default: false)
+#
+# Since: 2.12
+##
+{ 'struct': 'BlockdevCreateOptionsVpc',
+  'data': { 'file':                 'BlockdevRef',
+            'size':                 'size',
+            '*subformat':           'BlockdevVpcSubformat',
+            '*force-size':          'bool' } }
+
+##
 # @BlockdevCreateNotSupported:
 #
 # This is used for all drivers that don't support creating images.
@@ -3936,7 +3967,7 @@
       'vdi':            'BlockdevCreateOptionsVdi',
       'vhdx':           'BlockdevCreateOptionsVhdx',
       'vmdk':           'BlockdevCreateNotSupported',
-      'vpc':            'BlockdevCreateNotSupported',
+      'vpc':            'BlockdevCreateOptionsVpc',
       'vvfat':          'BlockdevCreateNotSupported',
       'vxhs':           'BlockdevCreateNotSupported'
   } }
diff --git a/block/vpc.c b/block/vpc.c
index b2e2b9ebd4..8824211713 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -32,6 +32,9 @@
 #include "migration/blocker.h"
 #include "qemu/bswap.h"
 #include "qemu/uuid.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qobject-input-visitor.h"
+#include "qapi/qapi-visit-block-core.h"
 
 /**************************************************************/
 
@@ -166,6 +169,8 @@ static QemuOptsList vpc_runtime_opts = {
     }
 };
 
+static QemuOptsList vpc_create_opts;
+
 static uint32_t vpc_checksum(uint8_t* buf, size_t size)
 {
     uint32_t res = 0;
@@ -897,12 +902,15 @@ static int create_fixed_disk(BlockBackend *blk, uint8_t *buf,
     return ret;
 }
 
-static int coroutine_fn vpc_co_create_opts(const char *filename, QemuOpts *opts,
-                                           Error **errp)
+static int coroutine_fn vpc_co_create(BlockdevCreateOptions *opts,
+                                      Error **errp)
 {
+    BlockdevCreateOptionsVpc *vpc_opts;
+    BlockBackend *blk = NULL;
+    BlockDriverState *bs = NULL;
+
     uint8_t buf[1024];
     VHDFooter *footer = (VHDFooter *) buf;
-    char *disk_type_param;
     int i;
     uint16_t cyls = 0;
     uint8_t heads = 0;
@@ -911,45 +919,38 @@ static int coroutine_fn vpc_co_create_opts(const char *filename, QemuOpts *opts,
     int64_t total_size;
     int disk_type;
     int ret = -EIO;
-    bool force_size;
-    Error *local_err = NULL;
-    BlockBackend *blk = NULL;
 
-    /* Read out options */
-    total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
-                          BDRV_SECTOR_SIZE);
-    disk_type_param = qemu_opt_get_del(opts, BLOCK_OPT_SUBFMT);
-    if (disk_type_param) {
-        if (!strcmp(disk_type_param, "dynamic")) {
-            disk_type = VHD_DYNAMIC;
-        } else if (!strcmp(disk_type_param, "fixed")) {
-            disk_type = VHD_FIXED;
-        } else {
-            error_setg(errp, "Invalid disk type, %s", disk_type_param);
-            ret = -EINVAL;
-            goto out;
-        }
-    } else {
+    assert(opts->driver == BLOCKDEV_DRIVER_VPC);
+    vpc_opts = &opts->u.vpc;
+
+    /* Validate options and set default values */
+    total_size = vpc_opts->size;
+
+    if (!vpc_opts->has_subformat) {
+        vpc_opts->subformat = BLOCKDEV_VPC_SUBFORMAT_DYNAMIC;
+    }
+    switch (vpc_opts->subformat) {
+    case BLOCKDEV_VPC_SUBFORMAT_DYNAMIC:
         disk_type = VHD_DYNAMIC;
+        break;
+    case BLOCKDEV_VPC_SUBFORMAT_FIXED:
+        disk_type = VHD_FIXED;
+        break;
+    default:
+        g_assert_not_reached();
     }
 
-    force_size = qemu_opt_get_bool_del(opts, VPC_OPT_FORCE_SIZE, false);
-
-    ret = bdrv_create_file(filename, opts, &local_err);
-    if (ret < 0) {
-        error_propagate(errp, local_err);
-        goto out;
+    /* Create BlockBackend to write to the image */
+    bs = bdrv_open_blockdev_ref(vpc_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;
+    blk = blk_new(BLK_PERM_WRITE | BLK_PERM_RESIZE, BLK_PERM_ALL);
+    ret = blk_insert_bs(blk, bs, errp);
+    if (ret < 0) {
         goto out;
     }
-
     blk_set_allow_write_beyond_eof(blk, true);
 
     /*
@@ -961,7 +962,7 @@ static int coroutine_fn vpc_co_create_opts(const char *filename, QemuOpts *opts,
      * we set the geometry to 65535 x 16 x 255 (CxHxS) sectors and use
      * the image size from the VHD footer to calculate total_sectors.
      */
-    if (force_size) {
+    if (vpc_opts->force_size) {
         /* This will force the use of total_size for sector count, below */
         cyls         = VHD_CHS_MAX_C;
         heads        = VHD_CHS_MAX_H;
@@ -990,7 +991,7 @@ static int coroutine_fn vpc_co_create_opts(const char *filename, QemuOpts *opts,
     memset(buf, 0, 1024);
 
     memcpy(footer->creator, "conectix", 8);
-    if (force_size) {
+    if (vpc_opts->force_size) {
         memcpy(footer->creator_app, "qem2", 4);
     } else {
         memcpy(footer->creator_app, "qemu", 4);
@@ -1032,10 +1033,86 @@ static int coroutine_fn vpc_co_create_opts(const char *filename, QemuOpts *opts,
 
 out:
     blk_unref(blk);
-    g_free(disk_type_param);
+    bdrv_unref(bs);
+    return ret;
+}
+
+static int coroutine_fn vpc_co_create_opts(const char *filename,
+                                           QemuOpts *opts, Error **errp)
+{
+    BlockdevCreateOptions *create_options = NULL;
+    QDict *qdict = NULL;
+    QObject *qobj;
+    Visitor *v;
+    BlockDriverState *bs = NULL;
+    Error *local_err = NULL;
+    int ret;
+
+    static const QDictRenames opt_renames[] = {
+        { VPC_OPT_FORCE_SIZE,           "force-size" },
+        { NULL, NULL },
+    };
+
+    /* Parse options and convert legacy syntax */
+    qdict = qemu_opts_to_qdict_filtered(opts, NULL, &vpc_create_opts, true);
+
+    if (!qdict_rename_keys(qdict, opt_renames, errp)) {
+        ret = -EINVAL;
+        goto fail;
+    }
+
+    /* Create and open the file (protocol layer) */
+    ret = bdrv_create_file(filename, opts, &local_err);
+    if (ret < 0) {
+        error_propagate(errp, local_err);
+        goto fail;
+    }
+
+    bs = bdrv_open(filename, NULL, NULL,
+                   BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, errp);
+    if (bs == NULL) {
+        ret = -EIO;
+        goto fail;
+    }
+
+    /* Now get the QAPI type BlockdevCreateOptions */
+    qdict_put_str(qdict, "driver", "vpc");
+    qdict_put_str(qdict, "file", bs->node_name);
+
+    qobj = qdict_crumple(qdict, errp);
+    QDECREF(qdict);
+    qdict = qobject_to_qdict(qobj);
+    if (qdict == NULL) {
+        ret = -EINVAL;
+        goto fail;
+    }
+
+    v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
+    visit_type_BlockdevCreateOptions(v, NULL, &create_options, &local_err);
+    visit_free(v);
+
+    if (local_err) {
+        error_propagate(errp, local_err);
+        ret = -EINVAL;
+        goto fail;
+    }
+
+    /* Silently round up size */
+    assert(create_options->driver == BLOCKDEV_DRIVER_VPC);
+    create_options->u.vpc.size =
+        ROUND_UP(create_options->u.vpc.size, BDRV_SECTOR_SIZE);
+
+    /* Create the vpc image (format layer) */
+    ret = vpc_co_create(create_options, errp);
+
+fail:
+    QDECREF(qdict);
+    bdrv_unref(bs);
+    qapi_free_BlockdevCreateOptions(create_options);
     return ret;
 }
 
+
 static int vpc_has_zero_init(BlockDriverState *bs)
 {
     BDRVVPCState *s = bs->opaque;
@@ -1096,6 +1173,7 @@ static BlockDriver bdrv_vpc = {
     .bdrv_close             = vpc_close,
     .bdrv_reopen_prepare    = vpc_reopen_prepare,
     .bdrv_child_perm        = bdrv_format_default_perms,
+    .bdrv_co_create         = vpc_co_create,
     .bdrv_co_create_opts    = vpc_co_create_opts,
 
     .bdrv_co_preadv             = vpc_co_preadv,
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v2 8/8] vpc: Require aligned size in .bdrv_co_create
  2018-03-13 14:47 [Qemu-devel] [PATCH v2 0/8] block: .bdrv_co_create for format drivers Kevin Wolf
                   ` (6 preceding siblings ...)
  2018-03-13 14:47 ` [Qemu-devel] [PATCH v2 7/8] vpc: " Kevin Wolf
@ 2018-03-13 14:47 ` Kevin Wolf
  2018-03-13 15:04   ` Max Reitz
  2018-03-13 15:12 ` [Qemu-devel] [PATCH v2 0/8] block: .bdrv_co_create for format drivers Kevin Wolf
  8 siblings, 1 reply; 14+ messages in thread
From: Kevin Wolf @ 2018-03-13 14:47 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, mreitz, den, jcody, eblake, qemu-devel

Perform the rounding to match a CHS geometry only in the legacy code
path in .bdrv_co_create_opts. QMP now requires that the user already
passes a CHS aligned image size, unless force-size=true is given.

CHS alignment is required to make the image compatible with Virtual PC,
but not for use with newer Microsoft hypervisors.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/vpc.c | 113 +++++++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 82 insertions(+), 31 deletions(-)

diff --git a/block/vpc.c b/block/vpc.c
index 8824211713..28ffa0d2f8 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -902,6 +902,62 @@ static int create_fixed_disk(BlockBackend *blk, uint8_t *buf,
     return ret;
 }
 
+static int calculate_rounded_image_size(BlockdevCreateOptionsVpc *vpc_opts,
+                                        uint16_t *out_cyls,
+                                        uint8_t *out_heads,
+                                        uint8_t *out_secs_per_cyl,
+                                        int64_t *out_total_sectors,
+                                        Error **errp)
+{
+    int64_t total_size = vpc_opts->size;
+    uint16_t cyls = 0;
+    uint8_t heads = 0;
+    uint8_t secs_per_cyl = 0;
+    int64_t total_sectors;
+    int i;
+
+    /*
+     * Calculate matching total_size and geometry. Increase the number of
+     * sectors requested until we get enough (or fail). This ensures that
+     * qemu-img convert doesn't truncate images, but rather rounds up.
+     *
+     * If the image size can't be represented by a spec conformant CHS geometry,
+     * we set the geometry to 65535 x 16 x 255 (CxHxS) sectors and use
+     * the image size from the VHD footer to calculate total_sectors.
+     */
+    if (vpc_opts->force_size) {
+        /* This will force the use of total_size for sector count, below */
+        cyls         = VHD_CHS_MAX_C;
+        heads        = VHD_CHS_MAX_H;
+        secs_per_cyl = VHD_CHS_MAX_S;
+    } else {
+        total_sectors = MIN(VHD_MAX_GEOMETRY, total_size / BDRV_SECTOR_SIZE);
+        for (i = 0; total_sectors > (int64_t)cyls * heads * secs_per_cyl; i++) {
+            calculate_geometry(total_sectors + i, &cyls, &heads, &secs_per_cyl);
+        }
+    }
+
+    if ((int64_t)cyls * heads * secs_per_cyl == VHD_MAX_GEOMETRY) {
+        total_sectors = total_size / BDRV_SECTOR_SIZE;
+        /* Allow a maximum disk size of 2040 GiB */
+        if (total_sectors > VHD_MAX_SECTORS) {
+            error_setg(errp, "Disk size is too large, max size is 2040 GiB");
+            return -EFBIG;
+        }
+    } else {
+        total_sectors = (int64_t) cyls * heads * secs_per_cyl;
+    }
+
+    *out_total_sectors = total_sectors;
+    if (out_cyls) {
+        *out_cyls = cyls;
+        *out_heads = heads;
+        *out_secs_per_cyl = secs_per_cyl;
+    }
+
+    return 0;
+}
+
 static int coroutine_fn vpc_co_create(BlockdevCreateOptions *opts,
                                       Error **errp)
 {
@@ -911,7 +967,6 @@ static int coroutine_fn vpc_co_create(BlockdevCreateOptions *opts,
 
     uint8_t buf[1024];
     VHDFooter *footer = (VHDFooter *) buf;
-    int i;
     uint16_t cyls = 0;
     uint8_t heads = 0;
     uint8_t secs_per_cyl = 0;
@@ -953,38 +1008,22 @@ static int coroutine_fn vpc_co_create(BlockdevCreateOptions *opts,
     }
     blk_set_allow_write_beyond_eof(blk, true);
 
-    /*
-     * Calculate matching total_size and geometry. Increase the number of
-     * sectors requested until we get enough (or fail). This ensures that
-     * qemu-img convert doesn't truncate images, but rather rounds up.
-     *
-     * If the image size can't be represented by a spec conformant CHS geometry,
-     * we set the geometry to 65535 x 16 x 255 (CxHxS) sectors and use
-     * the image size from the VHD footer to calculate total_sectors.
-     */
-    if (vpc_opts->force_size) {
-        /* This will force the use of total_size for sector count, below */
-        cyls         = VHD_CHS_MAX_C;
-        heads        = VHD_CHS_MAX_H;
-        secs_per_cyl = VHD_CHS_MAX_S;
-    } else {
-        total_sectors = MIN(VHD_MAX_GEOMETRY, total_size / BDRV_SECTOR_SIZE);
-        for (i = 0; total_sectors > (int64_t)cyls * heads * secs_per_cyl; i++) {
-            calculate_geometry(total_sectors + i, &cyls, &heads, &secs_per_cyl);
-        }
+    /* Get geometry and check that it matches the image size*/
+    ret = calculate_rounded_image_size(vpc_opts, &cyls, &heads, &secs_per_cyl,
+                                       &total_sectors, errp);
+    if (ret < 0) {
+        goto out;
     }
 
-    if ((int64_t)cyls * heads * secs_per_cyl == VHD_MAX_GEOMETRY) {
-        total_sectors = total_size / BDRV_SECTOR_SIZE;
-        /* Allow a maximum disk size of 2040 GiB */
-        if (total_sectors > VHD_MAX_SECTORS) {
-            error_setg(errp, "Disk size is too large, max size is 2040 GiB");
-            ret = -EFBIG;
-            goto out;
-        }
-    } else {
-        total_sectors = (int64_t)cyls * heads * secs_per_cyl;
-        total_size = total_sectors * BDRV_SECTOR_SIZE;
+    if (total_size != total_sectors * BDRV_SECTOR_SIZE) {
+        error_setg(errp, "The requested image size cannot be represented in "
+                         "CHS geometry");
+        error_append_hint(errp, "Try size=%llu or force-size=on (the "
+                                "latter makes the image incompatible with "
+                                "Virtual PC)",
+                          total_sectors * BDRV_SECTOR_SIZE);
+        ret = -EINVAL;
+        goto out;
     }
 
     /* Prepare the Hard Disk Footer */
@@ -1102,6 +1141,18 @@ static int coroutine_fn vpc_co_create_opts(const char *filename,
     create_options->u.vpc.size =
         ROUND_UP(create_options->u.vpc.size, BDRV_SECTOR_SIZE);
 
+    if (!create_options->u.vpc.force_size) {
+        int64_t total_sectors;
+        ret = calculate_rounded_image_size(&create_options->u.vpc, NULL, NULL,
+                                           NULL, &total_sectors, errp);
+        if (ret < 0) {
+            goto fail;
+        }
+
+        create_options->u.vpc.size = total_sectors * BDRV_SECTOR_SIZE;
+    }
+
+
     /* Create the vpc image (format layer) */
     ret = vpc_co_create(create_options, errp);
 
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v2 5/8] vdi: Make comments consistent with other drivers
  2018-03-13 14:47 ` [Qemu-devel] [PATCH v2 5/8] vdi: Make comments consistent with other drivers Kevin Wolf
@ 2018-03-13 14:55   ` Max Reitz
  0 siblings, 0 replies; 14+ messages in thread
From: Max Reitz @ 2018-03-13 14:55 UTC (permalink / raw)
  To: Kevin Wolf, qemu-block; +Cc: den, jcody, eblake, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 358 bytes --]

On 2018-03-13 15:47, Kevin Wolf wrote:
> This makes the .bdrv_co_create(_opts) implementation of vdi look more
> like the other recently converted block drivers.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/vdi.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v2 6/8] vhdx: Support .bdrv_co_create
  2018-03-13 14:47 ` [Qemu-devel] [PATCH v2 6/8] vhdx: Support .bdrv_co_create Kevin Wolf
@ 2018-03-13 14:59   ` Max Reitz
  0 siblings, 0 replies; 14+ messages in thread
From: Max Reitz @ 2018-03-13 14:59 UTC (permalink / raw)
  To: Kevin Wolf, qemu-block; +Cc: den, jcody, eblake, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 1507 bytes --]

On 2018-03-13 15:47, Kevin Wolf wrote:
> This adds the .bdrv_co_create driver callback to vhdx, which
> enables image creation over QMP.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  qapi/block-core.json |  40 +++++++++-
>  block/vhdx.c         | 216 ++++++++++++++++++++++++++++++++++++++-------------
>  2 files changed, 203 insertions(+), 53 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>

> diff --git a/block/vhdx.c b/block/vhdx.c
> index d82350d07c..f1b97f4b49 100644
> --- a/block/vhdx.c
> +++ b/block/vhdx.c

[...]

> @@ -1792,59 +1797,71 @@ 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)
>  {

[...]

>      /* These are pretty arbitrary, and mainly designed to keep the BAT
>       * size reasonable to load into RAM */
> -    if (block_size == 0) {
> +    if (vhdx_opts->has_block_size) {
> +        block_size = vhdx_opts->block_size;
> +    } else {

Ah, good.  I have to admit I was a bit worried you'd keep the special
meaning of cluster_size == 0.

Max

>          if (image_size > 32 * TiB) {
>              block_size = 64 * MiB;
>          } else if (image_size > (uint64_t) 100 * GiB) {


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v2 7/8] vpc: Support .bdrv_co_create
  2018-03-13 14:47 ` [Qemu-devel] [PATCH v2 7/8] vpc: " Kevin Wolf
@ 2018-03-13 15:00   ` Max Reitz
  0 siblings, 0 replies; 14+ messages in thread
From: Max Reitz @ 2018-03-13 15:00 UTC (permalink / raw)
  To: Kevin Wolf, qemu-block; +Cc: den, jcody, eblake, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 428 bytes --]

On 2018-03-13 15:47, Kevin Wolf wrote:
> This adds the .bdrv_co_create driver callback to vpc, which
> enables image creation over QMP.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  qapi/block-core.json |  33 ++++++++++-
>  block/vpc.c          | 152 ++++++++++++++++++++++++++++++++++++++-------------
>  2 files changed, 147 insertions(+), 38 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v2 8/8] vpc: Require aligned size in .bdrv_co_create
  2018-03-13 14:47 ` [Qemu-devel] [PATCH v2 8/8] vpc: Require aligned size in .bdrv_co_create Kevin Wolf
@ 2018-03-13 15:04   ` Max Reitz
  0 siblings, 0 replies; 14+ messages in thread
From: Max Reitz @ 2018-03-13 15:04 UTC (permalink / raw)
  To: Kevin Wolf, qemu-block; +Cc: den, jcody, eblake, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 629 bytes --]

On 2018-03-13 15:47, Kevin Wolf wrote:
> Perform the rounding to match a CHS geometry only in the legacy code
> path in .bdrv_co_create_opts. QMP now requires that the user already
> passes a CHS aligned image size, unless force-size=true is given.
> 
> CHS alignment is required to make the image compatible with Virtual PC,
> but not for use with newer Microsoft hypervisors.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/vpc.c | 113 +++++++++++++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 82 insertions(+), 31 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/8] block: .bdrv_co_create for format drivers
  2018-03-13 14:47 [Qemu-devel] [PATCH v2 0/8] block: .bdrv_co_create for format drivers Kevin Wolf
                   ` (7 preceding siblings ...)
  2018-03-13 14:47 ` [Qemu-devel] [PATCH v2 8/8] vpc: Require aligned size in .bdrv_co_create Kevin Wolf
@ 2018-03-13 15:12 ` Kevin Wolf
  8 siblings, 0 replies; 14+ messages in thread
From: Kevin Wolf @ 2018-03-13 15:12 UTC (permalink / raw)
  To: qemu-block; +Cc: mreitz, den, jcody, eblake, qemu-devel

Am 13.03.2018 um 15:47 hat Kevin Wolf geschrieben:
> This series adds a .bdrv_co_create implementation to almost all format
> drivers that support creating images where its still missing. The only
> exception is VMDK because its support for extents will make the QAPI
> design a bit more complicated.
> 
> The other format driver not covered in this series are qcow2 (already
> merged) and luks (already posted in a separate series).
> 
> v2:
> - Rebased, the vdi patch consists just of some cosmetic cleanups now
> - vhdx, vpc: Don't do any silent rounding in .bdrv_co_create, error out
>   if the passed size isn't properly aligned yet. The legacy code paths
>   compensate for this.

Applied to the block branch.

Kevin

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2018-03-13 15:13 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-13 14:47 [Qemu-devel] [PATCH v2 0/8] block: .bdrv_co_create for format drivers Kevin Wolf
2018-03-13 14:47 ` [Qemu-devel] [PATCH v2 1/8] parallels: Support .bdrv_co_create Kevin Wolf
2018-03-13 14:47 ` [Qemu-devel] [PATCH v2 2/8] qemu-iotests: Enable write tests for parallels Kevin Wolf
2018-03-13 14:47 ` [Qemu-devel] [PATCH v2 3/8] qcow: Support .bdrv_co_create Kevin Wolf
2018-03-13 14:47 ` [Qemu-devel] [PATCH v2 4/8] qed: " Kevin Wolf
2018-03-13 14:47 ` [Qemu-devel] [PATCH v2 5/8] vdi: Make comments consistent with other drivers Kevin Wolf
2018-03-13 14:55   ` Max Reitz
2018-03-13 14:47 ` [Qemu-devel] [PATCH v2 6/8] vhdx: Support .bdrv_co_create Kevin Wolf
2018-03-13 14:59   ` Max Reitz
2018-03-13 14:47 ` [Qemu-devel] [PATCH v2 7/8] vpc: " Kevin Wolf
2018-03-13 15:00   ` Max Reitz
2018-03-13 14:47 ` [Qemu-devel] [PATCH v2 8/8] vpc: Require aligned size in .bdrv_co_create Kevin Wolf
2018-03-13 15:04   ` Max Reitz
2018-03-13 15:12 ` [Qemu-devel] [PATCH v2 0/8] block: .bdrv_co_create for format drivers Kevin Wolf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).