[6/7] block: add target-id option to drive-backup QMP command
diff mbox

Message ID 1372744789-997-7-git-send-email-famz@redhat.com
State New
Headers show

Commit Message

Fam Zheng July 2, 2013, 5:59 a.m. UTC
Add target-id (optional) to drive-backup command, to make the target bs
a named drive so that we can operate on it (e.g. export with NBD).

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 blockdev.c       | 4 +++-
 qapi-schema.json | 7 +++++--
 qmp-commands.hx  | 3 ++-
 3 files changed, 10 insertions(+), 4 deletions(-)

Comments

Eric Blake July 2, 2013, 7:59 p.m. UTC | #1
On 07/01/2013 11:59 PM, Fam Zheng wrote:
> Add target-id (optional) to drive-backup command, to make the target bs
> a named drive so that we can operate on it (e.g. export with NBD).
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  blockdev.c       | 4 +++-
>  qapi-schema.json | 7 +++++--
>  qmp-commands.hx  | 3 ++-
>  3 files changed, 10 insertions(+), 4 deletions(-)
> 

> +++ b/qapi-schema.json
> @@ -1654,7 +1654,8 @@
>  # Since: 1.6
>  ##
>  { 'type': 'DriveBackup',
> -  'data': { 'device': 'str', 'target': 'str', '*format': 'str',
> +  'data': { 'device': 'str', 'target': 'str',
> +            '*target-id': 'str', '*format': 'str',

Seems undocumented...

>              '*mode': 'NewImageMode', '*speed': 'int',
>              '*on-source-error': 'BlockdevOnError',
>              '*on-target-error': 'BlockdevOnError' } }
> @@ -1807,6 +1808,7 @@
>  #          is a device, the existing file/device will be used as the new
>  #          destination.  If it does not exist, a new file will be created.
>  #
> +# @target-id: #optional the drive id of the target.

...until I read this.  Hmm, I think we should first consolidate things
for DriveBackup (so that documentation is listed only once, prior to the
DriveBackup 'type' declaration), by rebasing things on top of
in the same was as Kevin's series "[PATCH v3 0/3] qapi: Top-level type
reference for command definitions" does for BlockdevSnapshot.

The documentation is not incorrect, but it also isn't very helpful -
what is the "drive id of the target" and when would I want to set it?
What do I gain by overriding the drive id, and what is the default
behavior when I don't pass in the option?

> +++ b/qmp-commands.hx
> @@ -913,7 +913,7 @@ EQMP
>  
>      {
>          .name       = "drive-backup",
> -        .args_type  = "device:B,target:s,speed:i?,mode:s?,format:s?,"
> +        .args_type  = "device:B,target:s,target-id:s?,speed:i?,mode:s?,format:s?,"
>                        "on-source-error:s?,on-target-error:s?",
>          .mhandler.cmd_new = qmp_marshal_input_drive_backup,
>      },
> @@ -936,6 +936,7 @@ Arguments:
>              device, the existing file/device will be used as the new
>              destination.  If it does not exist, a new file will be created.
>              (json-string)
> +- "target-id": the drive id of the target image.

Should probably mention (json-string, optional), as done elsewhere in
this command.

>  - "format": the format of the new destination, default is to probe if 'mode' is
>              'existing', else the format of the source
>              (json-string, optional)
>

Patch
diff mbox

diff --git a/blockdev.c b/blockdev.c
index d02d99a..a297eaf 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -936,6 +936,7 @@  static void drive_backup_prepare(BlkTransactionState *common, Error **errp)
     backup = common->action->drive_backup;
 
     qmp_drive_backup(backup->device, backup->target,
+                     backup->has_target_id, backup->target_id,
                      backup->has_format, backup->format,
                      backup->has_mode, backup->mode,
                      backup->has_speed, backup->speed,
@@ -1421,6 +1422,7 @@  void qmp_block_commit(const char *device,
 }
 
 void qmp_drive_backup(const char *device, const char *target,
+                      bool has_target_id, const char *target_id,
                       bool has_format, const char *format,
                       bool has_mode, enum NewImageMode mode,
                       bool has_speed, int64_t speed,
@@ -1495,7 +1497,7 @@  void qmp_drive_backup(const char *device, const char *target,
         return;
     }
 
-    target_bs = bdrv_new("");
+    target_bs = bdrv_new(has_target_id ? target_id : "");
     ret = bdrv_open(target_bs, target, NULL, flags, drv);
     if (ret < 0) {
         bdrv_delete(target_bs);
diff --git a/qapi-schema.json b/qapi-schema.json
index 5c32528..2f2a87f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1654,7 +1654,8 @@ 
 # Since: 1.6
 ##
 { 'type': 'DriveBackup',
-  'data': { 'device': 'str', 'target': 'str', '*format': 'str',
+  'data': { 'device': 'str', 'target': 'str',
+            '*target-id': 'str', '*format': 'str',
             '*mode': 'NewImageMode', '*speed': 'int',
             '*on-source-error': 'BlockdevOnError',
             '*on-target-error': 'BlockdevOnError' } }
@@ -1807,6 +1808,7 @@ 
 #          is a device, the existing file/device will be used as the new
 #          destination.  If it does not exist, a new file will be created.
 #
+# @target-id: #optional the drive id of the target.
 # @format: #optional the format of the new destination, default is to
 #          probe if @mode is 'existing', else the format of the source
 #
@@ -1833,7 +1835,8 @@ 
 # Since 1.6
 ##
 { 'command': 'drive-backup',
-  'data': { 'device': 'str', 'target': 'str', '*format': 'str',
+  'data': { 'device': 'str', 'target': 'str',
+            '*target-id': 'str', '*format': 'str',
             '*mode': 'NewImageMode', '*speed': 'int',
             '*on-source-error': 'BlockdevOnError',
             '*on-target-error': 'BlockdevOnError' } }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 362f0e1..3ed03de 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -913,7 +913,7 @@  EQMP
 
     {
         .name       = "drive-backup",
-        .args_type  = "device:B,target:s,speed:i?,mode:s?,format:s?,"
+        .args_type  = "device:B,target:s,target-id:s?,speed:i?,mode:s?,format:s?,"
                       "on-source-error:s?,on-target-error:s?",
         .mhandler.cmd_new = qmp_marshal_input_drive_backup,
     },
@@ -936,6 +936,7 @@  Arguments:
             device, the existing file/device will be used as the new
             destination.  If it does not exist, a new file will be created.
             (json-string)
+- "target-id": the drive id of the target image.
 - "format": the format of the new destination, default is to probe if 'mode' is
             'existing', else the format of the source
             (json-string, optional)