diff mbox series

[RFC,10/10] block: x-blockdev-create QMP command

Message ID 20180111195225.4226-11-kwolf@redhat.com
State New
Headers show
Series x-blockdev-create for qcow2 | expand

Commit Message

Kevin Wolf Jan. 11, 2018, 7:52 p.m. UTC
This adds a synchronous x-blockdev-create QMP command that can create
qcow2 images on a given node name.

We don't want to block while creating an image, so this is not the final
interface in all aspects, but BlockdevCreateOptionsQcow2 and
.bdrv_co_create() are what they actually might look like in the end. In
any case, this should be good enough to test whether we interpret
BlockdevCreateOptions as we should.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qapi/block-core.json      | 12 ++++++++++++
 include/block/block_int.h |  2 ++
 block.c                   | 48 +++++++++++++++++++++++++++++++++++++++++++++++
 block/qcow2.c             |  3 ++-
 4 files changed, 64 insertions(+), 1 deletion(-)

Comments

Eric Blake Jan. 16, 2018, 8:06 p.m. UTC | #1
On 01/11/2018 01:52 PM, Kevin Wolf wrote:
> This adds a synchronous x-blockdev-create QMP command that can create
> qcow2 images on a given node name.
> 
> We don't want to block while creating an image, so this is not the final
> interface in all aspects, but BlockdevCreateOptionsQcow2 and
> .bdrv_co_create() are what they actually might look like in the end. In
> any case, this should be good enough to test whether we interpret
> BlockdevCreateOptions as we should.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  qapi/block-core.json      | 12 ++++++++++++
>  include/block/block_int.h |  2 ++
>  block.c                   | 48 +++++++++++++++++++++++++++++++++++++++++++++++
>  block/qcow2.c             |  3 ++-
>  4 files changed, 64 insertions(+), 1 deletion(-)
> 
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 9341f6708d..93357a4d5d 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -3415,6 +3415,18 @@
>    } }
>  
>  ##
> +# @x-blockdev-create:
> +#
> +# Create an image format on a given node.
> +# TODO Replace with something asynchronous (block job?)
> +#
> +# Since: 2.12
> +##
> +{ 'command': 'x-blockdev-create',
> +  'data': 'BlockdevCreateOptions',
> +  'boxed': true }
> +

So simple, compared to all the prep work in earlier patches ;)

I like the approach.  As you say, there's still more work before we can
remove the x- prefix, but I think you're headed on a good track.
Kevin Wolf Jan. 17, 2018, 5:50 p.m. UTC | #2
Am 11.01.2018 um 20:52 hat Kevin Wolf geschrieben:
> This adds a synchronous x-blockdev-create QMP command that can create
> qcow2 images on a given node name.
> 
> We don't want to block while creating an image, so this is not the final
> interface in all aspects, but BlockdevCreateOptionsQcow2 and
> .bdrv_co_create() are what they actually might look like in the end. In
> any case, this should be good enough to test whether we interpret
> BlockdevCreateOptions as we should.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  qapi/block-core.json      | 12 ++++++++++++
>  include/block/block_int.h |  2 ++
>  block.c                   | 48 +++++++++++++++++++++++++++++++++++++++++++++++
>  block/qcow2.c             |  3 ++-
>  4 files changed, 64 insertions(+), 1 deletion(-)
> 
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 9341f6708d..93357a4d5d 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -3415,6 +3415,18 @@
>    } }
>  
>  ##
> +# @x-blockdev-create:
> +#
> +# Create an image format on a given node.
> +# TODO Replace with something asynchronous (block job?)

I just realised that this won't be a block job. It will be a job without
block, because there isn't necessarily any BlockDriverState involved
(when you create the protocol layer).

So it looks like my first job there is to make block jobs work with
bs == NULL... Or actually, we should probably try and do the proper
thing with the whole new job API that we were planning anyway?

Kevin
diff mbox series

Patch

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 9341f6708d..93357a4d5d 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3415,6 +3415,18 @@ 
   } }
 
 ##
+# @x-blockdev-create:
+#
+# Create an image format on a given node.
+# TODO Replace with something asynchronous (block job?)
+#
+# Since: 2.12
+##
+{ 'command': 'x-blockdev-create',
+  'data': 'BlockdevCreateOptions',
+  'boxed': true }
+
+##
 # @blockdev-open-tray:
 #
 # Opens a block device's tray. If there is a block driver state tree inserted as
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 29cafa4236..a9f144d7bd 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -130,6 +130,8 @@  struct BlockDriver {
     int (*bdrv_file_open)(BlockDriverState *bs, QDict *options, int flags,
                           Error **errp);
     void (*bdrv_close)(BlockDriverState *bs);
+    int coroutine_fn (*bdrv_co_create)(BlockdevCreateOptions *opts,
+                                       Error **errp);
     int (*bdrv_create)(const char *filename, QemuOpts *opts, Error **errp);
     int (*bdrv_make_empty)(BlockDriverState *bs);
 
diff --git a/block.c b/block.c
index c9b0e1d6d3..7521884de8 100644
--- a/block.c
+++ b/block.c
@@ -485,6 +485,54 @@  int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp)
     return ret;
 }
 
+typedef struct BlockdevCreateCo {
+    BlockDriver *drv;
+    BlockdevCreateOptions *opts;
+    int ret;
+    Error **errp;
+} BlockdevCreateCo;
+
+static void coroutine_fn bdrv_co_create_co_entry(void *opaque)
+{
+    BlockdevCreateCo *cco = opaque;
+    cco->ret = cco->drv->bdrv_co_create(cco->opts, cco->errp);
+}
+
+void qmp_x_blockdev_create(BlockdevCreateOptions *options, Error **errp)
+{
+    const char *fmt = BlockdevDriver_str(options->driver);
+    BlockDriver* drv = bdrv_find_format(fmt);
+    Coroutine *co;
+    BlockdevCreateCo cco;
+
+    /* If the driver is in the schema, we know that it exists. But it may not
+     * be whitelisted. */
+    assert(drv);
+    if (bdrv_uses_whitelist() && !bdrv_is_whitelisted(drv, true)) {
+        error_setg(errp, "Driver is not whitelisted");
+        return;
+    }
+
+    /* Call callback if it exists */
+    if (!drv->bdrv_co_create) {
+        error_setg(errp, "Driver does not support blockdev-create");
+        return;
+    }
+
+    cco = (BlockdevCreateCo) {
+        .drv = drv,
+        .opts = options,
+        .ret = NOT_DONE,
+        .errp = errp,
+    };
+
+    co = qemu_coroutine_create(bdrv_co_create_co_entry, &cco);
+    qemu_coroutine_enter(co);
+    while (cco.ret == NOT_DONE) {
+        aio_poll(qemu_get_aio_context(), true);
+    }
+}
+
 /**
  * Try to get @bs's logical and physical block size.
  * On success, store them in @bsz struct and return 0.
diff --git a/block/qcow2.c b/block/qcow2.c
index 4031a18a77..5c5386e0b6 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -4436,7 +4436,8 @@  BlockDriver bdrv_qcow2 = {
     .bdrv_reopen_abort    = qcow2_reopen_abort,
     .bdrv_join_options    = qcow2_join_options,
     .bdrv_child_perm      = bdrv_format_default_perms,
-    .bdrv_create        = qcow2_create,
+    .bdrv_create          = qcow2_create,
+    .bdrv_co_create       = qcow2_create2,
     .bdrv_has_zero_init = bdrv_has_zero_init_1,
     .bdrv_co_get_block_status = qcow2_co_get_block_status,