[Qemu-devel] [PATCH 0/3] block: block-backup live backup command

[Qemu-devel] [PATCH 0/3] block: block-backup live backup command

[Stefan Hajnoczi]

This series adds a new QMP command, block-backup, which takes a point-in-time
snapshot of a block device.  The snapshot is copied out to a target block
device.  A simple example is:

  block-backup device=virtio0 format=qcow2 target=backup-20130401.qcow2

The original block-backup blockjob was written by Dietmar Maurer
<dietmar@proxmox.com>.  He is currently busy but I feel the feature is worth
pushing into QEMU since there has been interest.  This is my version of his
patch, plus the QMP command and qemu-iotests test case.

How is this different from block-stream and drive-mirror?
---------------------------------------------------------
Both block-stream and drive-mirror do not provide immediate point-in-time
snapshots.  Instead they copy data into a new file and then switch to it.  In
other words, the point at which the "snapshot" is taken cannot be controlled
directly.

block-backup intercepts guest writes and saves data into the target block
device before it is overwritten.  The target block device can be a raw image
file, backing files are not used to implement this feature.

How can block-backup be used?
-----------------------------
The simplest use-case is to copy a point-in-time snapshot to a local file.

More advanced users may wish to make the target an NBD URL.  The NBD server
listening on the other side can process the backup writes any way it wishes.  I
previously posted an RFC series with a backup server that streamed Dietmar's
VMA backup archive format.

What's next for block-backup?
-----------------------------
The following enhancements are left for future patches:

1. QMP 'transaction' support.  It is handy to atomically snapshot multiple
   block devices.  We need qmp_transaction() support for this.  Wenchao Xia is
   currently making qmp_transaction() extensible so new action types, like
   block-backup, can be added.

2. Sync modes like drive-mirror (top, full, none).  This makes it possible to
   preserve the backing file chain.

Dietmar Maurer (1):
  block: add basic backup support to block driver

Stefan Hajnoczi (2):
  block: add block_backup QMP command
  qemu-iotests: add 054 block-backup test case

 block.c                    |  69 ++++++++++++-
 block/Makefile.objs        |   1 +
 block/backup.c             | 252 +++++++++++++++++++++++++++++++++++++++++++++
 blockdev.c                 |  92 +++++++++++++++++
 include/block/block.h      |   2 +
 include/block/block_int.h  |  16 +++
 include/block/blockjob.h   |  10 ++
 qapi-schema.json           |  28 +++++
 qmp-commands.hx            |   6 ++
 tests/qemu-iotests/054     | 230 +++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/054.out |   5 +
 tests/qemu-iotests/group   |   1 +
 12 files changed, 707 insertions(+), 5 deletions(-)
 create mode 100644 block/backup.c
 create mode 100755 tests/qemu-iotests/054
 create mode 100644 tests/qemu-iotests/054.out

-- 
1.8.1.4

[Qemu-devel] [PATCH 1/3] block: add basic backup support to block driver

[Stefan Hajnoczi]

From: Dietmar Maurer <dietmar@proxmox.com>

backup_start() creates a block job that copies a point-in-time snapshot
of a block device to a target block device.

We call backup_do_cow() for each write during backup. That function
reads the original data from the block device before it gets
overwritten.  The data is then written to the target device.

The tracked_request infrastructure is used to serialize access.  Both
reads and writes are serialized if they overlap.

Currently backup cluster size is hardcoded to 65536 bytes.

[I made a number of changes to Dietmar's original patch and folded them
in to make code review easy.  Here is the full list:

 * Drop BackupDumpFunc interface in favor of a target block device
 * Detect zero clusters with buffer_is_zero()
 * Don't write zero clusters to the target
 * Use 0 delay instead of 1us, like other block jobs
 * Unify creation/start functions into backup_start()
 * Simplify cleanup, free bitmap in backup_run() instead of cb function
 * Use HBitmap to avoid duplicating bitmap code
 * Use bdrv_getlength() instead of accessing ->total_sectors directly
 * Delete the backup.h header file, it is no longer necessary
 * Move ./backup.c to block/backup.c
 * Remove #ifdefed out code
 * Coding style and whitespace cleanups

-- stefanha]

Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block.c                   |  69 ++++++++++++-
 block/Makefile.objs       |   1 +
 block/backup.c            | 252 ++++++++++++++++++++++++++++++++++++++++++++++
 include/block/block.h     |   2 +
 include/block/block_int.h |  16 +++
 include/block/blockjob.h  |  10 ++
 6 files changed, 345 insertions(+), 5 deletions(-)
 create mode 100644 block/backup.c

diff --git a/block.c b/block.c
index aa9a533..c5c09b7 100644
--- a/block.c
+++ b/block.c
@@ -54,6 +54,7 @@
 typedef enum {
     BDRV_REQ_COPY_ON_READ = 0x1,
     BDRV_REQ_ZERO_WRITE   = 0x2,
+    BDRV_REQ_BACKUP_ONLY  = 0x4,
 } BdrvRequestFlags;
 
 static void bdrv_dev_change_media_cb(BlockDriverState *bs, bool load);
@@ -1902,6 +1903,22 @@ void bdrv_round_to_clusters(BlockDriverState *bs,
     }
 }
 
