[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V2 1/2] Implement sync modes for drive-backup.
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH V2 1/2] Implement sync modes for drive-backup. |
Date: |
Mon, 8 Jul 2013 17:23:17 +0800 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Great work! The patch looks good for me, I made two trivial style
comments (if you respin, you can check your patches for style problems
by running scripts/checkpatch.pl).
On Fri, 07/05 18:35, Ian Main wrote:
> 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.
>
> Signed-off-by: Ian Main <address@hidden>
> ---
> block/backup.c | 90
> +++++++++++++++++++++++++++++++----------------
> blockdev.c | 25 ++++++++-----
> include/block/block_int.h | 4 ++-
> qapi-schema.json | 4 +++
> qmp-commands.hx | 1 +
> 5 files changed, 85 insertions(+), 39 deletions(-)
>
> diff --git a/block/backup.c b/block/backup.c
> index 16105d4..e72a5af 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,68 @@ 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 n, alloced;
> +
> + /* Check to see if these blocks are already in the backing
> file. */
line over 80 characters
> +
> + alloced =
> + bdrv_co_is_allocated(bs, start *
> BACKUP_SECTORS_PER_CLUSTER,
> + BACKUP_SECTORS_PER_CLUSTER, &n);
> + /* The above call returns true if the given sector is in the
> + * topmost image. If that is the case then we must copy it
> as
> + * it has been modified from the original backing file.
Trailing whitespace.
Thanks
--
Fam