Patchwork [v3,7/8] add mirroring to transaction

login
register
mail settings
Submitter Paolo Bonzini
Date March 5, 2012, 5:34 p.m.
Message ID <1330968842-24635-8-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/144728/
State New
Headers show

Comments

Paolo Bonzini - March 5, 2012, 5:34 p.m.
With it comes a new image creation mode, "no-backing-file", that can
be used to stream an image so that the destination does not need the
original image's backing file(s).

Both bdrv_append and blkmirror will set the backing_hd on the target,
even if the image is created without one, so that both streaming and
copy-on-write work properly (at least with qcow2 or qed, not raw).

Streaming mode works with the following gotchas:

- streaming will rewrite every bit of the source image;

- zero writes are not supported by the blkmirror driver, hence both
  the source and the destination image will grow to full size.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 blockdev.c       |   53 +++++++++++++++++++++++++++++++++++++++++++----------
 qapi-schema.json |   21 +++++++++++++++++++--
 qmp-commands.hx  |   12 +++++++++++-
 3 files changed, 73 insertions(+), 13 deletions(-)
Eric Blake - March 5, 2012, 7:04 p.m.
On 03/05/2012 10:34 AM, Paolo Bonzini wrote:
> With it comes a new image creation mode, "no-backing-file", that can
> be used to stream an image so that the destination does not need the
> original image's backing file(s).
> 
> Both bdrv_append and blkmirror will set the backing_hd on the target,
> even if the image is created without one, so that both streaming and
> copy-on-write work properly (at least with qcow2 or qed, not raw).
> 
> Streaming mode works with the following gotchas:
> 
> - streaming will rewrite every bit of the source image;
> 
> - zero writes are not supported by the blkmirror driver, hence both
>   the source and the destination image will grow to full size.
> 
> @@ -1159,7 +1175,8 @@
>  ##
>  { 'union': 'BlockdevAction',
>    'data': {
> -       'blockdev-snapshot-sync': 'BlockdevSnapshot'
> +       'blockdev-snapshot-sync': 'BlockdevSnapshot',

Minor nit - should we rebase this trailing comma earlier in the series,
to make it obvious that BlockdevAction will grow and for less churn in
this patch?

Patch

diff --git a/blockdev.c b/blockdev.c
index 06c3017..df086d1 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -689,6 +689,7 @@  void qmp_transaction(BlockdevActionList *dev_list, Error **errp)
     int ret = 0;
     BlockdevActionList *dev_entry = dev_list;
     BlkTransactionStates *states;
+    char *new_source = NULL;
 
     QSIMPLEQ_HEAD(snap_bdrv_states, BlkTransactionStates) snap_bdrv_states;
     QSIMPLEQ_INIT(&snap_bdrv_states);
@@ -700,7 +701,8 @@  void qmp_transaction(BlockdevActionList *dev_list, Error **errp)
     while (NULL != dev_entry) {
         BlockdevAction *dev_info = NULL;
         BlockDriver *proto_drv;
-        BlockDriver *drv;
+        BlockDriver *target_drv;
+        BlockDriver *drv = NULL;
         int flags;
         enum NewImageMode mode;
         const char *new_image_file;
@@ -724,16 +726,36 @@  void qmp_transaction(BlockdevActionList *dev_list, Error **errp)
                 format = dev_info->blockdev_snapshot_sync->format;
             }
             mode = dev_info->blockdev_snapshot_sync->mode;
+            new_source = g_strdup(new_image_file);
             break;
+
+        case BLOCKDEV_ACTION_KIND_DRIVE_MIRROR:
+            device = dev_info->drive_mirror->device;
+            drv = bdrv_find_format("blkmirror");
+            if (!dev_info->drive_mirror->has_mode) {
+                dev_info->drive_mirror->mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
+            }
+            new_image_file = dev_info->drive_mirror->target;
+            if (dev_info->drive_mirror->has_format) {
+                format = dev_info->drive_mirror->format;
+            }
+            mode = dev_info->drive_mirror->mode;
+            new_source = g_strdup_printf("blkmirror:%s:%s", format,
+                                         dev_info->drive_mirror->target);
+            break;
+
         default:
             abort();
         }
 
-        drv = bdrv_find_format(format);
-        if (!drv) {
+        target_drv = bdrv_find_format(format);
+        if (!target_drv) {
             error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
             goto delete_and_fail;
         }
+        if (!drv) {
+            drv = target_drv;
+        }
 
         states->old_bs = bdrv_find(device);
         if (!states->old_bs) {
@@ -757,7 +779,7 @@  void qmp_transaction(BlockdevActionList *dev_list, Error **errp)
 
         flags = states->old_bs->open_flags;
 
-        proto_drv = bdrv_find_protocol(new_image_file);
+        proto_drv = bdrv_find_protocol(new_source);
         if (!proto_drv) {
             error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
             goto delete_and_fail;
@@ -765,10 +787,18 @@  void qmp_transaction(BlockdevActionList *dev_list, Error **errp)
 
         /* create new image w/backing file */
         if (mode != NEW_IMAGE_MODE_EXISTING) {
-            ret = bdrv_img_create(new_image_file, format,
-                                  states->old_bs->filename,
-                                  states->old_bs->drv->format_name,
-                                  NULL, -1, flags);
+            if (mode == NEW_IMAGE_MODE_ABSOLUTE_PATHS) {
+                ret = bdrv_img_create(new_image_file, format,
+                                      states->old_bs->filename,
+                                      states->old_bs->drv->format_name,
+                                      NULL, -1, flags);
+            } else {
+                uint64_t size;
+                bdrv_get_geometry(states->old_bs, &size);
+                size *= 512;
+                ret = bdrv_img_create(new_image_file, format,
+                                      NULL, NULL, NULL, size, flags);
+            }
             if (ret) {
                 error_set(errp, QERR_OPEN_FILE_FAILED, new_image_file);
                 goto delete_and_fail;
@@ -777,12 +807,14 @@  void qmp_transaction(BlockdevActionList *dev_list, Error **errp)
 
         /* We will manually add the backing_hd field to the bs later */
         states->new_bs = bdrv_new("");
-        ret = bdrv_open(states->new_bs, new_image_file,
+        ret = bdrv_open(states->new_bs, new_source,
                         flags | BDRV_O_NO_BACKING, drv);
         if (ret != 0) {
-            error_set(errp, QERR_OPEN_FILE_FAILED, new_image_file);
+            error_set(errp, QERR_OPEN_FILE_FAILED, new_source);
             goto delete_and_fail;
         }
+        g_free(new_source);
+        new_source = NULL;
     }
 
 
@@ -810,6 +842,7 @@  exit:
     QSIMPLEQ_FOREACH(states, &snap_bdrv_states, entry) {
         g_free(states);
     }
+    g_free(new_source);
     return;
 }
 
diff --git a/qapi-schema.json b/qapi-schema.json
index 0d24240..a61e90c 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1133,7 +1133,7 @@ 
 # Since: 1.1
 ##
 { 'enum': 'NewImageMode'
-  'data': [ 'existing', 'absolute-paths' ] }
+  'data': [ 'existing', 'absolute-paths', 'no-backing-file' ] }
 
 ##
 # @BlockdevSnapshot
@@ -1152,6 +1152,23 @@ 
             '*mode': 'NewImageMode' } }
 
 ##
+# @BlockdevMirror
+#
+# @device:  the name of the device to start mirroring.
+#
+# @target: the image that will start receiving writes for @device. A new
+#          file will be created unless @mode is "existing".
+#
+# @format: #optional the format of the target image, default is 'qcow2'.
+#
+# @mode: #optional whether and how QEMU should create a new image, default is
+# 'absolute-paths'.
+##
+{ 'type': 'BlockdevMirror',
+  'data': { 'device': 'str', 'target': 'str', '*format': 'str',
+            '*mode': 'NewImageMode' } }
+
+##
 # @BlockdevAction
 #
 # A discriminated record of operations that can be performed with
@@ -1159,7 +1175,8 @@ 
 ##
 { 'union': 'BlockdevAction',
   'data': {
-       'blockdev-snapshot-sync': 'BlockdevSnapshot'
+       'blockdev-snapshot-sync': 'BlockdevSnapshot',
+       'drive-mirror': 'BlockdevMirror',
    } }
 
 ##
diff --git a/qmp-commands.hx b/qmp-commands.hx
index dfe8a5b..9dcafd4 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -701,12 +701,16 @@  operation for now is snapshotting.  If there is any failure performing
 transaction
 -----------
 
-Atomically operate on one or more block devices.  The only supported
-operation for now is snapshotting.  If there is any failure performing
+Atomically operate on one or more block devices, snapshotting them
+or enabling mirrored writes.  If there is any failure performing
 any of the operations, all snapshots for the group are abandoned, and
 the original disks pre-snapshot attempt are used.
 
+Mirrored writes keep the previous image file open, and start writing
+data also to the new file specified in the command.
+
 A list of dictionaries is accepted, that contains the actions to be performed.
+
 For snapshots this is the device, the file to use for the new snapshot,
 and the format.  The default format, if not specified, is qcow2.
 
@@ -716,7 +720,7 @@  Arguments:
 
 actions array:
     - "type": the operation to perform.  The only supported
-      value is "blockdev-snapshot-sync". (json-string)
+      values are "blockdev-snapshot-sync" and "mirror". (json-string)
     - "data": a dictionary.  The contents depend on the value
       of "type".  When "type" is "blockdev-snapshot-sync":
       - "device": device name to snapshot (json-string)
@@ -724,6 +728,12 @@  actions array:
       - "format": format of new image (json-string, optional)
       - "mode": whether and how QEMU should create the snapshot file
         (NewImageMode, optional, default "absolute-paths")
+      When "type" is "drive-mirror":
+      - "device": device name to snapshot (json-string)
+      - "target": name of destination image file (json-string)
+      - "format": format of new image (json-string, optional)
+      - "mode": how QEMU should look for an existing image file
+        (NewImageMode, optional, default "absolute-paths")
 
 Example: