Skip to contentSkip to conversation

vdi: Implement .bdrv_co_create

PATCH Applied Max Reitz 2 participants 5 messages Mar 12, 2018 – Mar 13, 2018 Block layer

lore
Share & tools
lore

Max Reitz

author
#0 lore raw

cover vdi: Implement .bdrv_co_create

I think cluster-size should not be exposed in QAPI (yet), but Kevin's patch does that, while I had these patches lying around, so here is my proposal.

Max Reitz (3):
vdi: Pull option parsing from vdi_co_create
vdi: Move file creation to vdi_co_create_opts
vdi: Implement .bdrv_co_create

 qapi/block-core.json |  20 +++++++-
 block/vdi.c          | 139 +++++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 132 insertions(+), 27 deletions(-)
Signature · 1 line
2.14.3
author
#1 lore raw

PATCH 1/3 vdi: Pull option parsing from vdi_co_create

In preparation of QAPI-fying VDI image creation, we have to create a BlockdevCreateOptionsVdi type which is received by a (future) vdi_co_create().

vdi_co_create_opts() now converts the QemuOpts object into such a BlockdevCreateOptionsVdi object. The protocol-layer file is still created in vdi_co_do_create() (and BlockdevCreateOptionsVdi.file is set to an empty string), but that will be addressed by a follow-up patch.

Note that cluster-size is not part of the QAPI schema because it is not supported by default.

---
 qapi/block-core.json | 18 +++++++++++
 block/vdi.c          | 91 ++++++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 95 insertions(+), 14 deletions(-)
qapi/block-core.json +18 -0
index 524d51567a..e7110c9a47 100644
@@ -3606,6 +3606,24 @@
   'data': { 'location':         'BlockdevOptionsSsh',
             'size':             'size' } }
 
+##
+# @BlockdevCreateOptionsVdi:
+#
+# Driver specific image creation options for VDI.
+#
+# @file             Node to create the image format on
+# @size             Size of the virtual disk in bytes
+# @static           Whether to create a statically (true) or
+#                   dynamically (false) allocated image
+#                   (default: false, i.e. dynamic)
+#
+# Since: 2.12
+##
+{ 'struct': 'BlockdevCreateOptionsVdi',
+  'data': { 'file':             'BlockdevRef',
+            'size':             'size',
+            '*static':          'bool' } }
+
 ##
 # @BlockdevCreateNotSupported:
 #
block/vdi.c +77 -14

133 lines not on this page · raw message

Signature · 1 line
2.14.3
author
#2 lore raw

PATCH 2/3 vdi: Move file creation to vdi_co_create_opts

In reply to Max Reitz
---
 block/vdi.c | 46 ++++++++++++++++++++++++++++------------------
 1 file changed, 28 insertions(+), 18 deletions(-)
block/vdi.c +28 -18
index 0c8f8204ce..2a39b0ac98 100644
@@ -721,9 +721,7 @@ nonallocating_write:
     return ret;
 }
 
