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 <im...@redhat.com> > --- > 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