Re: [Qemu-block] [PATCH 2/2] backup: Use copy offloading

[Stefan Hajnoczi]

On Thu, May 31, 2018 at 10:34:45AM +0800, Fam Zheng wrote:
> The implementation is similar to the 'qemu-img convert'. In the
> beginning of the job, offloaded copy is attempted. If it fails, further
> I/O will go through the existing bounce buffer code path.
> 
> Signed-off-by: Fam Zheng <address@hidden>
> ---
>  block/backup.c     | 93 
> +++++++++++++++++++++++++++++++++++-------------------
>  block/trace-events |  1 +
>  2 files changed, 62 insertions(+), 32 deletions(-)
> 
> diff --git a/block/backup.c b/block/backup.c
> index 4e228e959b..ab189693f4 100644
> --- a/block/backup.c
> +++ b/block/backup.c
> @@ -45,6 +45,8 @@ typedef struct BackupBlockJob {
>      QLIST_HEAD(, CowRequest) inflight_reqs;
>  
>      HBitmap *copy_bitmap;
> +    bool use_copy_range;
> +    int64_t copy_range_size;
>  } BackupBlockJob;
>  
>  static const BlockJobDriver backup_job_driver;
> @@ -111,49 +113,70 @@ static int coroutine_fn backup_do_cow(BackupBlockJob 
> *job,
>      cow_request_begin(&cow_request, job, start, end);
>  
>      for (; start < end; start += job->cluster_size) {
> +retry:

This for loop is becoming complex.  Please introduce helper functions.
The loop body can be replaced with something like this:

  if (!hbitmap_get(job->copy_bitmap, start / job->cluster_size)) {
      trace_backup_do_cow_skip(job, start);
      continue; /* already copied */
  }

  trace_backup_do_cow_process(job, start);

  ret = -ENOTSUPP;
  if (job->use_copy_range) {
      ret = cow_with_offload(...);
  }
  if (ret < 0) {
      job->use_copy_range = false;
      ret = cow_with_bounce_buffer(...);
  }
  if (ret < 0) {
      trace_backup_do_cow_write_fail(job, start, ret);
      goto out;
  }

signature.asc
Description: PGP signature