* [Qemu-devel] [PATCH V9 1/2] Implement sync modes for drive-backup.
2013-07-26 18:39 [Qemu-devel] [PATCH V9 0/2] Implement sync modes for drive-backup Ian Main
@ 2013-07-26 18:39 ` Ian Main
2013-07-26 18:39 ` [Qemu-devel] [PATCH V9 2/2] Add tests for sync modes 'TOP' and 'NONE' Ian Main
2013-07-29 8:52 ` [Qemu-devel] [PATCH V9 0/2] Implement sync modes for drive-backup Kevin Wolf
2 siblings, 0 replies; 10+ messages in thread
From: Ian Main @ 2013-07-26 18:39 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, famz, rjones, Ian Main, stefanha, pbonzini
This patch adds sync-modes to the drive-backup interface and
implements the FULL, NONE and TOP modes of synchronization.
FULL performs as before copying the entire contents of the drive
while preserving the point-in-time using CoW.
NONE only copies new writes to the target drive.
TOP copies changes to the topmost drive image and preserves the
point-in-time using CoW.
For sync mode TOP are creating a new target image using the same backing
file as the original disk image. Then any new data that has been laid
on top of it since creation is copied in the main backup_run() loop.
There is an extra check in the 'TOP' case so that we don't bother to copy
all the data of the backing file as it already exists in the target.
This is where the bdrv_co_is_allocated() is used to determine if the
data exists in the topmost layer or below.
Also any new data being written is intercepted via the write_notifier
hook which ends up calling backup_do_cow() to copy old data out before
it gets overwritten.
For mode 'NONE' we create the new target image and only copy in the
original data from the disk image starting from the time the call was
made. This preserves the point in time data by only copying the parts
that are *going to change* to the target image. This way we can
reconstruct the final image by checking to see if the given block exists
in the new target image first, and if it does not, you can get it from
the original image. This is basically an optimization allowing you to
do point-in-time snapshots with low overhead vs the 'FULL' version.
Since there is no old data to copy out the loop in backup_run() for the
NONE case just calls qemu_coroutine_yield() which only wakes up after
an event (usually cancel in this case). The rest is handled by the
before_write notifier which again calls backup_do_cow() to write out
the old data so it can be preserved.
Signed-off-by: Ian Main <imain@redhat.com>
---
block/backup.c | 105 +++++++++++++++++++++++++++++++++-------------
blockdev.c | 29 +++++++++----
include/block/block_int.h | 4 +-
qmp-commands.hx | 1 +
4 files changed, 101 insertions(+), 38 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index 16105d4..6ae8a05 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -37,6 +37,7 @@ typedef struct CowRequest {
typedef struct BackupBlockJob {
BlockJob common;
BlockDriverState *target;
+ MirrorSyncMode sync_mode;
RateLimit limit;
BlockdevOnError on_source_error;
BlockdevOnError on_target_error;
@@ -247,40 +248,83 @@ static void coroutine_fn backup_run(void *opaque)
bdrv_add_before_write_notifier(bs, &before_write);
- for (; start < end; start++) {
- bool error_is_read;
-
- if (block_job_is_cancelled(&job->common)) {
- break;
+ if (job->sync_mode == MIRROR_SYNC_MODE_NONE) {
+ while (!block_job_is_cancelled(&job->common)) {
+ /* Yield until the job is cancelled. We just let our before_write
+ * notify callback service CoW requests. */
+ job->common.busy = false;
+ qemu_coroutine_yield();
+ job->common.busy = true;
}
+ } else {
+ /* Both FULL and TOP SYNC_MODE's require copying.. */
+ for (; start < end; start++) {
+ bool error_is_read;
- /* 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)) {
+ break;
+ }
- if (block_job_is_cancelled(&job->common)) {
- 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);
+ }
- ret = backup_do_cow(bs, start * BACKUP_SECTORS_PER_CLUSTER,
- BACKUP_SECTORS_PER_CLUSTER, &error_is_read);
- if (ret < 0) {
- /* Depending on error action, fail now or retry cluster */
- BlockErrorAction action =
- backup_error_action(job, error_is_read, -ret);
- if (action == BDRV_ACTION_REPORT) {
+ if (block_job_is_cancelled(&job->common)) {
break;
- } else {
- start--;
- continue;
+ }
+
+ if (job->sync_mode == MIRROR_SYNC_MODE_TOP) {
+ int i, n;
+ int alloced = 0;
+
+ /* Check to see if these blocks are already in the
+ * backing file. */
+
+ for (i = 0; i < BACKUP_SECTORS_PER_CLUSTER;) {
+ /* bdrv_co_is_allocated() only returns true/false based
+ * on the first set of sectors it comes accross that
+ * are are all in the same state.
+ * For that reason we must verify each sector in the
+ * backup cluster length. We end up copying more than
+ * needed but at some point that is always the case. */
+ alloced =
+ bdrv_co_is_allocated(bs,
+ start * BACKUP_SECTORS_PER_CLUSTER + i,
+ BACKUP_SECTORS_PER_CLUSTER - i, &n);
+ i += n;
+
+ if (alloced == 1) {
+ break;
+ }
+ }
+
+ /* If the above loop never found any sectors that are in
+ * the topmost image, skip this backup. */
+ if (alloced == 0) {
+ continue;
+ }
+ }
+ /* FULL sync mode we copy the whole drive. */
+ ret = backup_do_cow(bs, start * BACKUP_SECTORS_PER_CLUSTER,
+ BACKUP_SECTORS_PER_CLUSTER, &error_is_read);
+ if (ret < 0) {
+ /* Depending on error action, fail now or retry cluster */
+ BlockErrorAction action =
+ backup_error_action(job, error_is_read, -ret);
+ if (action == BDRV_ACTION_REPORT) {
+ break;
+ } else {
+ start--;
+ continue;
+ }
}
}
}
@@ -300,7 +344,7 @@ static void coroutine_fn backup_run(void *opaque)
}
void backup_start(BlockDriverState *bs, BlockDriverState *target,
- int64_t speed,
+ int64_t speed, MirrorSyncMode sync_mode,
BlockdevOnError on_source_error,
BlockdevOnError on_target_error,
BlockDriverCompletionFunc *cb, void *opaque,
@@ -335,6 +379,7 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target,
job->on_source_error = on_source_error;
job->on_target_error = on_target_error;
job->target = target;
+ job->sync_mode = sync_mode;
job->common.len = len;
job->common.co = qemu_coroutine_create(backup_run);
qemu_coroutine_enter(job->common.co, job);
diff --git a/blockdev.c b/blockdev.c
index c5abd65..1d49a19 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1431,16 +1431,13 @@ void qmp_drive_backup(const char *device, const char *target,
{
BlockDriverState *bs;
BlockDriverState *target_bs;
+ BlockDriverState *source = NULL;
BlockDriver *drv = NULL;
Error *local_err = NULL;
int flags;
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;
}
@@ -1483,6 +1480,18 @@ void qmp_drive_backup(const char *device, const char *target,
flags = bs->open_flags | BDRV_O_RDWR;
+ /* See if we have a backing HD we can use to create our new image
+ * on top of. */
+ if (sync == MIRROR_SYNC_MODE_TOP) {
+ source = bs->backing_hd;
+ if (!source) {
+ sync = MIRROR_SYNC_MODE_FULL;
+ }
+ }
+ if (sync == MIRROR_SYNC_MODE_NONE) {
+ source = bs;
+ }
+
size = bdrv_getlength(bs);
if (size < 0) {
error_setg_errno(errp, -size, "bdrv_getlength failed");
@@ -1491,8 +1500,14 @@ void qmp_drive_backup(const char *device, const char *target,
if (mode != NEW_IMAGE_MODE_EXISTING) {
assert(format && drv);
- bdrv_img_create(target, format,
- NULL, NULL, NULL, size, flags, &local_err, false);
+ if (source) {
+ bdrv_img_create(target, format, source->filename,
+ source->drv->format_name, NULL,
+ size, flags, &local_err, false);
+ } else {
+ bdrv_img_create(target, format, NULL, NULL, NULL,
+ size, flags, &local_err, false);
+ }
}
if (error_is_set(&local_err)) {
@@ -1508,7 +1523,7 @@ void qmp_drive_backup(const char *device, const char *target,
return;
}
- backup_start(bs, target_bs, speed, on_source_error, on_target_error,
+ backup_start(bs, target_bs, speed, sync, on_source_error, on_target_error,
block_job_cb, bs, &local_err);
if (local_err != NULL) {
bdrv_delete(target_bs);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index c6ac871..e45f2a0 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -404,6 +404,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
* @bs: Block device to operate on.
* @target: Block device to write to.
* @speed: The maximum speed, in bytes per second, or 0 for unlimited.
+ * @sync_mode: What parts of the disk image should be copied to the destination.
* @on_source_error: The action to take upon error reading from the source.
* @on_target_error: The action to take upon error writing to the target.
* @cb: Completion function for the job.
@@ -413,7 +414,8 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
* until the job is cancelled or manually completed.
*/
void backup_start(BlockDriverState *bs, BlockDriverState *target,
- int64_t speed, BlockdevOnError on_source_error,
+ int64_t speed, MirrorSyncMode sync_mode,
+ BlockdevOnError on_source_error,
BlockdevOnError on_target_error,
BlockDriverCompletionFunc *cb, void *opaque,
Error **errp);
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 65a9e26..2e59b0d 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -960,6 +960,7 @@ Arguments:
Example:
-> { "execute": "drive-backup", "arguments": { "device": "drive0",
+ "sync": "full",
"target": "backup.img" } }
<- { "return": {} }
EQMP
--
1.8.1.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* [Qemu-devel] [PATCH V9 2/2] Add tests for sync modes 'TOP' and 'NONE'
2013-07-26 18:39 [Qemu-devel] [PATCH V9 0/2] Implement sync modes for drive-backup Ian Main
2013-07-26 18:39 ` [Qemu-devel] [PATCH V9 1/2] " Ian Main
@ 2013-07-26 18:39 ` Ian Main
2013-07-29 8:52 ` [Qemu-devel] [PATCH V9 0/2] Implement sync modes for drive-backup Kevin Wolf
2 siblings, 0 replies; 10+ messages in thread
From: Ian Main @ 2013-07-26 18:39 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, famz, rjones, Ian Main, stefanha, pbonzini
This patch adds tests for sync modes top and none. Test for 'TOP'
is separated out as it requires a backing file. Also added a test
for invalid format.
Signed-off-by: Ian Main <imain@redhat.com>
---
tests/qemu-iotests/055 | 6 +++
tests/qemu-iotests/055.out | 4 +-
tests/qemu-iotests/056 | 94 +++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/056.out | 5 +++
tests/qemu-iotests/group | 1 +
tests/qemu-iotests/iotests.py | 5 +++
6 files changed, 113 insertions(+), 2 deletions(-)
create mode 100755 tests/qemu-iotests/056
create mode 100644 tests/qemu-iotests/056.out
diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055
index c66f8db..44bb025 100755
--- a/tests/qemu-iotests/055
+++ b/tests/qemu-iotests/055
@@ -97,6 +97,12 @@ class TestSingleDrive(iotests.QMPTestCase):
target=target_img, sync='full', mode='existing')
self.assert_qmp(result, 'error/class', 'GenericError')
+ def test_invalid_format(self):
+ result = self.vm.qmp('drive-backup', device='drive0',
+ target=target_img, sync='full',
+ format='spaghetti-noodles')
+ self.assert_qmp(result, 'error/class', 'GenericError')
+
def test_device_not_found(self):
result = self.vm.qmp('drive-backup', device='nonexistent',
target=target_img, sync='full')
diff --git a/tests/qemu-iotests/055.out b/tests/qemu-iotests/055.out
index fa16b5c..6323079 100644
--- a/tests/qemu-iotests/055.out
+++ b/tests/qemu-iotests/055.out
@@ -1,5 +1,5 @@
-.............
+..............
----------------------------------------------------------------------
-Ran 13 tests
+Ran 14 tests
OK
diff --git a/tests/qemu-iotests/056 b/tests/qemu-iotests/056
new file mode 100755
index 0000000..6389342
--- /dev/null
+++ b/tests/qemu-iotests/056
@@ -0,0 +1,94 @@
+#!/usr/bin/env python
+#
+# Tests for drive-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, create_image
+
+backing_img = os.path.join(iotests.test_dir, 'backing.img')
+test_img = os.path.join(iotests.test_dir, 'test.img')
+target_img = os.path.join(iotests.test_dir, 'target.img')
+
+class TestSyncModesNoneAndTop(iotests.QMPTestCase):
+ image_len = 64 * 1024 * 1024 # MB
+
+ def setUp(self):
+ create_image(backing_img, TestSyncModesNoneAndTop.image_len)
+ qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, test_img)
+ qemu_io('-c', 'write -P0x41 0 512', test_img)
+ qemu_io('-c', 'write -P0xd5 1M 32k', test_img)
+ qemu_io('-c', 'write -P0xdc 32M 124k', test_img)
+ qemu_io('-c', 'write -P0xdc 67043328 64k', test_img)
+ self.vm = iotests.VM().add_drive(test_img)
+ self.vm.launch()
+
+ def tearDown(self):
+ self.vm.shutdown()
+ os.remove(test_img)
+ os.remove(backing_img)
+ try:
+ os.remove(target_img)
+ except OSError:
+ pass
+
+ def test_complete_top(self):
+ self.assert_no_active_block_jobs()
+ result = self.vm.qmp('drive-backup', device='drive0', sync='top',
+ format=iotests.imgfmt, target=target_img)
+ self.assert_qmp(result, 'return', {})
+
+ # Custom completed check as we are not copying all data.
+ 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/device', 'drive0')
+ self.assert_qmp_absent(event, 'data/error')
+ completed = True
+
+ self.assert_no_active_block_jobs()
+ self.vm.shutdown()
+ self.assertTrue(iotests.compare_images(test_img, target_img),
+ 'target image does not match source after backup')
+
+ def test_cancel_sync_none(self):
+ self.assert_no_active_block_jobs()
+
+ result = self.vm.qmp('drive-backup', device='drive0',
+ sync='none', target=target_img)
+ self.assert_qmp(result, 'return', {})
+ time.sleep(1)
+ self.vm.hmp_qemu_io('drive0', 'write -P0x5e 0 512')
+ self.vm.hmp_qemu_io('drive0', 'aio_flush')
+ # Verify that the original contents exist in the target image.
+
+ event = self.cancel_and_wait()
+ self.assert_qmp(event, 'data/type', 'backup')
+
+ self.vm.shutdown()
+ time.sleep(1)
+ self.assertEqual(-1, qemu_io('-c', 'read -P0x41 0 512', target_img).find("verification failed"))
+
+
+if __name__ == '__main__':
+ iotests.main(supported_fmts=['qcow2', 'qed'])
diff --git a/tests/qemu-iotests/056.out b/tests/qemu-iotests/056.out
new file mode 100644
index 0000000..fbc63e6
--- /dev/null
+++ b/tests/qemu-iotests/056.out
@@ -0,0 +1,5 @@
+..
+----------------------------------------------------------------------
+Ran 2 tests
+
+OK
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index fdc6ed1..b1d03c7 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -62,3 +62,4 @@
053 rw auto
054 rw auto
055 rw auto
+056 rw auto backing
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index b028a89..33ad0ec 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -95,6 +95,11 @@ class VM(object):
self._num_drives += 1
return self
+ def hmp_qemu_io(self, drive, cmd):
+ '''Write to a given drive using an HMP command'''
+ return self.qmp('human-monitor-command',
+ command_line='qemu-io %s "%s"' % (drive, cmd))
+
def add_fd(self, fd, fdset, opaque, opts=''):
'''Pass a file descriptor to the VM'''
options = ['fd=%d' % fd,
--
1.8.1.4
^ permalink raw reply related [flat|nested] 10+ messages in thread