[Qemu-devel] [PATCH v2 0/2] block: add drive_backup HMP command

[Qemu-devel] [PATCH v2 0/2] block: add drive_backup HMP command

[Stefan Hajnoczi]

These patches add the sync mode argument to drive-backup (just like
drive-mirror) and then introduce the drive_backup HMP command.

It's necessary to add the sync mode argument although only the 'full' is
supported, since the drive_mirror HMP command defaults to sync mode 'top'.  To
avoid confusion, we implement the same behavior as drive_mirror and use the
sync mode argument which Ian Main <imain@redhat.com> is currently working on
implementing.

Stefan Hajnoczi (2):
  blockdev: add sync mode to drive-backup QMP command
  block: add drive_backup HMP command

 blockdev.c             |  6 ++++++
 hmp-commands.hx        | 20 ++++++++++++++++++++
 hmp.c                  | 28 ++++++++++++++++++++++++++++
 hmp.h                  |  1 +
 qapi-schema.json       | 14 ++++++++++++--
 qmp-commands.hx        |  6 +++++-
 tests/qemu-iotests/055 | 36 +++++++++++++++++++++---------------
 7 files changed, 93 insertions(+), 18 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [PATCH v2 1/2] blockdev: add sync mode to drive-backup QMP command

[Stefan Hajnoczi]

The drive-backup command is similar to the drive-mirror command, except
no guest data written after the command executes gets copied.  Add a
sync mode argument which determines whether the entire disk is copied,
just allocated clusters, or only clusters being written to by the guest.

Currently only sync mode 'full' is supported - it copies the entire disk.
For read-only point-in-time snapshots we may only need sync mode 'none'
since the target can be a qcow2 file using the guest's disk as its
backing file (no need to copy the entire disk).  Finally, sync mode
'top' is useful if we wish to preserve the backing chain.

