All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.