-static int coroutine_fn vdi_co_do_create(const char *filename,
-                                         QemuOpts *file_opts,
-                                         BlockdevCreateOptionsVdi *vdi_opts,
+static int coroutine_fn vdi_co_do_create(BlockdevCreateOptionsVdi *vdi_opts,
                                          size_t block_size, Error **errp)
 {
     int ret = 0;
@@ -734,7 +732,7 @@ static int coroutine_fn vdi_co_do_create(const char *filename,
     size_t i;
     size_t bmap_size;
     int64_t offset = 0;
-    Error *local_err = NULL;
+    BlockDriverState *bs_file = NULL;
     BlockBackend *blk = NULL;
     uint32_t *bmap = NULL;
 
@@ -770,18 +768,15 @@ static int coroutine_fn vdi_co_do_create(const char *filename,
         goto exit;
     }
 
-    ret = bdrv_create_file(filename, file_opts, &local_err);
-    if (ret < 0) {
-        error_propagate(errp, local_err);
+    bs_file = bdrv_open_blockdev_ref(vdi_opts->file, errp);
+    if (!bs_file) {
+        ret = -EIO;
         goto exit;
     }
 
-    blk = blk_new_open(filename, NULL, NULL,
-                       BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL,
-                       &local_err);
-    if (blk == NULL) {
-        error_propagate(errp, local_err);
-        ret = -EIO;
+    blk = blk_new(BLK_PERM_WRITE | BLK_PERM_RESIZE, BLK_PERM_ALL);
+    ret = blk_insert_bs(blk, bs_file, errp);
+    if (ret < 0) {
         goto exit;
     }
 
@@ -818,7 +813,7 @@ static int coroutine_fn vdi_co_do_create(const char *filename,
     vdi_header_to_le(&header);
     ret = blk_pwrite(blk, offset, &header, sizeof(header), 0);
     if (ret < 0) {
-        error_setg(errp, "Error writing header to %s", filename);
+        error_setg(errp, "Error writing header");
         goto exit;
     }
     offset += sizeof(header);
@@ -839,7 +834,7 @@ static int coroutine_fn vdi_co_do_create(const char *filename,
         }
         ret = blk_pwrite(blk, offset, bmap, bmap_size, 0);
         if (ret < 0) {
-            error_setg(errp, "Error writing bmap to %s", filename);
+            error_setg(errp, "Error writing bmap");
             goto exit;
         }
         offset += bmap_size;
@@ -849,13 +844,14 @@ static int coroutine_fn vdi_co_do_create(const char *filename,
         ret = blk_truncate(blk, offset + blocks * block_size,
                            PREALLOC_MODE_OFF, errp);
         if (ret < 0) {
-            error_prepend(errp, "Failed to statically allocate %s", filename);
+            error_prepend(errp, "Failed to statically allocate file");
             goto exit;
         }
     }
 
 exit:
     blk_unref(blk);
+    bdrv_unref(bs_file);
     g_free(bmap);
     return ret;
 }
@@ -865,6 +861,7 @@ static int coroutine_fn vdi_co_create_opts(const char *filename, QemuOpts *opts,
 {
     QDict *qdict = NULL;
     BlockdevCreateOptionsVdi *create_options = NULL;
+    BlockDriverState *bs_file = NULL;
     uint64_t block_size = DEFAULT_CLUSTER_SIZE;
     Visitor *v;
     Error *local_err = NULL;
@@ -888,7 +885,19 @@ 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);
 
-    qdict_put_str(qdict, "file", ""); /* FIXME */
+    ret = bdrv_create_file(filename, opts, errp);
+    if (ret < 0) {
+        goto done;
+    }
+
+    bs_file = bdrv_open(filename, NULL, NULL,
+                        BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, errp);
+    if (!bs_file) {
+        ret = -EIO;
+        goto done;
+    }
+
+    qdict_put_str(qdict, "file", bs_file->node_name);
 
     /* Get the QAPI object */
     v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
@@ -903,10 +912,11 @@ static int coroutine_fn vdi_co_create_opts(const char *filename, QemuOpts *opts,
 
     create_options->size = ROUND_UP(create_options->size, BDRV_SECTOR_SIZE);
 
-    ret = vdi_co_do_create(filename, opts, create_options, block_size, errp);
+    ret = vdi_co_do_create(create_options, block_size, errp);
 done:
     QDECREF(qdict);
     qapi_free_BlockdevCreateOptionsVdi(create_options);
+    bdrv_unref(bs_file);
     return ret;
 }
 
Signature · 1 line
2.14.3
author
#3 lore raw

PATCH 3/3 vdi: Implement .bdrv_co_create

In reply to Max Reitz
---
 qapi/block-core.json |  2 +-
 block/vdi.c          | 24 +++++++++++++++++++-----
 2 files changed, 20 insertions(+), 6 deletions(-)
qapi/block-core.json +1 -1
index e7110c9a47..5c3186fe4f 100644
@@ -3678,7 +3678,7 @@
       'sheepdog':       'BlockdevCreateOptionsSheepdog',
       'ssh':            'BlockdevCreateOptionsSsh',
       'throttle':       'BlockdevCreateNotSupported',
-      'vdi':            'BlockdevCreateNotSupported',
+      'vdi':            'BlockdevCreateOptionsVdi',
       'vhdx':           'BlockdevCreateNotSupported',
       'vmdk':           'BlockdevCreateNotSupported',
       'vpc':            'BlockdevCreateNotSupported',
block/vdi.c +19 -5
index 2a39b0ac98..8132e3adfe 100644
@@ -721,9 +721,10 @@ nonallocating_write:
     return ret;
 }
 
-static int coroutine_fn vdi_co_do_create(BlockdevCreateOptionsVdi *vdi_opts,
+static int coroutine_fn vdi_co_do_create(BlockdevCreateOptions *create_options,
                                          size_t block_size, Error **errp)
 {
+    BlockdevCreateOptionsVdi *vdi_opts;
     int ret = 0;
     uint64_t bytes = 0;
     uint32_t blocks;
@@ -736,6 +737,9 @@ static int coroutine_fn vdi_co_do_create(BlockdevCreateOptionsVdi *vdi_opts,
     BlockBackend *blk = NULL;
     uint32_t *bmap = NULL;
 
+    assert(create_options->driver == BLOCKDEV_DRIVER_VDI);
+    vdi_opts = &create_options->u.vdi;
+
     logout("\n");
 
     /* Read out options. */
@@ -856,11 +860,17 @@ exit:
     return ret;
 }
 
+static int coroutine_fn vdi_co_create(BlockdevCreateOptions *create_options,
+                                      Error **errp)
+{
+    return vdi_co_do_create(create_options, DEFAULT_CLUSTER_SIZE, errp);
+}
+
 static int coroutine_fn vdi_co_create_opts(const char *filename, QemuOpts *opts,
                                            Error **errp)
 {
     QDict *qdict = NULL;
-    BlockdevCreateOptionsVdi *create_options = NULL;
+    BlockdevCreateOptions *create_options = NULL;
     BlockDriverState *bs_file = NULL;
     uint64_t block_size = DEFAULT_CLUSTER_SIZE;
     Visitor *v;
@@ -897,11 +907,12 @@ static int coroutine_fn vdi_co_create_opts(const char *filename, QemuOpts *opts,
         goto done;
     }
 
+    qdict_put_str(qdict, "driver", "vdi");
     qdict_put_str(qdict, "file", bs_file->node_name);
 
     /* Get the QAPI object */
     v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
-    visit_type_BlockdevCreateOptionsVdi(v, NULL, &create_options, &local_err);
+    visit_type_BlockdevCreateOptions(v, NULL, &create_options, &local_err);
     visit_free(v);
 
     if (local_err) {
@@ -910,12 +921,14 @@ static int coroutine_fn vdi_co_create_opts(const char *filename, QemuOpts *opts,
         goto done;
     }
 
-    create_options->size = ROUND_UP(create_options->size, BDRV_SECTOR_SIZE);
+    assert(create_options->driver == BLOCKDEV_DRIVER_VDI);
+    create_options->u.vdi.size = ROUND_UP(create_options->u.vdi.size,
+                                          BDRV_SECTOR_SIZE);
 
     ret = vdi_co_do_create(create_options, block_size, errp);
 done:
     QDECREF(qdict);
-    qapi_free_BlockdevCreateOptionsVdi(create_options);
+    qapi_free_BlockdevCreateOptions(create_options);
     bdrv_unref(bs_file);
     return ret;
 }
@@ -969,6 +982,7 @@ static BlockDriver bdrv_vdi = {
     .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_has_zero_init = bdrv_has_zero_init_1,
     .bdrv_co_block_status = vdi_co_block_status,
     .bdrv_make_empty = vdi_make_empty,
Signature · 1 line
2.14.3
Applied
#4 lore raw
In reply to Max Reitz

Am 12.03.2018 um 17:55 hat Max Reitz geschrieben:

Quoted 3 lines

I think cluster-size should not be exposed in QAPI (yet), but Kevin's patch does that, while I had these patches lying around, so here is my proposal.

Thanks, applied to the block branch.

Kevin