Note that this patch just adds the sync mode argument to drive-backup.
It does not implement sync modes 'top' or 'none'.  This patch is
necessary so we can add a drive-backup HMP command that behaves like the
existing drive-mirror HMP command and takes a sync mode.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 blockdev.c             |  6 ++++++
 qapi-schema.json       | 14 ++++++++++++--
 qmp-commands.hx        |  6 +++++-
 tests/qemu-iotests/055 | 36 +++++++++++++++++++++---------------
 4 files changed, 44 insertions(+), 18 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index edca843..fe73278 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -936,6 +936,7 @@ static void drive_backup_prepare(BlkTransactionState *common, Error **errp)
 
     qmp_drive_backup(backup->device, backup->target,
                      backup->has_format, backup->format,
+                     backup->sync,
                      backup->has_mode, backup->mode,
                      backup->has_speed, backup->speed,
                      backup->has_on_source_error, backup->on_source_error,
@@ -1421,6 +1422,7 @@ void qmp_block_commit(const char *device,
 
 void qmp_drive_backup(const char *device, const char *target,
                       bool has_format, const char *format,
+                      enum MirrorSyncMode sync,
                       bool has_mode, enum NewImageMode mode,
                       bool has_speed, int64_t speed,
                       bool has_on_source_error, BlockdevOnError on_source_error,
@@ -1435,6 +1437,10 @@ void qmp_drive_backup(const char *device, const char *target,
     int64_t size;
     int ret;
 
+    if (sync != MIRROR_SYNC_MODE_FULL) {
+        error_setg(errp, "only sync mode 'full' is currently supported");
+        return;
+    }
     if (!has_speed) {
         speed = 0;
     }
diff --git a/qapi-schema.json b/qapi-schema.json
index d6479e1..1e19e4b 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1626,6 +1626,10 @@
 # @format: #optional the format of the new destination, default is to
 #          probe if @mode is 'existing', else the format of the source
 #
+# @sync: what parts of the disk image should be copied to the destination
+#        (all the disk, only the sectors allocated in the topmost image, or
+#        only new I/O).
+#
 # @mode: #optional whether and how QEMU should create a new image, default is
 #        'absolute-paths'.
 #
@@ -1647,7 +1651,8 @@
 ##
 { 'type': 'DriveBackup',
   'data': { 'device': 'str', 'target': 'str', '*format': 'str',
-            '*mode': 'NewImageMode', '*speed': 'int',
+            'sync': 'MirrorSyncMode', '*mode': 'NewImageMode',
+            '*speed': 'int',
             '*on-source-error': 'BlockdevOnError',
             '*on-target-error': 'BlockdevOnError' } }
 
@@ -1807,6 +1812,10 @@
 #
 # @speed: #optional the maximum speed, in bytes per second
 #
+# @sync: what parts of the disk image should be copied to the destination
+#        (all the disk, only the sectors allocated in the topmost image, or
+#        only new I/O).
+#
 # @on-source-error: #optional the action to take on an error on the source,
 #                   default 'report'.  'stop' and 'enospc' can only be used
 #                   if the block device supports io-status (see BlockInfo).
@@ -1826,7 +1835,8 @@
 ##
 { 'command': 'drive-backup',
   'data': { 'device': 'str', 'target': 'str', '*format': 'str',
-            '*mode': 'NewImageMode', '*speed': 'int',
+            'sync': 'MirrorSyncMode', '*mode': 'NewImageMode',
+            '*speed': 'int',
             '*on-source-error': 'BlockdevOnError',
             '*on-target-error': 'BlockdevOnError' } }
 
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 362f0e1..e075df4 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  = "sync:s,device:B,target:s,speed:i?,mode:s?,format:s?,"
                       "on-source-error:s?,on-target-error:s?",
         .mhandler.cmd_new = qmp_marshal_input_drive_backup,
     },
@@ -939,6 +939,10 @@ Arguments:
 - "format": the format of the new destination, default is to probe if 'mode' is
             'existing', else the format of the source
             (json-string, optional)
+- "sync": what parts of the disk image should be copied to the destination;
+  possibilities include "full" for all the disk, "top" for only the sectors
+  allocated in the topmost image, or "none" to only replicate new I/O
+  (MirrorSyncMode).
 - "mode": whether and how QEMU should create a new image
           (NewImageMode, optional, default 'absolute-paths')
 - "speed": the maximum speed, in bytes per second (json-int, optional)
diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055
index 887c959..c66f8db 100755
--- a/tests/qemu-iotests/055
+++ b/tests/qemu-iotests/055
@@ -54,7 +54,7 @@ class TestSingleDrive(iotests.QMPTestCase):
         self.assert_no_active_block_jobs()
 
         result = self.vm.qmp('drive-backup', device='drive0',
-                             target=target_img)
+                             target=target_img, sync='full')
         self.assert_qmp(result, 'return', {})
 
         event = self.cancel_and_wait()
@@ -64,7 +64,7 @@ class TestSingleDrive(iotests.QMPTestCase):
         self.assert_no_active_block_jobs()
 
         result = self.vm.qmp('drive-backup', device='drive0',
-                             target=target_img)
+                             target=target_img, sync='full')
         self.assert_qmp(result, 'return', {})
 
         result = self.vm.qmp('block-job-pause', device='drive0')
@@ -89,17 +89,17 @@ class TestSingleDrive(iotests.QMPTestCase):
 
     def test_medium_not_found(self):
         result = self.vm.qmp('drive-backup', device='ide1-cd0',
-                             target=target_img)
+                             target=target_img, sync='full')
         self.assert_qmp(result, 'error/class', 'GenericError')
 
     def test_image_not_found(self):
         result = self.vm.qmp('drive-backup', device='drive0',
-                             mode='existing', target=target_img)
+                             target=target_img, sync='full', mode='existing')
         self.assert_qmp(result, 'error/class', 'GenericError')
 
     def test_device_not_found(self):
         result = self.vm.qmp('drive-backup', device='nonexistent',
-                             target=target_img)
+                             target=target_img, sync='full')
         self.assert_qmp(result, 'error/class', 'DeviceNotFound')
 
 class TestSetSpeed(iotests.QMPTestCase):
@@ -119,7 +119,7 @@ class TestSetSpeed(iotests.QMPTestCase):
         self.assert_no_active_block_jobs()
 
         result = self.vm.qmp('drive-backup', device='drive0',
-                             target=target_img)
+                             target=target_img, sync='full')
         self.assert_qmp(result, 'return', {})
 
         # Default speed is 0
@@ -140,7 +140,7 @@ class TestSetSpeed(iotests.QMPTestCase):
 
         # Check setting speed in drive-backup works
         result = self.vm.qmp('drive-backup', device='drive0',
-                             target=target_img, speed=4*1024*1024)
+                             target=target_img, sync='full', speed=4*1024*1024)
         self.assert_qmp(result, 'return', {})
 
         result = self.vm.qmp('query-block-jobs')
