From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:53857)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1UpGLM-0002WF-CW
	for qemu-devel@nongnu.org; Wed, 19 Jun 2013 07:14:29 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1UpGLL-0000d3-4y
	for qemu-devel@nongnu.org; Wed, 19 Jun 2013 07:14:28 -0400
Received: from mx1.redhat.com ([209.132.183.28]:5628)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1UpGLK-0000cz-T0
	for qemu-devel@nongnu.org; Wed, 19 Jun 2013 07:14:27 -0400
Message-ID: <51C19289.1080301@redhat.com>
Date: Wed, 19 Jun 2013 13:14:17 +0200
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
References: <1369917299-5725-1-git-send-email-stefanha@redhat.com>
	<1369917299-5725-4-git-send-email-stefanha@redhat.com>
	<20130619105015.GA2934@dhcp-200-207.str.redhat.com>
In-Reply-To: <20130619105015.GA2934@dhcp-200-207.str.redhat.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v5 03/11] block: add basic backup support
	to block driver
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Kevin Wolf <kwolf@redhat.com>
Cc: Fam Zheng <famz@redhat.com>, qemu-devel@nongnu.org, dietmar@proxmox.com, imain@redhat.com, Stefan Hajnoczi <stefanha@redhat.com>, xiawenc@linux.vnet.ibm.com

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.

Paolo

> Maybe a comment would be appropriate.