+/**
+ * Round a region to job cluster boundaries
+ */
+static void round_to_job_clusters(BlockDriverState *bs,
+                                  int64_t sector_num, int nb_sectors,
+                                  int job_cluster_size,
+                                  int64_t *cluster_sector_num,
+                                  int *cluster_nb_sectors)
+{
+    int64_t c = job_cluster_size / BDRV_SECTOR_SIZE;
+
+    *cluster_sector_num = QEMU_ALIGN_DOWN(sector_num, c);
+    *cluster_nb_sectors = QEMU_ALIGN_UP(sector_num - *cluster_sector_num +
+                                        nb_sectors, c);
+}
+
 static bool tracked_request_overlaps(BdrvTrackedRequest *req,
                                      int64_t sector_num, int nb_sectors) {
     /*        aaaa   bbbb */
@@ -1916,7 +1933,9 @@ static bool tracked_request_overlaps(BdrvTrackedRequest *req,
 }
 
 static void coroutine_fn wait_for_overlapping_requests(BlockDriverState *bs,
-        int64_t sector_num, int nb_sectors)
+                                                       int64_t sector_num,
+                                                       int nb_sectors,
+                                                       int job_cluster_size)
 {
     BdrvTrackedRequest *req;
     int64_t cluster_sector_num;
@@ -1932,6 +1951,11 @@ static void coroutine_fn wait_for_overlapping_requests(BlockDriverState *bs,
     bdrv_round_to_clusters(bs, sector_num, nb_sectors,
                            &cluster_sector_num, &cluster_nb_sectors);
 
+    if (job_cluster_size) {
+        round_to_job_clusters(bs, sector_num, nb_sectors, job_cluster_size,
+                              &cluster_sector_num, &cluster_nb_sectors);
+    }
+
     do {
         retry = false;
         QLIST_FOREACH(req, &bs->tracked_requests, list) {
@@ -2507,12 +2531,24 @@ static int coroutine_fn bdrv_co_do_readv(BlockDriverState *bs,
         bs->copy_on_read_in_flight++;
     }
 
-    if (bs->copy_on_read_in_flight) {
-        wait_for_overlapping_requests(bs, sector_num, nb_sectors);
+    int job_cluster_size = bs->job && bs->job->cluster_size ?
+        bs->job->cluster_size : 0;
+
+    if (bs->copy_on_read_in_flight || job_cluster_size) {
+        wait_for_overlapping_requests(bs, sector_num, nb_sectors,
+                                      job_cluster_size);
     }
 
     tracked_request_begin(&req, bs, sector_num, nb_sectors, false);
 
+    if (bs->job && bs->job->job_type->before_read) {
+        ret = bs->job->job_type->before_read(bs, sector_num, nb_sectors, qiov);
+        if ((ret < 0) || (flags & BDRV_REQ_BACKUP_ONLY)) {
+            /* Note: We do not return any data to the caller */
+            goto out;
+        }
+    }
+
     if (flags & BDRV_REQ_COPY_ON_READ) {
         int pnum;
 
@@ -2556,6 +2592,17 @@ int coroutine_fn bdrv_co_copy_on_readv(BlockDriverState *bs,
                             BDRV_REQ_COPY_ON_READ);
 }
 
+int coroutine_fn bdrv_co_backup(BlockDriverState *bs,
+    int64_t sector_num, int nb_sectors)
+{
+    if (!bs->job) {
+        return -ENOTSUP;
+    }
+
+    return bdrv_co_do_readv(bs, sector_num, nb_sectors, NULL,
+                            BDRV_REQ_BACKUP_ONLY);
+}
+
 static int coroutine_fn bdrv_co_do_write_zeroes(BlockDriverState *bs,
     int64_t sector_num, int nb_sectors)
 {
@@ -2613,12 +2660,23 @@ static int coroutine_fn bdrv_co_do_writev(BlockDriverState *bs,
         bdrv_io_limits_intercept(bs, true, nb_sectors);
     }
 
-    if (bs->copy_on_read_in_flight) {
-        wait_for_overlapping_requests(bs, sector_num, nb_sectors);
+    int job_cluster_size = bs->job && bs->job->cluster_size ?
+        bs->job->cluster_size : 0;
+
+    if (bs->copy_on_read_in_flight || job_cluster_size) {
+        wait_for_overlapping_requests(bs, sector_num, nb_sectors,
+                                      job_cluster_size);
     }
 
     tracked_request_begin(&req, bs, sector_num, nb_sectors, true);
 
+    if (bs->job && bs->job->job_type->before_write) {
+        ret = bs->job->job_type->before_write(bs, sector_num, nb_sectors, qiov);
+        if (ret < 0) {
+            goto out;
+        }
+    }
+
     if (flags & BDRV_REQ_ZERO_WRITE) {
         ret = bdrv_co_do_write_zeroes(bs, sector_num, nb_sectors);
     } else {
@@ -2637,6 +2695,7 @@ static int coroutine_fn bdrv_co_do_writev(BlockDriverState *bs,
         bs->wr_highest_sector = sector_num + nb_sectors - 1;
     }
 
+out:
     tracked_request_end(&req);
 
     return ret;
diff --git a/block/Makefile.objs b/block/Makefile.objs
index 6c4b5bc..dcab6d3 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -19,5 +19,6 @@ endif
 common-obj-y += stream.o
 common-obj-y += commit.o
 common-obj-y += mirror.o
+common-obj-y += backup.o
 
 $(obj)/curl.o: QEMU_CFLAGS+=$(CURL_CFLAGS)
diff --git a/block/backup.c b/block/backup.c
new file mode 100644
index 0000000..45d8c21
--- /dev/null
+++ b/block/backup.c
@@ -0,0 +1,252 @@
+/*
+ * QEMU backup
+ *
+ * Copyright (C) 2013 Proxmox Server Solutions
+ *
+ * Authors:
+ *  Dietmar Maurer (dietmar@proxmox.com)
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include <stdio.h>
+#include <errno.h>
+#include <unistd.h>
+
+#include "block/block.h"
+#include "block/block_int.h"
+#include "block/blockjob.h"
+#include "qemu/ratelimit.h"
+
+#define DEBUG_BACKUP 0
+
+#define DPRINTF(fmt, ...) \
+    do { \
+        if (DEBUG_BACKUP) { \
+            fprintf(stderr, "backup: " fmt, ## __VA_ARGS__); \
+        } \
+    } while (0)
+
+#define BACKUP_CLUSTER_BITS 16
+#define BACKUP_CLUSTER_SIZE (1 << BACKUP_CLUSTER_BITS)
+#define BACKUP_BLOCKS_PER_CLUSTER (BACKUP_CLUSTER_SIZE / BDRV_SECTOR_SIZE)
+
+#define SLICE_TIME 100000000ULL /* ns */
+
+typedef struct BackupBlockJob {
+    BlockJob common;
+    BlockDriverState *target;
+    RateLimit limit;
+    CoRwlock rwlock;
+    uint64_t sectors_read;
+    HBitmap *bitmap;
+} BackupBlockJob;
+
+static int coroutine_fn backup_do_cow(BlockDriverState *bs,
+                                      int64_t sector_num, int nb_sectors)
+{
+    assert(bs);
+    BackupBlockJob *job = (BackupBlockJob *)bs->job;
+    assert(job);
+
+    BlockDriver *drv = bs->drv;
+    struct iovec iov;
+    QEMUIOVector bounce_qiov;
+    void *bounce_buffer = NULL;
+    int ret = 0;
+
+    qemu_co_rwlock_rdlock(&job->rwlock);
+
+    int64_t start, end;
+
+    start = sector_num / BACKUP_BLOCKS_PER_CLUSTER;
+    end = DIV_ROUND_UP(sector_num + nb_sectors, BACKUP_BLOCKS_PER_CLUSTER);
+
+    DPRINTF("brdv_co_backup_cow enter %s C%" PRId64 " %" PRId64 " %d\n",
+            bdrv_get_device_name(bs), start, sector_num, nb_sectors);
+
+    for (; start < end; start++) {
+        if (hbitmap_get(job->bitmap, start)) {
+            DPRINTF("brdv_co_backup_cow skip C%" PRId64 "\n", start);
+            continue; /* already copied */
+        }
+
+        /* immediately set bitmap (avoid coroutine race) */
+        hbitmap_set(job->bitmap, start, 1);
+
+        DPRINTF("brdv_co_backup_cow C%" PRId64 "\n", start);
+
+        if (!bounce_buffer) {
+            iov.iov_len = BACKUP_CLUSTER_SIZE;
+            iov.iov_base = bounce_buffer = qemu_blockalign(bs, iov.iov_len);
+            qemu_iovec_init_external(&bounce_qiov, &iov, 1);
+        }
+
+        ret = drv->bdrv_co_readv(bs, start * BACKUP_BLOCKS_PER_CLUSTER,
+                                 BACKUP_BLOCKS_PER_CLUSTER,
+                                 &bounce_qiov);
+        if (ret < 0) {
+            DPRINTF("brdv_co_backup_cow bdrv_read C%" PRId64 " failed\n",
+                    start);
+            goto out;
+        }
+
+        job->sectors_read += BACKUP_BLOCKS_PER_CLUSTER;
+
+        if (!buffer_is_zero(bounce_buffer, BACKUP_CLUSTER_SIZE)) {
+            ret = bdrv_co_writev(job->target, start * BACKUP_BLOCKS_PER_CLUSTER,
+                                 BACKUP_BLOCKS_PER_CLUSTER,
+                                 &bounce_qiov);
+            if (ret < 0) {
+                DPRINTF("brdv_co_backup_cow dump_cluster_cb C%" PRId64
+                        " failed\n", start);
+                goto out;
+            }
+        }
+
+        DPRINTF("brdv_co_backup_cow done C%" PRId64 "\n", start);
+    }
+
+out:
+    if (bounce_buffer) {
+        qemu_vfree(bounce_buffer);
+    }
+
+    qemu_co_rwlock_unlock(&job->rwlock);
+
+    return ret;
+}
+
+static int coroutine_fn backup_before_read(BlockDriverState *bs,
+                                           int64_t sector_num,
+                                           int nb_sectors, QEMUIOVector *qiov)
+{
+    return backup_do_cow(bs, sector_num, nb_sectors);
+}
+
+static int coroutine_fn backup_before_write(BlockDriverState *bs,
+                                            int64_t sector_num,
+                                            int nb_sectors, QEMUIOVector *qiov)
+{
+    return backup_do_cow(bs, sector_num, nb_sectors);
+}
+
+static void backup_set_speed(BlockJob *job, int64_t speed, Error **errp)
+{
+    BackupBlockJob *s = container_of(job, BackupBlockJob, common);
+
+    if (speed < 0) {
+        error_set(errp, QERR_INVALID_PARAMETER, "speed");
+        return;
+    }
+    ratelimit_set_speed(&s->limit, speed / BDRV_SECTOR_SIZE, SLICE_TIME);
+}
+
+static BlockJobType backup_job_type = {
+    .instance_size = sizeof(BackupBlockJob),
+    .before_read = backup_before_read,
+    .before_write = backup_before_write,
+    .job_type = "backup",
+    .set_speed = backup_set_speed,
+};
+
+static void coroutine_fn backup_run(void *opaque)
+{
+    BackupBlockJob *job = opaque;
+    BlockDriverState *bs = job->common.bs;
+    assert(bs);
+
+    int64_t start, end;
+
+    start = 0;
+    end = DIV_ROUND_UP(bdrv_getlength(bs) / BDRV_SECTOR_SIZE,
+                       BACKUP_BLOCKS_PER_CLUSTER);
+
+    job->bitmap = hbitmap_alloc(end, 0);
+
+    DPRINTF("backup_run start %s %" PRId64 " %" PRId64 "\n",
+            bdrv_get_device_name(bs), start, end);
+
+    int ret = 0;
+
+    for (; start < end; start++) {
+        if (block_job_is_cancelled(&job->common)) {
+            ret = -1;
+            break;
+        }
+
+        /* we need to yield so that qemu_aio_flush() returns.
+         * (without, VM does not reboot)
+         */
+        if (job->common.speed) {
+            uint64_t delay_ns = ratelimit_calculate_delay(
+                &job->limit, job->sectors_read);
+            job->sectors_read = 0;
+            block_job_sleep_ns(&job->common, rt_clock, delay_ns);
+        } else {
+            block_job_sleep_ns(&job->common, rt_clock, 0);
+        }
+
+        if (block_job_is_cancelled(&job->common)) {
+            ret = -1;
+            break;
+        }
+
+        if (hbitmap_get(job->bitmap, start)) {
+            continue; /* already copied */
+        }
+
+        DPRINTF("backup_run loop C%" PRId64 "\n", start);
+
+        /**
+         * This triggers a cluster copy
+         * Note: avoid direct call to brdv_co_backup_cow, because
+         * this does not call tracked_request_begin()
+         */
+        ret = bdrv_co_backup(bs, start*BACKUP_BLOCKS_PER_CLUSTER, 1);
+        if (ret < 0) {
+            break;
+        }
+        /* Publish progress */
+        job->common.offset += BACKUP_CLUSTER_SIZE;
+    }
+
+    /* wait until pending backup_do_cow()calls have completed */
+    qemu_co_rwlock_wrlock(&job->rwlock);
+    qemu_co_rwlock_unlock(&job->rwlock);
+
+    hbitmap_free(job->bitmap);
+
+    bdrv_delete(job->target);
+
+    DPRINTF("backup_run complete %d\n", ret);
+    block_job_completed(&job->common, ret);
+}
+
+void backup_start(BlockDriverState *bs, BlockDriverState *target,
+                  int64_t speed,
+                  BlockDriverCompletionFunc *cb, void *opaque,
+                  Error **errp)
+{
+    assert(bs);
+    assert(target);
+    assert(cb);
+
+    DPRINTF("backup_start %s\n", bdrv_get_device_name(bs));
+
+    BackupBlockJob *job = block_job_create(&backup_job_type, bs, speed,
+                                           cb, opaque, errp);
+    if (!job) {
+        return;
+    }
+
+    qemu_co_rwlock_init(&job->rwlock);
+
+    job->target = target;
+    job->common.cluster_size = BACKUP_CLUSTER_SIZE;
+    job->common.len = bdrv_getlength(bs);
+    job->common.co = qemu_coroutine_create(backup_run);
+    qemu_coroutine_enter(job->common.co, job);
+}
diff --git a/include/block/block.h b/include/block/block.h
index 1251c5c..19a41f4 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -178,6 +178,8 @@ int coroutine_fn bdrv_co_readv(BlockDriverState *bs, int64_t sector_num,
     int nb_sectors, QEMUIOVector *qiov);
 int coroutine_fn bdrv_co_copy_on_readv(BlockDriverState *bs,
     int64_t sector_num, int nb_sectors, QEMUIOVector *qiov);
+int coroutine_fn bdrv_co_backup(BlockDriverState *bs,
+    int64_t sector_num, int nb_sectors);
 int coroutine_fn bdrv_co_writev(BlockDriverState *bs, int64_t sector_num,
     int nb_sectors, QEMUIOVector *qiov);
 /*
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 6078dd3..bd51abf 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -377,4 +377,20 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
                   BlockDriverCompletionFunc *cb,
                   void *opaque, Error **errp);
 
+/*
+ * backup_start:
+ * @bs: Block device to operate on.
+ * @target: Block device to write to.
+ * @speed: The maximum speed, in bytes per second, or 0 for unlimited.
+ * @cb: Completion function for the job.
+ * @opaque: Opaque pointer value passed to @cb.
+ *
+ * Start a backup operation on @bs.  Clusters in @bs are written to @target
+ * until the job is cancelled or manually completed.
+ */
+void backup_start(BlockDriverState *bs, BlockDriverState *target,
+                  int64_t speed,
+                  BlockDriverCompletionFunc *cb, void *opaque,
+                  Error **errp);
+
 #endif /* BLOCK_INT_H */
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index c290d07..6f42495 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -50,6 +50,13 @@ typedef struct BlockJobType {
      * manually.
      */
     void (*complete)(BlockJob *job, Error **errp);
+
+    /** tracked requests */
+    int coroutine_fn (*before_read)(BlockDriverState *bs, int64_t sector_num,
+                                    int nb_sectors, QEMUIOVector *qiov);
+    int coroutine_fn (*before_write)(BlockDriverState *bs, int64_t sector_num,
+                                     int nb_sectors, QEMUIOVector *qiov);
+
 } BlockJobType;
 
 /**
@@ -103,6 +110,9 @@ struct BlockJob {
     /** Speed that was set with @block_job_set_speed.  */
     int64_t speed;
 
+    /** tracked requests */
+    int cluster_size;
+
     /** The completion function that will be called when the job completes.  */
     BlockDriverCompletionFunc *cb;
 
-- 
1.8.1.4

[Qemu-devel] [PATCH 2/3] block: add block_backup QMP command

[Stefan Hajnoczi]

@block-backup

Start a point-in-time copy of a block device to a new destination.

@device:  the name of the device whose writes should be mirrored.

@target: the target of the new image. If the file exists, or if it
         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.

@format: #optional the format of the new destination, default is to
         probe if @mode is 'existing', else the format of the source

@mode: #optional whether and how QEMU should create a new image, default is
       'absolute-paths'.

@speed:  #optional the maximum speed, in bytes per second

Returns: nothing on success
         If @device is not a valid block device, DeviceNotFound

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 blockdev.c       | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qapi-schema.json | 28 +++++++++++++++++
 qmp-commands.hx  |  6 ++++
 3 files changed, 126 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index 8a1652b..521d999 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1279,6 +1279,98 @@ void qmp_block_commit(const char *device,
     drive_get_ref(drive_get_by_blockdev(bs));
 }
 
+void qmp_block_backup(const char *device, const char *target,
+                      bool has_format, const char *format,
+                      bool has_mode, enum NewImageMode mode,
+                      bool has_speed, int64_t speed,
+                      Error **errp)
+{
+    BlockDriverState *bs;
+    BlockDriverState *target_bs;
+    BlockDriver *proto_drv;
+    BlockDriver *drv = NULL;
+    Error *local_err = NULL;
+    int flags;
+    uint64_t size;
+    int ret;
+
+    if (!has_speed) {
+        speed = 0;
+    }
+    if (!has_mode) {
+        mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
+    }
+
+    bs = bdrv_find(device);
+    if (!bs) {
+        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+        return;
+    }
+
+    if (!bdrv_is_inserted(bs)) {
+        error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
+        return;
+    }
+
+    if (!has_format) {
+        format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name;
+    }
+    if (format) {
+        drv = bdrv_find_format(format);
+        if (!drv) {
+            error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
+            return;
+        }
+    }
+
+    if (bdrv_in_use(bs)) {
+        error_set(errp, QERR_DEVICE_IN_USE, device);
+        return;
+    }
+
+    flags = bs->open_flags | BDRV_O_RDWR;
+
+    proto_drv = bdrv_find_protocol(target);
+    if (!proto_drv) {
+        error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
+        return;
+    }
+
+    bdrv_get_geometry(bs, &size);
+    size *= 512;
+    if (mode != NEW_IMAGE_MODE_EXISTING) {
+        assert(format && drv);
+        bdrv_img_create(target, format,
+                        NULL, NULL, NULL, size, flags, &local_err, false);
+    }
+
+    if (error_is_set(&local_err)) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    target_bs = bdrv_new("");
+    ret = bdrv_open(target_bs, target, NULL, flags, drv);
+
+    if (ret < 0) {
+        bdrv_delete(target_bs);
+        error_set(errp, QERR_OPEN_FILE_FAILED, target);
+        return;
+    }
+
+    backup_start(bs, target_bs, speed, block_job_cb, bs, &local_err);
+    if (local_err != NULL) {
+        bdrv_delete(target_bs);
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    /* Grab a reference so hotplug does not delete the BlockDriverState from
+     * underneath us.
+     */
+    drive_get_ref(drive_get_by_blockdev(bs));
+}
+
 #define DEFAULT_MIRROR_BUF_SIZE   (10 << 20)
 
 void qmp_drive_mirror(const char *device, const char *target,
diff --git a/qapi-schema.json b/qapi-schema.json
index 751d3c2..903d2a5 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1715,6 +1715,34 @@
             '*speed': 'int' } }
 
 ##
+# @block-backup
+#
+# Start a point-in-time copy of a block device to a new destination.
+#
+# @device:  the name of the device whose writes should be mirrored.
+#
+# @target: the target of the new image. If the file exists, or if it
+#          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.
+#
+# @format: #optional the format of the new destination, default is to
+#          probe if @mode is 'existing', else the format of the source
+#
+# @mode: #optional whether and how QEMU should create a new image, default is
+#        'absolute-paths'.
+#
+# @speed:  #optional the maximum speed, in bytes per second
+#
+# Returns: nothing on success
+#          If @device is not a valid block device, DeviceNotFound
+#
+# Since 1.5
+##
+{ 'command': 'block-backup',
+  'data': { 'device': 'str', 'target': 'str', '*format': 'str',
+            '*mode': 'NewImageMode', '*speed': 'int' } }
+
+##
 # @drive-mirror
 #
 # Start mirroring a block device's writes to a new destination.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 4d65422..ce73e44 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -889,6 +889,12 @@ EQMP
     },
 
     {
+        .name       = "block-backup",
+        .args_type  = "device:B,target:s,speed:i?,mode:s?,format:s?",
+        .mhandler.cmd_new = qmp_marshal_input_block_backup,
+    },
+
+    {
         .name       = "block-job-set-speed",
         .args_type  = "device:B,speed:o",
         .mhandler.cmd_new = qmp_marshal_input_block_job_set_speed,
-- 
1.8.1.4

[Qemu-devel] [PATCH 3/3] qemu-iotests: add 054 block-backup test case

[Stefan Hajnoczi]

Testing block-backup is similar to image streaming and drive mirroring.
This test case is based on 041.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qemu-iotests/054     | 230 +++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/054.out |   5 +
 tests/qemu-iotests/group   |   1 +
 3 files changed, 236 insertions(+)
 create mode 100755 tests/qemu-iotests/054
 create mode 100644 tests/qemu-iotests/054.out

diff --git a/tests/qemu-iotests/054 b/tests/qemu-iotests/054
new file mode 100755
index 0000000..506bf05
--- /dev/null
+++ b/tests/qemu-iotests/054
@@ -0,0 +1,230 @@
+#!/usr/bin/env python
+#
+# Tests for block-backup
+#
+# Copyright (C) 2013 Red Hat, Inc.
+#
+# Based on 041.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+import time
+import os
+import iotests
+from iotests import qemu_img, qemu_io
+
+test_img = os.path.join(iotests.test_dir, 'test.img')
+target_img = os.path.join(iotests.test_dir, 'target.img')
+
+class BlockBackupTestCase(iotests.QMPTestCase):
+    '''Abstract base class for block-backup test cases'''
+
+    def assert_no_active_backups(self):
+        result = self.vm.qmp('query-block-jobs')
+        self.assert_qmp(result, 'return', [])
+
+    def cancel_and_wait(self, drive='drive0'):
+        '''Cancel a block job and wait for it to finish'''
+        result = self.vm.qmp('block-job-cancel', device=drive)
+        self.assert_qmp(result, 'return', {})
+
+        cancelled = False
+        while not cancelled:
+            for event in self.vm.get_qmp_events(wait=True):
+                if event['event'] == 'BLOCK_JOB_COMPLETED' or \
+                   event['event'] == 'BLOCK_JOB_CANCELLED':
+                    self.assert_qmp(event, 'data/type', 'backup')
+                    self.assert_qmp(event, 'data/device', drive)
+                    cancelled = True
+
+        self.assert_no_active_backups()
+
+    def complete_and_wait(self):
+        completed = False
+        while not completed:
+            for event in self.vm.get_qmp_events(wait=True):
+                if event['event'] == 'BLOCK_JOB_COMPLETED':
+                    self.assert_qmp(event, 'data/type', 'backup')
+                    self.assert_qmp(event, 'data/device', 'drive0')
+                    self.assert_qmp(event, 'data/offset', self.image_len)
+                    self.assert_qmp(event, 'data/len', self.image_len)
+                    completed = True
+        self.assert_no_active_backups()
+
+    def compare_images(self, img1, img2):
+        try:
+            qemu_img('convert', '-f', iotests.imgfmt, '-O', 'raw', img1, img1 + '.raw')
+            qemu_img('convert', '-f', iotests.imgfmt, '-O', 'raw', img2, img2 + '.raw')
+            file1 = open(img1 + '.raw', 'r')
+            file2 = open(img2 + '.raw', 'r')
+            return file1.read() == file2.read()
+        finally:
+            if file1 is not None:
+                file1.close()
+            if file2 is not None:
+                file2.close()
+            try:
+                os.remove(img1 + '.raw')
+            except OSError:
+                pass
+            try:
+                os.remove(img2 + '.raw')
+            except OSError:
+                pass
+
+class TestSingleDrive(BlockBackupTestCase):
+    image_len = 120 * 1024 * 1024 # MB
+
+    def setUp(self):
+        qemu_img('create', '-f', iotests.imgfmt, test_img, str(TestSingleDrive.image_len))
+        self.vm = iotests.VM().add_drive(test_img)
+        self.vm.launch()
+
+    def tearDown(self):
+        self.vm.shutdown()
+        os.remove(test_img)
+        try:
+            os.remove(target_img)
+        except OSError:
+            pass
+
+    def test_complete(self):
+        self.assert_no_active_backups()
+
+        result = self.vm.qmp('block-backup', device='drive0',
+                             target=target_img)
+        self.assert_qmp(result, 'return', {})
+
+        self.complete_and_wait()
+        self.vm.shutdown()
+        self.assertTrue(self.compare_images(test_img, target_img),
+                        'target image does not match source after backup')
+
+    def test_cancel(self):
+        self.assert_no_active_backups()
+
+        result = self.vm.qmp('block-backup', device='drive0',
+                             target=target_img)
+        self.assert_qmp(result, 'return', {})
+
+        self.cancel_and_wait()
+
+    def test_pause(self):
+        self.assert_no_active_backups()
+
+        result = self.vm.qmp('block-backup', device='drive0',
+                             target=target_img)
+        self.assert_qmp(result, 'return', {})
+
+        result = self.vm.qmp('block-job-pause', device='drive0')
+        self.assert_qmp(result, 'return', {})
+
+        time.sleep(1)
+        result = self.vm.qmp('query-block-jobs')
+        offset = self.dictpath(result, 'return[0]/offset')
+
+        time.sleep(1)
+        result = self.vm.qmp('query-block-jobs')
+        self.assert_qmp(result, 'return[0]/offset', offset)
+
+        result = self.vm.qmp('block-job-resume', device='drive0')
+        self.assert_qmp(result, 'return', {})
+
+        self.complete_and_wait()
+        self.vm.shutdown()
+        self.assertTrue(self.compare_images(test_img, target_img),
+                        'target image does not match source after backup')
+
+    def test_medium_not_found(self):
+        result = self.vm.qmp('block-backup', device='ide1-cd0',
+                             target=target_img)
+        self.assert_qmp(result, 'error/class', 'GenericError')
+
+    def test_image_not_found(self):
+        result = self.vm.qmp('block-backup', device='drive0',
+                             mode='existing', target=target_img)
+        self.assert_qmp(result, 'error/class', 'GenericError')
+
+    def test_device_not_found(self):
+        result = self.vm.qmp('block-backup', device='nonexistent',
+                             target=target_img)
+        self.assert_qmp(result, 'error/class', 'DeviceNotFound')
+
+class TestSetSpeed(BlockBackupTestCase):
+    image_len = 80 * 1024 * 1024 # MB
+
+    def setUp(self):
+        qemu_img('create', '-f', iotests.imgfmt, test_img, str(TestSetSpeed.image_len))
+        self.vm = iotests.VM().add_drive(test_img)
+        self.vm.launch()
+
+    def tearDown(self):
+        self.vm.shutdown()
+        os.remove(test_img)
+        os.remove(target_img)
+
+    def test_set_speed(self):
+        self.assert_no_active_backups()
+
+        result = self.vm.qmp('block-backup', device='drive0',
+                             target=target_img)
+        self.assert_qmp(result, 'return', {})
+
+        # Default speed is 0
+        result = self.vm.qmp('query-block-jobs')
+        self.assert_qmp(result, 'return[0]/device', 'drive0')
+        self.assert_qmp(result, 'return[0]/speed', 0)
+
+        result = self.vm.qmp('block-job-set-speed', device='drive0', speed=8 * 1024 * 1024)
+        self.assert_qmp(result, 'return', {})
+
+        # Ensure the speed we set was accepted
+        result = self.vm.qmp('query-block-jobs')
+        self.assert_qmp(result, 'return[0]/device', 'drive0')
+        self.assert_qmp(result, 'return[0]/speed', 8 * 1024 * 1024)
+
+        self.cancel_and_wait()
+
+        # Check setting speed in block-backup works
+        result = self.vm.qmp('block-backup', device='drive0',
+                             target=target_img, speed=4*1024*1024)
+        self.assert_qmp(result, 'return', {})
+
+        result = self.vm.qmp('query-block-jobs')
+        self.assert_qmp(result, 'return[0]/device', 'drive0')
+        self.assert_qmp(result, 'return[0]/speed', 4 * 1024 * 1024)
+
+        self.cancel_and_wait()
+
+    def test_set_speed_invalid(self):
+        self.assert_no_active_backups()
+
+        result = self.vm.qmp('block-backup', device='drive0',
+                             target=target_img, speed=-1)
+        self.assert_qmp(result, 'error/class', 'GenericError')
+
+        self.assert_no_active_backups()
+
+        result = self.vm.qmp('block-backup', device='drive0',
+                             target=target_img)
+        self.assert_qmp(result, 'return', {})
+
+        result = self.vm.qmp('block-job-set-speed', device='drive0', speed=-1)
+        self.assert_qmp(result, 'error/class', 'GenericError')
+
+        self.cancel_and_wait()
+
+if __name__ == '__main__':
+    iotests.main(supported_fmts=['raw', 'qcow2'])
diff --git a/tests/qemu-iotests/054.out b/tests/qemu-iotests/054.out
new file mode 100644
index 0000000..594c16f
--- /dev/null
+++ b/tests/qemu-iotests/054.out
@@ -0,0 +1,5 @@
+........
+----------------------------------------------------------------------
+Ran 8 tests
+
+OK
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 68eabda..f487a8f 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -60,3 +60,4 @@
 051 rw auto
 052 rw auto backing
 053 rw auto
+054 rw auto
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH 0/3] block: block-backup live backup command

[Eric Blake]

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

On 04/23/2013 10:25 AM, Stefan Hajnoczi wrote:
> This series adds a new QMP command, block-backup, which takes a point-in-time
> snapshot of a block device.  The snapshot is copied out to a target block
> device.  A simple example is:
> 
>   block-backup device=virtio0 format=qcow2 target=backup-20130401.qcow2

> What's next for block-backup?
> -----------------------------
> The following enhancements are left for future patches:
> 
> 1. QMP 'transaction' support.  It is handy to atomically snapshot multiple
>    block devices.  We need qmp_transaction() support for this.  Wenchao Xia is
>    currently making qmp_transaction() extensible so new action types, like
>    block-backup, can be added.
> 
> 2. Sync modes like drive-mirror (top, full, none).  This makes it possible to
>    preserve the backing file chain.

Based on today's phone call, it sounds like this would mean adding
optional parameters to the QMP command.  We already did that for
drive-mirror (1.4 has more parameters than 1.3), but without a way to
introspect when those parameters are available, the new parameters
aren't quite as useful.  So we don't repeat that mistake, we need to
decide whether this should still go into 1.5 with a plan of adding
parameters for 1.6, or whether we should add a counterpart query-*
command that makes it easy to determine how much of block-backup is
supported, or [your suggestion here]....

-- 
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 0/3] block: block-backup live backup command

[Paolo Bonzini]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Il 23/04/2013 18:49, Eric Blake ha scritto:
> Based on today's phone call, it sounds like this would mean adding 
> optional parameters to the QMP command.  We already did that for 
> drive-mirror (1.4 has more parameters than 1.3), but without a way
> to introspect when those parameters are available, the new
> parameters aren't quite as useful.  So we don't repeat that
> mistake, we need to decide whether this should still go into 1.5
> with a plan of adding parameters for 1.6, or whether we should add
> a counterpart query-* command that makes it easy to determine how
> much of block-backup is supported, or [your suggestion here]....

... libvirt can omit the optional parameters if they match the default
and just try executing the command.  If it gets an error, it just
reports it.

That is, instead of

     dictionary = json_object_new()
     if (has_capability)
         add argument to JSON dictionary
     else if (argument != default)
         return error

     send JSON dictionary via QMP

do this

     dictionary = json_object_new()
     if (argument != default)
         add argument to JSON dictionary
     send JSON dictionary via QMP

Paolo
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJRdr2AAAoJEBvWZb6bTYbydkAP/0EegweCJX37/XIJ9vtYy4c1
RdS1Jkg/t+KNfsmUp/Qs0JpYbbxhLbbsM9pboKc1LU2Mv5HZd2koibIYNOwTTN3q
y2A/4Ecr8A+Q9pv8mblTb8HFea3FDNwtBugTQG7LM/CywYdAk6tg4qZ/0NEXVAPA
XEmR0RXRP0ln52f/fByFuq2czg4VvXqygKeBtRJ20ROeUCNpNxYDjiBX03I/4OE8
B1iTkMy7fJwKoc34tOPZ04IzNIRI/Zxtl2my5j3wx7iheYUXNAYPrKpSKXwqYj3g
x+jRe7A/QxUZpBTmIynBmDvOxut8ZaLqeWAHYWlcNZameWGSZJgsav5hMjaOVlS3
Kk0Tkm4rrgSYuzC+vhttvGaj9iacnRaG6IMmwyfq1TFcETXLH6ILt8sCxoO5dpHp
XetKnEF4m1zmQDXceMbFLRZ3eOKMjwObf6QQIjLkrs/0Xc+T+GfsqUdN6ylY17Rk
s6Jsx5ICbS4IUJlktkVv/U9n4VD7y9IBwRlglKgj86noHPFFkb/Z+C0cW8Zkwej/
ZsrjXTNB1abI6/X11PvoaqEnFauaradbtMfRbEmDMhBi+CoBZ9ps578OSn9+x1rn
j+WYup9Z5qnDO+BvG+tDElaY6qQUR4ZaGOKo05ODjXkPd83Jn/pOjD2ioaa1dybY
KP+0EDFS6SwiSX5jsoei
=C9uD
-----END PGP SIGNATURE-----

Re: [Qemu-devel] [PATCH 0/3] block: block-backup live backup command

[Stefan Hajnoczi]

On Tue, Apr 23, 2013 at 10:49:35AM -0600, Eric Blake wrote:
> On 04/23/2013 10:25 AM, Stefan Hajnoczi wrote:
> > This series adds a new QMP command, block-backup, which takes a point-in-time
> > snapshot of a block device.  The snapshot is copied out to a target block
> > device.  A simple example is:
> > 
> >   block-backup device=virtio0 format=qcow2 target=backup-20130401.qcow2
> 
> > What's next for block-backup?
> > -----------------------------
> > The following enhancements are left for future patches:
> > 
> > 1. QMP 'transaction' support.  It is handy to atomically snapshot multiple
> >    block devices.  We need qmp_transaction() support for this.  Wenchao Xia is
> >    currently making qmp_transaction() extensible so new action types, like
> >    block-backup, can be added.
> > 
> > 2. Sync modes like drive-mirror (top, full, none).  This makes it possible to
> >    preserve the backing file chain.
> 
> Based on today's phone call, it sounds like this would mean adding
> optional parameters to the QMP command.  We already did that for
> drive-mirror (1.4 has more parameters than 1.3), but without a way to
> introspect when those parameters are available, the new parameters
> aren't quite as useful.  So we don't repeat that mistake, we need to
> decide whether this should still go into 1.5 with a plan of adding
> parameters for 1.6, or whether we should add a counterpart query-*
> command that makes it easy to determine how much of block-backup is
> supported, or [your suggestion here]....

I have ideas for the QMP optional parameters discussion that I'll share
in another thread.

It's not a problem in this case because block-backup and the sync mode
optional parameter will be added in the 1.6 release cycle.

In other words, since QMP is versioned by QEMU release number it's fine
to have multiple commits that build up a QMP command - as long as they
fall within the same release.

Stefan

Re: [Qemu-devel] [PATCH 0/3] block: block-backup live backup command

[Eric Blake]

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

On 04/24/2013 01:41 AM, Stefan Hajnoczi wrote:

>>>
>>> 2. Sync modes like drive-mirror (top, full, none).  This makes it possible to
>>>    preserve the backing file chain.
>>
>> Based on today's phone call, it sounds like this would mean adding
>> optional parameters to the QMP command.

> 
> It's not a problem in this case because block-backup and the sync mode
> optional parameter will be added in the 1.6 release cycle.

Ah, I misunderstood - I thought you were shooting for 1.5 for the
initial release.  But it definitely makes more sense if the entire
command is waiting for 1.6, since we've already entered soft freeze for 1.5.

-- 
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 2/3] block: add block_backup QMP command

[Eric Blake]

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

On 04/23/2013 10:25 AM, Stefan Hajnoczi wrote:
> @block-backup
> 
> Start a point-in-time copy of a block device to a new destination.
> 
> @device:  the name of the device whose writes should be mirrored.


> +++ b/qapi-schema.json
> @@ -1715,6 +1715,34 @@
>              '*speed': 'int' } }
>  
>  ##
> +# @block-backup
> +#
> +# Start a point-in-time copy of a block device to a new destination.
> +#
> +# @device:  the name of the device whose writes should be mirrored.

two spaces?

> +#
> +# @target: the target of the new image. If the file exists, or if it
> +#          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.
> +#
> +# @format: #optional the format of the new destination, default is to
> +#          probe if @mode is 'existing', else the format of the source
> +#
> +# @mode: #optional whether and how QEMU should create a new image, default is
> +#        'absolute-paths'.
> +#
> +# @speed:  #optional the maximum speed, in bytes per second
> +#
> +# Returns: nothing on success
> +#          If @device is not a valid block device, DeviceNotFound
> +#
> +# Since 1.5

1.6, now

> +##
> +{ 'command': 'block-backup',
> +  'data': { 'device': 'str', 'target': 'str', '*format': 'str',
> +            '*mode': 'NewImageMode', '*speed': 'int' } }

Looks reasonable (since it is closely aligned to drive-mirror).

-- 
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 2/3] block: add block_backup QMP command

[Eric Blake]

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

On 04/26/2013 04:52 PM, Eric Blake wrote:
> On 04/23/2013 10:25 AM, Stefan Hajnoczi wrote:
>> @block-backup

[hit send too soon]

Subject line should mention block-backup, not block_backup.

>> +##
>> +{ 'command': 'block-backup',
>> +  'data': { 'device': 'str', 'target': 'str', '*format': 'str',
>> +            '*mode': 'NewImageMode', '*speed': 'int' } }
> 
> Looks reasonable (since it is closely aligned to drive-mirror).
> 

-- 
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 2/3] block: add block_backup QMP command

[Eric Blake]

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

On 04/23/2013 10:25 AM, Stefan Hajnoczi wrote:
> @block-backup
> 
> Start a point-in-time copy of a block device to a new destination.
> 
> @device:  the name of the device whose writes should be mirrored.
> 
> @target: the target of the new image. If the file exists, or if it
>          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.
> 
> @format: #optional the format of the new destination, default is to
>          probe if @mode is 'existing', else the format of the source
> 
> @mode: #optional whether and how QEMU should create a new image, default is
>        'absolute-paths'.
> 
> @speed:  #optional the maximum speed, in bytes per second
> 
> Returns: nothing on success
>          If @device is not a valid block device, DeviceNotFound

This starts a new block job type; I assume the existing block-job-cancel
and query-block-jobs can track it.

I'd really love to see us change 'BlockJobInfo' to use an enum for
'type', instead of its open-coded 'str'.  Likewise, the block-job
related events in QMP/qmp-events.txt should be updated to refer to the
enum instead of also being open-coded 'str'.  Will this job be called
"backup"?

-- 
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 0/3] block: block-backup live backup command