@@ -154,13 +154,13 @@ class TestSetSpeed(iotests.QMPTestCase):
         self.assert_no_active_block_jobs()
 
         result = self.vm.qmp('drive-backup', device='drive0',
-                             target=target_img, speed=-1)
+                             target=target_img, sync='full', speed=-1)
         self.assert_qmp(result, 'error/class', 'GenericError')
 
         self.assert_no_active_block_jobs()
 
         result = self.vm.qmp('drive-backup', device='drive0',
-                             target=target_img)
+                             target=target_img, sync='full')
         self.assert_qmp(result, 'return', {})
 
         result = self.vm.qmp('block-job-set-speed', device='drive0', speed=-1)
@@ -196,7 +196,8 @@ class TestSingleTransaction(iotests.QMPTestCase):
         result = self.vm.qmp('transaction', actions=[{
                 'type': 'drive-backup',
                 'data': { 'device': 'drive0',
-                          'target': target_img },
+                          'target': target_img,
+                          'sync': 'full' },
             }
         ])
         self.assert_qmp(result, 'return', {})
@@ -210,7 +211,8 @@ class TestSingleTransaction(iotests.QMPTestCase):
         result = self.vm.qmp('transaction', actions=[{
                 'type': 'drive-backup',
                 'data': { 'device': 'drive0',
-                          'target': target_img },
+                          'target': target_img,
+                          'sync': 'full' },
             }
         ])
         self.assert_qmp(result, 'return', {})
@@ -239,7 +241,8 @@ class TestSingleTransaction(iotests.QMPTestCase):
         result = self.vm.qmp('transaction', actions=[{
                 'type': 'drive-backup',
                 'data': { 'device': 'ide1-cd0',
-                          'target': target_img },
+                          'target': target_img,
+                          'sync': 'full' },
             }
         ])
         self.assert_qmp(result, 'error/class', 'GenericError')
@@ -249,7 +252,8 @@ class TestSingleTransaction(iotests.QMPTestCase):
                 'type': 'drive-backup',
                 'data': { 'device': 'drive0',
                           'mode': 'existing',
-                          'target': target_img },
+                          'target': target_img,
+                          'sync': 'full' },
             }
         ])
         self.assert_qmp(result, 'error/class', 'GenericError')
@@ -259,7 +263,8 @@ class TestSingleTransaction(iotests.QMPTestCase):
                 'type': 'drive-backup',
                 'data': { 'device': 'nonexistent',
                           'mode': 'existing',
-                          'target': target_img },
+                          'target': target_img,
+                          'sync': 'full' },
             }
         ])
         self.assert_qmp(result, 'error/class', 'DeviceNotFound')
@@ -269,7 +274,8 @@ class TestSingleTransaction(iotests.QMPTestCase):
                 'type': 'drive-backup',
                 'data': { 'device': 'nonexistent',
                           'mode': 'existing',
-                          'target': target_img },
+                          'target': target_img,
+                          'sync': 'full' },
             }, {
                 'type': 'Abort',
                 'data': {},
-- 
1.8.1.4

[Qemu-devel] [PATCH v2 2/2] block: add drive_backup HMP command

[Stefan Hajnoczi]

Make "drive_backup" available on the HMP monitor:

  drive_backup [-n] [-f] device target [format]

The -n flag requests QEMU to reuse the image found in new-image-file,
instead of recreating it from scratch.

The -f flag requests QEMU to copy the whole disk, so that the result
does not need a backing file.  Note that this flag *must* currently be
passed since the other sync modes ('none' and 'top') have not been
implemented yet.  Requiring it ensures that "drive_backup" behaves like
"drive_mirror".

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hmp-commands.hx | 20 ++++++++++++++++++++
 hmp.c           | 28 ++++++++++++++++++++++++++++
 hmp.h           |  1 +
 3 files changed, 49 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 915b0d1..800101b 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1059,6 +1059,26 @@ using the specified target.
 ETEXI
 
     {
+        .name       = "drive_backup",
+        .args_type  = "reuse:-n,full:-f,device:B,target:s,format:s?",
+        .params     = "[-n] [-f] device target [format]",
+        .help       = "initiates a point-in-time\n\t\t\t"
+                      "copy for a device. The device's contents are\n\t\t\t"
+                      "copied to the new image file, excluding data that\n\t\t\t"
+                      "is written after the command is started.\n\t\t\t"
+                      "The -n flag requests QEMU to reuse the image found\n\t\t\t"
+                      "in new-image-file, instead of recreating it from scratch.\n\t\t\t"
+                      "The -f flag requests QEMU to copy the whole disk,\n\t\t\t"
+                      "so that the result does not need a backing file.\n\t\t\t",
+        .mhandler.cmd = hmp_drive_backup,
+    },
+STEXI
+@item drive_backup
+@findex drive_backup
+Start a point-in-time copy of a block device to a specificed target.
+ETEXI
+
+    {
         .name       = "drive_add",
         .args_type  = "pci_addr:s,opts:s",
         .params     = "[[<domain>:]<bus>:]<slot>\n"
diff --git a/hmp.c b/hmp.c
index 494a9aa..f9fb999 100644
--- a/hmp.c
+++ b/hmp.c
@@ -889,6 +889,34 @@ void hmp_drive_mirror(Monitor *mon, const QDict *qdict)
     hmp_handle_error(mon, &errp);
 }
 
+void hmp_drive_backup(Monitor *mon, const QDict *qdict)
+{
+    const char *device = qdict_get_str(qdict, "device");
+    const char *filename = qdict_get_str(qdict, "target");
+    const char *format = qdict_get_try_str(qdict, "format");
+    int reuse = qdict_get_try_bool(qdict, "reuse", 0);
+    int full = qdict_get_try_bool(qdict, "full", 0);
+    enum NewImageMode mode;
+    Error *errp = NULL;
+
+    if (!filename) {
+        error_set(&errp, QERR_MISSING_PARAMETER, "target");
+        hmp_handle_error(mon, &errp);
+        return;
+    }
+
+    if (reuse) {
+        mode = NEW_IMAGE_MODE_EXISTING;
+    } else {
+        mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
+    }
+
+    qmp_drive_backup(device, filename, !!format, format,
+                     full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP,
+                     true, mode, false, 0, false, 0, false, 0, &errp);
+    hmp_handle_error(mon, &errp);
+}
+
 void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict)
 {
     const char *device = qdict_get_str(qdict, "device");
diff --git a/hmp.h b/hmp.h
index 56d2e92..6c3bdcd 100644
--- a/hmp.h
+++ b/hmp.h
@@ -55,6 +55,7 @@ void hmp_balloon(Monitor *mon, const QDict *qdict);
 void hmp_block_resize(Monitor *mon, const QDict *qdict);
 void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict);
 void hmp_drive_mirror(Monitor *mon, const QDict *qdict);
+void hmp_drive_backup(Monitor *mon, const QDict *qdict);
 void hmp_migrate_cancel(Monitor *mon, const QDict *qdict);
 void hmp_migrate_set_downtime(Monitor *mon, const QDict *qdict);
 void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict);
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH v2 0/2] block: add drive_backup HMP command

[Kevin Wolf]

Am 26.06.2013 um 14:11 hat Stefan Hajnoczi geschrieben:
> These patches add the sync mode argument to drive-backup (just like
> drive-mirror) and then introduce the drive_backup HMP command.
> 
> It's necessary to add the sync mode argument although only the 'full' is
> supported, since the drive_mirror HMP command defaults to sync mode 'top'.  To
> avoid confusion, we implement the same behavior as drive_mirror and use the
> sync mode argument which Ian Main <imain@redhat.com> is currently working on
> implementing.
> 
> Stefan Hajnoczi (2):
>   blockdev: add sync mode to drive-backup QMP command
>   block: add drive_backup HMP command
> 
>  blockdev.c             |  6 ++++++
>  hmp-commands.hx        | 20 ++++++++++++++++++++
>  hmp.c                  | 28 ++++++++++++++++++++++++++++
>  hmp.h                  |  1 +
>  qapi-schema.json       | 14 ++++++++++++--
>  qmp-commands.hx        |  6 +++++-
>  tests/qemu-iotests/055 | 36 +++++++++++++++++++++---------------
>  7 files changed, 93 insertions(+), 18 deletions(-)

Thanks, applied to the block branch.

Kevin

Re: [Qemu-devel] [PATCH v2 1/2] blockdev: add sync mode to drive-backup QMP command

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2584 bytes --]

On 06/26/2013 06:11 AM, Stefan Hajnoczi wrote:
> The drive-backup command is similar to the drive-mirror command, except
> no guest data written after the command executes gets copied.  Add a
> sync mode argument which determines whether the entire disk is copied,
> just allocated clusters, or only clusters being written to by the guest.
> 
> Currently only sync mode 'full' is supported - it copies the entire disk.
> For read-only point-in-time snapshots we may only need sync mode 'none'
> since the target can be a qcow2 file using the guest's disk as its
> backing file (no need to copy the entire disk).  Finally, sync mode
> 'top' is useful if we wish to preserve the backing chain.
> 
> Note that this patch just adds the sync mode argument to drive-backup.
> It does not implement sync modes 'top' or 'none'.  This patch is
> necessary so we can add a drive-backup HMP command that behaves like the
> existing drive-mirror HMP command and takes a sync mode.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---

