Re: [Qemu-devel] [PATCH v5 03/11] block: add basic backup support to block driver

[Stefan Hajnoczi]

On Wed, Jun 19, 2013 at 01:14:17PM +0200, Paolo Bonzini wrote:
> Il 19/06/2013 12:50, Kevin Wolf ha scritto:
> >> > +        /* Publish progress */
> >> > +        job->sectors_read += n;
> >> > +        job->common.offset += n * BDRV_SECTOR_SIZE;
> > This is interesting, because the function is not only called by the
> > background job, but also by write notifiers. So 'offset' in a literal
> > sense doesn't make too much sense because we're not operating purely
> > sequential.
> > 
> > The QAPI documentation describes 'offset' like this:
> > 
> > # @offset: the current progress value
> > 
> > If we take it as just that, I think we could actually consider this code
> > correct, because it's a useful measure for the progress (each sector is
> > copied only once, either by the job or by a notifier), even though it
> > really has nothing to do with an offset into the image.
> 
> Yes, this is similar to what we do for mirroring.  I think it is a feature.

Yes, we explicitly defined "offset" to be an opaque progress value for
this reason.

I will still add a comment since the name "offset" does make the
drive-backup code suggest the wrong thing.

Stefan