[Wenchao Xia]

于 2013-4-24 0:25, Stefan Hajnoczi 写道:
> This series adds a new QMP command, block-backup, which takes a point-in-time
> snapshot of a block device.  The snapshot is copied out to a target block
> device.  A simple example is:
> 
>    block-backup device=virtio0 format=qcow2 target=backup-20130401.qcow2
> 
> The original block-backup blockjob was written by Dietmar Maurer
> <dietmar@proxmox.com>.  He is currently busy but I feel the feature is worth
> pushing into QEMU since there has been interest.  This is my version of his
> patch, plus the QMP command and qemu-iotests test case.
> 
> How is this different from block-stream and drive-mirror?
> ---------------------------------------------------------
> Both block-stream and drive-mirror do not provide immediate point-in-time
> snapshots.  Instead they copy data into a new file and then switch to it.  In
> other words, the point at which the "snapshot" is taken cannot be controlled
> directly.
> 
> block-backup intercepts guest writes and saves data into the target block
> device before it is overwritten.  The target block device can be a raw image
> file, backing files are not used to implement this feature.
> 
  Besides, compared to snapshot-blkdev, it mainly brings better
performance by avoid merging later, however, other tool may
be needed to form an incremental backup, which may be not related to
this patch.
  No objection to this patch, but perhaps a better way is using