> +++ b/qapi-schema.json
> @@ -1626,6 +1626,10 @@
>  # @format: #optional the format of the new destination, default is to
>  #          probe if @mode is 'existing', else the format of the source
>  #
> +# @sync: what parts of the disk image should be copied to the destination
> +#        (all the disk, only the sectors allocated in the topmost image, or
> +#        only new I/O).
> +#
>  # @mode: #optional whether and how QEMU should create a new image, default is
>  #        'absolute-paths'.
>  #
> @@ -1647,7 +1651,8 @@
>  ##
>  { 'type': 'DriveBackup',
>    'data': { 'device': 'str', 'target': 'str', '*format': 'str',
> -            '*mode': 'NewImageMode', '*speed': 'int',
> +            'sync': 'MirrorSyncMode', '*mode': 'NewImageMode',

Making it mandatory is fine, since the command is new to 1.6 (if this
were after 1.6, we'd have to make it optional).

>  #
> +# @sync: what parts of the disk image should be copied to the destination
> +#        (all the disk, only the sectors allocated in the topmost image, or
> +#        only new I/O).

This duplication will conflict with Kevin's patch currently on the qmp
queue for 'qapi-schema: Use existing type for drive-backup arguments';
you may want to coordinate which series gets merged first, and rebase
the other one appropriately.

If you do rebase, feel free to add:
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH v2 1/2] blockdev: add sync mode to drive-backup QMP command

[Kevin Wolf]

Am 10.07.2013 um 16:31 hat Eric Blake geschrieben:
> On 06/26/2013 06:11 AM, Stefan Hajnoczi wrote:
> > The drive-backup command is similar to the drive-mirror command, except
> > no guest data written after the command executes gets copied.  Add a
> > sync mode argument which determines whether the entire disk is copied,
> > just allocated clusters, or only clusters being written to by the guest.
> > 
> > Currently only sync mode 'full' is supported - it copies the entire disk.
> > For read-only point-in-time snapshots we may only need sync mode 'none'
> > since the target can be a qcow2 file using the guest's disk as its
> > backing file (no need to copy the entire disk).  Finally, sync mode
> > 'top' is useful if we wish to preserve the backing chain.
> > 
> > Note that this patch just adds the sync mode argument to drive-backup.
> > It does not implement sync modes 'top' or 'none'.  This patch is
> > necessary so we can add a drive-backup HMP command that behaves like the
> > existing drive-mirror HMP command and takes a sync mode.
> > 
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> 
> > +++ b/qapi-schema.json
> > @@ -1626,6 +1626,10 @@
> >  # @format: #optional the format of the new destination, default is to
> >  #          probe if @mode is 'existing', else the format of the source
> >  #
> > +# @sync: what parts of the disk image should be copied to the destination
> > +#        (all the disk, only the sectors allocated in the topmost image, or
> > +#        only new I/O).
> > +#
> >  # @mode: #optional whether and how QEMU should create a new image, default is
> >  #        'absolute-paths'.
> >  #
> > @@ -1647,7 +1651,8 @@
> >  ##
> >  { 'type': 'DriveBackup',
> >    'data': { 'device': 'str', 'target': 'str', '*format': 'str',
> > -            '*mode': 'NewImageMode', '*speed': 'int',
> > +            'sync': 'MirrorSyncMode', '*mode': 'NewImageMode',
> 
> Making it mandatory is fine, since the command is new to 1.6 (if this
> were after 1.6, we'd have to make it optional).
> 
> >  #
> > +# @sync: what parts of the disk image should be copied to the destination
> > +#        (all the disk, only the sectors allocated in the topmost image, or
> > +#        only new I/O).
> 
> This duplication will conflict with Kevin's patch currently on the qmp
> queue for 'qapi-schema: Use existing type for drive-backup arguments';
> you may want to coordinate which series gets merged first, and rebase
> the other one appropriately.

Luiz said he's going to send his pull request today; then I'll rebase
my tree and fix this conflict tomorrow. (It's trivial enough that we
don't have to go through another version.)

> If you do rebase, feel free to add:
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks, I've added it.

Kevin