[Qemu-devel] [PATCH v6 12/16] block: simplify blockdev-backup

From: "Denis V. Lunev" <den@openvz.org>
To: qemu-block@nongnu.org, qemu-devel@nongnu.org
Cc: den@openvz.org, Pavel Butsykin <pbutsykin@virtuozzo.com>,
	Jeff Cody <jcody@redhat.com>,
	Markus Armbruster <armbru@redhat.com>,
	Eric Blake <eblake@redhat.com>, John Snow <jsnow@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	Kevin Wolf <kwolf@redhat.com>
Subject: [Qemu-devel] [PATCH v6 12/16] block: simplify blockdev-backup
Date: Thu, 21 Jul 2016 22:40:15 +0300	[thread overview]
Message-ID: <1469130019-18497-13-git-send-email-den@openvz.org> (raw)
In-Reply-To: <1469130019-18497-1-git-send-email-den@openvz.org>

From: Pavel Butsykin <pbutsykin@virtuozzo.com>

Now that we can support boxed commands, use it to greatly reduce the
number of parameters (and likelihood of getting out of sync) when
adjusting blockdev-backup parameters.

Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Jeff Cody <jcody@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Eric Blake <eblake@redhat.com>
CC: John Snow <jsnow@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c           | 70 +++++++++++++++++-----------------------------------
 qapi/block-core.json |  6 ++++-
 2 files changed, 27 insertions(+), 49 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index e29147a..0c5ea25 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1906,14 +1906,8 @@ typedef struct BlockdevBackupState {
     AioContext *aio_context;
 } BlockdevBackupState;
 
-static void do_blockdev_backup(const char *job_id, const char *device,
-                               const char *target, enum MirrorSyncMode sync,
-                               bool has_speed, int64_t speed,
-                               bool has_on_source_error,
-                               BlockdevOnError on_source_error,
-                               bool has_on_target_error,
-                               BlockdevOnError on_target_error,
-                               BlockJobTxn *txn, Error **errp);
+static void do_blockdev_backup(BlockdevBackup *backup, BlockJobTxn *txn,
+                               Error **errp);
 
 static void blockdev_backup_prepare(BlkActionState *common, Error **errp)
 {
@@ -1953,12 +1947,7 @@ static void blockdev_backup_prepare(BlkActionState *common, Error **errp)
     state->bs = blk_bs(blk);
     bdrv_drained_begin(state->bs);
 
-    do_blockdev_backup(backup->has_job_id ? backup->job_id : NULL,
-                       backup->device, backup->target, backup->sync,
-                       backup->has_speed, backup->speed,
-                       backup->has_on_source_error, backup->on_source_error,
-                       backup->has_on_target_error, backup->on_target_error,
-                       common->block_job_txn, &local_err);
+    do_blockdev_backup(backup, common->block_job_txn, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         return;
@@ -3275,14 +3264,7 @@ BlockDeviceInfoList *qmp_query_named_block_nodes(Error **errp)
     return bdrv_named_nodes_list(errp);
 }
 
-void do_blockdev_backup(const char *job_id, const char *device,
-                        const char *target, enum MirrorSyncMode sync,
-                         bool has_speed, int64_t speed,
-                         bool has_on_source_error,
-                         BlockdevOnError on_source_error,
-                         bool has_on_target_error,
-                         BlockdevOnError on_target_error,
-                         BlockJobTxn *txn, Error **errp)
+void do_blockdev_backup(BlockdevBackup *backup, BlockJobTxn *txn, Error **errp)
 {
     BlockBackend *blk;
     BlockDriverState *bs;
@@ -3290,19 +3272,22 @@ void do_blockdev_backup(const char *job_id, const char *device,
     Error *local_err = NULL;
     AioContext *aio_context;
 
-    if (!has_speed) {
-        speed = 0;
+    if (!backup->has_speed) {
+        backup->speed = 0;
     }
-    if (!has_on_source_error) {
-        on_source_error = BLOCKDEV_ON_ERROR_REPORT;
+    if (!backup->has_on_source_error) {
+        backup->on_source_error = BLOCKDEV_ON_ERROR_REPORT;
     }
-    if (!has_on_target_error) {
-        on_target_error = BLOCKDEV_ON_ERROR_REPORT;
+    if (!backup->has_on_target_error) {
+        backup->on_target_error = BLOCKDEV_ON_ERROR_REPORT;
+    }
+    if (!backup->has_job_id) {
+        backup->job_id = NULL;
     }
 
-    blk = blk_by_name(device);
+    blk = blk_by_name(backup->device);
     if (!blk) {
-        error_setg(errp, "Device '%s' not found", device);
+        error_setg(errp, "Device '%s' not found", backup->device);
         return;
     }
 
@@ -3310,12 +3295,12 @@ void do_blockdev_backup(const char *job_id, const char *device,
     aio_context_acquire(aio_context);
 
     if (!blk_is_available(blk)) {
-        error_setg(errp, "Device '%s' has no medium", device);
+        error_setg(errp, "Device '%s' has no medium", backup->device);
         goto out;
     }
     bs = blk_bs(blk);
 
-    target_bs = bdrv_lookup_bs(target, target, errp);
+    target_bs = bdrv_lookup_bs(backup->target, backup->target, errp);
     if (!target_bs) {
         goto out;
     }
@@ -3331,8 +3316,9 @@ void do_blockdev_backup(const char *job_id, const char *device,
             goto out;
         }
     }
-    backup_start(job_id, bs, target_bs, speed, sync, NULL, on_source_error,
-                 on_target_error, block_job_cb, bs, txn, &local_err);
+    backup_start(backup->job_id, bs, target_bs, backup->speed, backup->sync,
+                 NULL, backup->on_source_error, backup->on_target_error,
+                 block_job_cb, bs, txn, &local_err);
     if (local_err != NULL) {
         error_propagate(errp, local_err);
     }
@@ -3340,21 +3326,9 @@ out:
     aio_context_release(aio_context);
 }
 
-void qmp_blockdev_backup(bool has_job_id, const char *job_id,
-                         const char *device, const char *target,
-                         enum MirrorSyncMode sync,
-                         bool has_speed, int64_t speed,
-                         bool has_on_source_error,
-                         BlockdevOnError on_source_error,
-                         bool has_on_target_error,
-                         BlockdevOnError on_target_error,
-                         Error **errp)
+void qmp_blockdev_backup(BlockdevBackup *arg, Error **errp)
 {
-    do_blockdev_backup(has_job_id ? job_id : NULL, device, target,
-                       sync, has_speed, speed,
-                       has_on_source_error, on_source_error,
-                       has_on_target_error, on_target_error,
-                       NULL, errp);
+    do_blockdev_backup(arg, NULL, errp);
 }
 
 /* Parameter check and block job starting for drive mirroring.
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 121a540..44cf9c6 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1100,9 +1100,13 @@
 #
 # For the arguments, see the documentation of BlockdevBackup.
 #
+# Returns: nothing on success
+#          If @device is not a valid block device, DeviceNotFound
+#
 # Since 2.3
 ##
-{ 'command': 'blockdev-backup', 'data': 'BlockdevBackup' }
+{ 'command': 'blockdev-backup', 'boxed': true,
+  'data': 'BlockdevBackup' }
 
 
 ##
-- 
2.5.0