internal snapshot by adding base/delta data export support.


> How can block-backup be used?
> -----------------------------
> The simplest use-case is to copy a point-in-time snapshot to a local file.
> 
> More advanced users may wish to make the target an NBD URL.  The NBD server
> listening on the other side can process the backup writes any way it wishes.  I
> previously posted an RFC series with a backup server that streamed Dietmar's
> VMA backup archive format.
> 
> What's next for block-backup?
> -----------------------------
> The following enhancements are left for future patches:
> 
> 1. QMP 'transaction' support.  It is handy to atomically snapshot multiple
>     block devices.  We need qmp_transaction() support for this.  Wenchao Xia is
>     currently making qmp_transaction() extensible so new action types, like
>     block-backup, can be added.
> 
> 2. Sync modes like drive-mirror (top, full, none).  This makes it possible to
>     preserve the backing file chain.
> 
> Dietmar Maurer (1):
>    block: add basic backup support to block driver
> 
> Stefan Hajnoczi (2):
>    block: add block_backup QMP command
>    qemu-iotests: add 054 block-backup test case
> 
>   block.c                    |  69 ++++++++++++-
>   block/Makefile.objs        |   1 +
>   block/backup.c             | 252 +++++++++++++++++++++++++++++++++++++++++++++
>   blockdev.c                 |  92 +++++++++++++++++
>   include/block/block.h      |   2 +
>   include/block/block_int.h  |  16 +++
>   include/block/blockjob.h   |  10 ++
>   qapi-schema.json           |  28 +++++
>   qmp-commands.hx            |   6 ++
>   tests/qemu-iotests/054     | 230 +++++++++++++++++++++++++++++++++++++++++
>   tests/qemu-iotests/054.out |   5 +
>   tests/qemu-iotests/group   |   1 +
>   12 files changed, 707 insertions(+), 5 deletions(-)
>   create mode 100644 block/backup.c
>   create mode 100755 tests/qemu-iotests/054
>   create mode 100644 tests/qemu-iotests/054.out
> 


