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