[Qemu-devel] [RFC] block: convert block_stream command to QAPI

[Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>]

Move from plain HMP/QMP command implementation to QAPI.  The
block_stream command takes arguments, returns nothing, may raise errors,
and will raise a QMP event when the operation completes.
---
Hi Luiz,
I converted block_stream to QAPI.  The in-tree examples are quite simple so far
so I hope I've converted this command in a reasonable way.  Please let me know
what you think:

 - HMP wrapper does QError/Error housekeeping, is there a nicer way?

 - To avoid duplicating documentation I move it to qapi-schema.json.

 - QMP events seem independent of QAPI, of course raising a monitor event
   precludes this QAPI code from really being used as an internal C API today.
   Thoughts?

CCed qemu-devel because I suspect others are figuring out QAPI/monitor
integration too.

 blockdev.c       |   25 ++++++++++++-------------
 hmp-commands.hx  |    3 +--
 hmp.c            |   14 ++++++++++++++
 hmp.h            |    1 +
 qapi-schema.json |   53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 qmp-commands.hx  |   49 ++-----------------------------------------------
 trace-events     |    2 +-
 7 files changed, 84 insertions(+), 63 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 6d78597..380de39 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -16,6 +16,7 @@
 #include "qemu-objects.h"
 #include "sysemu.h"
 #include "block_int.h"
+#include "qmp-commands.h"
 #include "trace.h"
 
 static QTAILQ_HEAD(drivelist, DriveInfo) drives = QTAILQ_HEAD_INITIALIZER(drives);
@@ -817,39 +818,37 @@ static void block_stream_cb(void *opaque, int ret)
     qobject_decref(obj);
 }
 
-int do_block_stream(Monitor *mon, const QDict *params, QObject **ret_data)
+void qmp_block_stream(const char *device, bool has_base,
+                      const char *base, Error **errp)
 {
-    const char *device = qdict_get_str(params, "device");
-    const char *base = qdict_get_try_str(params, "base");
     BlockDriverState *bs;
     int ret;
 
     bs = bdrv_find(device);
     if (!bs) {
-        qerror_report(QERR_DEVICE_NOT_FOUND, device);
-        return -1;
+        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+        return;
     }
 
     /* Base device not supported */
     if (base) {
-        qerror_report(QERR_NOT_SUPPORTED);
-        return -1;
+        error_set(errp, QERR_NOT_SUPPORTED);
+        return;
     }
 
     ret = stream_start(bs, NULL, block_stream_cb, bs);
     if (ret < 0) {
         switch (ret) {
         case -EBUSY:
-            qerror_report(QERR_DEVICE_IN_USE, device);
-            return -1;
+            error_set(errp, QERR_DEVICE_IN_USE, device);
+            return;
         default:
-            qerror_report(QERR_NOT_SUPPORTED);
-            return -1;
+            error_set(errp, QERR_NOT_SUPPORTED);
+            return;
         }
     }
 
-    trace_do_block_stream(bs, bs->job);
-    return 0;
+    trace_qmp_block_stream(bs, bs->job);
 }
 
 static BlockJob *find_block_job(const char *device)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index a8b83f0..b2af867 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -75,8 +75,7 @@ ETEXI
         .args_type  = "device:B,base:s?",
         .params     = "device [base]",
         .help       = "copy data from a backing file into a block device",
-        .user_print = monitor_user_noop,
-        .mhandler.cmd_new = do_block_stream,
+        .mhandler.cmd = hmp_block_stream,
     },
 
 STEXI
diff --git a/hmp.c b/hmp.c
index 34416fc..e65510d 100644
--- a/hmp.c
+++ b/hmp.c
@@ -114,3 +114,17 @@ void hmp_system_powerdown(Monitor *mon, const QDict *qdict)
 {
     qmp_system_powerdown(NULL);
 }
+
+void hmp_block_stream(Monitor *mon, const QDict *qdict)
+{
+    Error *error = NULL;
+    const char *device = qdict_get_str(qdict, "device");
+    const char *base = qdict_get_try_str(qdict, "base");
+
+    qmp_block_stream(device, base != NULL, base, &error);
+
+    if (error_is_set(&error)) {
+        qerror_report_err(error);
+    }
+    error_free(error);
+}
diff --git a/hmp.h b/hmp.h
index 92433cf..779a601 100644
--- a/hmp.h
+++ b/hmp.h
@@ -27,5 +27,6 @@ void hmp_quit(Monitor *mon, const QDict *qdict);
 void hmp_stop(Monitor *mon, const QDict *qdict);
 void hmp_system_reset(Monitor *mon, const QDict *qdict);
 void hmp_system_powerdown(Monitor *mon, const QDict *qdict);