-- 
Best Regards

Wenchao Xia

Re: [Qemu-devel] [PATCH 2/3] block: add block_backup QMP command

[Stefan Hajnoczi]

On Fri, Apr 26, 2013 at 04:58:24PM -0600, Eric Blake wrote:
> On 04/23/2013 10:25 AM, Stefan Hajnoczi wrote:
> > @block-backup
> > 
> > Start a point-in-time copy of a block device to a new destination.
> > 
> > @device:  the name of the device whose writes should be mirrored.
> > 
> > @target: the target of the new image. If the file exists, or if it
> >          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.
> > 
> > @format: #optional the format of the new destination, default is to
> >          probe if @mode is 'existing', else the format of the source
> > 
> > @mode: #optional whether and how QEMU should create a new image, default is
> >        'absolute-paths'.
> > 
> > @speed:  #optional the maximum speed, in bytes per second
> > 
> > Returns: nothing on success
> >          If @device is not a valid block device, DeviceNotFound
> 
> This starts a new block job type; I assume the existing block-job-cancel
> and query-block-jobs can track it.

Right.  I will update the documentation to be explicit about the block
job that this command creates.

> I'd really love to see us change 'BlockJobInfo' to use an enum for
> 'type', instead of its open-coded 'str'.  Likewise, the block-job
> related events in QMP/qmp-events.txt should be updated to refer to the
> enum instead of also being open-coded 'str'.

Since the block job QMP API has been in released I'm not sure changing
this is worthwhile.  QEMU and libvirt would have to maintain
compatibility so the code will just be duplicated.

> Will this job be called "backup"?

Yes, it is called "backup".

Re: [Qemu-devel] [PATCH 0/3] block: block-backup live backup command

[Stefan Hajnoczi]

On Sat, Apr 27, 2013 at 01:37:07PM +0800, Wenchao Xia wrote:
> 于 2013-4-24 0:25, Stefan Hajnoczi 写道:
> > This series adds a new QMP command, block-backup, which takes a point-in-time
> > snapshot of a block device.  The snapshot is copied out to a target block
> > device.  A simple example is:
> > 
> >    block-backup device=virtio0 format=qcow2 target=backup-20130401.qcow2
> > 
> > The original block-backup blockjob was written by Dietmar Maurer
> > <dietmar@proxmox.com>.  He is currently busy but I feel the feature is worth
> > pushing into QEMU since there has been interest.  This is my version of his
> > patch, plus the QMP command and qemu-iotests test case.
> > 
> > How is this different from block-stream and drive-mirror?
> > ---------------------------------------------------------
> > Both block-stream and drive-mirror do not provide immediate point-in-time
> > snapshots.  Instead they copy data into a new file and then switch to it.  In
> > other words, the point at which the "snapshot" is taken cannot be controlled
> > directly.
> > 
> > block-backup intercepts guest writes and saves data into the target block
> > device before it is overwritten.  The target block device can be a raw image
> > file, backing files are not used to implement this feature.
> > 
>   Besides, compared to snapshot-blkdev, it mainly brings better
> performance by avoid merging later, however, other tool may
> be needed to form an incremental backup, which may be not related to
> this patch.
>   No objection to this patch, but perhaps a better way is using
> internal snapshot by adding base/delta data export support.