+void hmp_block_stream(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/qapi-schema.json b/qapi-schema.json
index 5922c4a..5c3f898 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -271,3 +271,56 @@
 #        prompting the user in some way.
 ##
 { 'command': 'system_powerdown' }
+
+##
+# @block_stream:
+#
+# Copy data from a backing file into a block device.
+#
+# The block streaming operation is performed in the background until the entire
+# backing file has been copied.  This command returns immediately once streaming
+# has started.  The status of ongoing block streaming operations can be checked
+# with query-block-jobs.  The operation can be stopped before it has completed
+# using the block_job_cancel command.
+#
+# If a base file is specified then sectors are not copied from that base file and
+# its backing chain.  When streaming completes the image file will have the base
+# file as its backing file.  This can be used to stream a subset of the backing
+# file chain instead of flattening the entire image.
+#
+# On successful completion the image file is updated to drop the backing file.
+#
+# Arguments:
+#
+# @device: device name
+# @base:   common backing file
+#
+# Errors:
+#
+# DeviceInUse:    streaming is already active on this device
+# DeviceNotFound: device name is invalid
+# NotSupported:   image streaming is not supported by this device
+#
+# Events:
+#
+# On completion the BLOCK_JOB_COMPLETED event is raised with the following
+# fields:
+#
+# - type:     job type ("stream" for image streaming, json-string)
+# - device:   device name (json-string)
+# - len:      maximum progress value (json-int)
+# - offset:   current progress value (json-int)
+# - speed:    rate limit, bytes per second (json-int)
+# - error:    error message (json-string, only on error)
+#
+# The completion event is raised both on success and on failure.  On
+# success offset is equal to len.  On failure offset and len can be
+# used to indicate at which point the operation failed.
+#
+# On failure the error field contains a human-readable error message.  There are
+# no semantics other than that streaming has failed and clients should not try
+# to interpret the error string.
+#
+# Since: 1.1
+##
+{ 'command': 'block_stream', 'data': { 'device': 'str', '*base': 'str' } }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 6e8207b..79ce09a 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -689,58 +689,13 @@ EQMP
         .args_type  = "device:B,base:s?",
         .params     = "device [base]",
         .user_print = monitor_user_noop,
-        .mhandler.cmd_new = do_block_stream,
+        .mhandler.cmd_new = qmp_marshal_input_block_stream,
     },
 
 SQMP
 block_stream
 ------------
-
-Copy data from a backing file into a block device.
-
-The block streaming operation is performed in the background until the entire
-backing file has been copied.  This command returns immediately once streaming
-has started.  The status of ongoing block streaming operations can be checked
-with query-block-jobs.  The operation can be stopped before it has completed
-using the block_job_cancel command.
-
-If a base file is specified then sectors are not copied from that base file and
-its backing chain.  When streaming completes the image file will have the base
-file as its backing file.  This can be used to stream a subset of the backing
-file chain instead of flattening the entire image.
-
-On successful completion the image file is updated to drop the backing file.
-
-Arguments:
-
-- device: device name (json-string)
-- base:   common backing file (json-string, optional)
-
-Errors:
-
-DeviceInUse:    streaming is already active on this device
-DeviceNotFound: device name is invalid
-NotSupported:   image streaming is not supported by this device
-
-Events:
-
-On completion the BLOCK_JOB_COMPLETED event is raised with the following
-fields:
-
-- type:     job type ("stream" for image streaming, json-string)
-- device:   device name (json-string)
-- len:      maximum progress value (json-int)
-- offset:   current progress value (json-int)
-- speed:    rate limit, bytes per second (json-int)
-- error:    error message (json-string, only on error)
-
-The completion event is raised both on success and on failure.  On
-success offset is equal to len.  On failure offset and len can be
-used to indicate at which point the operation failed.
-
-On failure the error field contains a human-readable error message.  There are
-no semantics other than that streaming has failed and clients should not try
-to interpret the error string.
+See qapi-schema.json for documentation.
 
 Examples:
 
diff --git a/trace-events b/trace-events
index 39023cc..8b4cdc3 100644
--- a/trace-events
+++ b/trace-events
@@ -79,7 +79,7 @@ stream_start(void *bs, void *base, void *s, void *co, void *opaque) "bs %p base
 block_job_cancel_cb(void *cancel_data, void *opaque) "cancel_data %p opaque %p"
 do_block_job_cancel(void *job, void *cancel_data, void *opaque) "job %p cancel_data %p opaque %p"
 block_stream_cb(void *bs, void *job, int ret) "bs %p job %p ret %d"
-do_block_stream(void *bs, void *job) "bs %p job %p"
+qmp_block_stream(void *bs, void *job) "bs %p job %p"
 
 # hw/virtio-blk.c
 virtio_blk_req_complete(void *req, int status) "req %p status %d"
-- 
1.7.7