Yes, incremental backups would need to be added later.

Stefan

Re: [Qemu-devel] [PATCH 2/3] block: add block_backup QMP command

[Paolo Bonzini]

Il 29/04/2013 09:21, Stefan Hajnoczi ha scritto:
> > I'd really love to see us change 'BlockJobInfo' to use an enum for
> > 'type', instead of its open-coded 'str'.  Likewise, the block-job
> > related events in QMP/qmp-events.txt should be updated to refer to the
> > enum instead of also being open-coded 'str'.
> 
> Since the block job QMP API has been in released I'm not sure changing
> this is worthwhile.  QEMU and libvirt would have to maintain
> compatibility so the code will just be duplicated.

I don't think this would change the actual data on the wire.  However,
it would let libvirt know the supported block job types by introspecting
the enum.

Paolo

Re: [Qemu-devel] [PATCH 2/3] block: add block_backup QMP command

[Eric Blake]

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

On 04/29/2013 03:27 AM, Paolo Bonzini wrote:
> Il 29/04/2013 09:21, Stefan Hajnoczi ha scritto:
>>> I'd really love to see us change 'BlockJobInfo' to use an enum for
>>> 'type', instead of its open-coded 'str'.  Likewise, the block-job
>>> related events in QMP/qmp-events.txt should be updated to refer to the
>>> enum instead of also being open-coded 'str'.
>>
>> Since the block job QMP API has been in released I'm not sure changing
>> this is worthwhile.  QEMU and libvirt would have to maintain
>> compatibility so the code will just be duplicated.
> 
> I don't think this would change the actual data on the wire.  However,
> it would let libvirt know the supported block job types by introspecting
> the enum.

Until we have introspection, the point is moot.  When we have
introspection, libvirt would much rather see an enum than a 'str'.  I
see absolutely no back-compat problem in changing the code to be
type-safe prior to the point that introspection is added.

-- 
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 2/3] block: add block_backup QMP command

[Stefan Hajnoczi]

On Mon, Apr 29, 2013 at 09:51:47AM -0600, Eric Blake wrote:
> On 04/29/2013 03:27 AM, Paolo Bonzini wrote:
> > Il 29/04/2013 09:21, Stefan Hajnoczi ha scritto:
> >>> I'd really love to see us change 'BlockJobInfo' to use an enum for
> >>> 'type', instead of its open-coded 'str'.  Likewise, the block-job
> >>> related events in QMP/qmp-events.txt should be updated to refer to the
> >>> enum instead of also being open-coded 'str'.
> >>
> >> Since the block job QMP API has been in released I'm not sure changing
> >> this is worthwhile.  QEMU and libvirt would have to maintain
> >> compatibility so the code will just be duplicated.
> > 
> > I don't think this would change the actual data on the wire.  However,
> > it would let libvirt know the supported block job types by introspecting
> > the enum.
> 
> Until we have introspection, the point is moot.  When we have
> introspection, libvirt would much rather see an enum than a 'str'.  I
> see absolutely no back-compat problem in changing the code to be
> type-safe prior to the point that introspection is added.

Okay, I haven't looked at how QAPI enums are implemented.  If we can
move from custom strings to enum without breaking existing libvirt, then
great!

Stefan