From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56041) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XhH75-0005ga-0m for qemu-devel@nongnu.org; Thu, 23 Oct 2014 08:03:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XhH6x-0004ZR-8Z for qemu-devel@nongnu.org; Thu, 23 Oct 2014 08:03:30 -0400 Received: from mx1.redhat.com ([209.132.183.28]:5646) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XhH6w-0004ZJ-Vr for qemu-devel@nongnu.org; Thu, 23 Oct 2014 08:03:23 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s9NC3MZ0013859 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Thu, 23 Oct 2014 08:03:22 -0400 Date: Thu, 23 Oct 2014 14:03:19 +0200 From: Kevin Wolf Message-ID: <20141023120319.GL3522@noname.redhat.com> References: <1413982283-10186-1-git-send-email-mreitz@redhat.com> <1413982283-10186-8-git-send-email-mreitz@redhat.com> <20141023105230.GJ3522@noname.redhat.com> <5448E1EB.1060507@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5448E1EB.1060507@redhat.com> Subject: Re: [Qemu-devel] [PATCH v13 07/14] block/mirror: Improve progress report List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: qemu-devel@nongnu.org, Stefan Hajnoczi Am 23.10.2014 um 13:09 hat Max Reitz geschrieben: > On 2014-10-23 at 12:52, Kevin Wolf wrote: > >Am 22.10.2014 um 14:51 hat Max Reitz geschrieben: > >>Instead of taking the total length of the block device as the block > >>job's length, use the number of dirty sectors. The progress is now the > >>number of sectors mirrored to the target block device. Note that this > >>may result in the job's length increasing during operation, which is > >>however in fact desirable. > >More importantly, because it might surprise management tools, is that > >the progress (as in offset/len) can actually decrease now. > > > >I can't say whether that creates any problem, I'll rely on Eric's > >Reviewed-by for that. > > Eric said, it would rather solve problems. > > >>Signed-off-by: Max Reitz > >>Reviewed-by: Eric Blake > >>--- > >> block/mirror.c | 34 ++++++++++++++++++++++------------ > >> 1 file changed, 22 insertions(+), 12 deletions(-) > >>@@ -409,6 +416,12 @@ static void coroutine_fn mirror_run(void *opaque) > >> } > >> cnt = bdrv_get_dirty_count(bs, s->dirty_bitmap); > >>+ /* s->common.offset contains the number of bytes already processed so > >>+ * far, cnt is the number of dirty sectors remaining and > >>+ * s->sectors_in_flight is the number of sectors currently being > >>+ * processed; together those are the current total operation length */ > >>+ s->common.len = s->common.offset + > >>+ (cnt + s->sectors_in_flight) * BDRV_SECTOR_SIZE; > >Isn't s->sectors_in_flight still contained in cnt? If I understand > >correctly, sectors are only marked as clean at the same time as > >s->sectors_in_flight is decremented again. > > As far as I see, bdrv_reset_dirty() is called in mirror_iteration() > right before s->sectors_in_flight is incremented. You're right, I confused it with the other bitmap operations in mirror_iteration_done(). So: Reviewed-by: Kevin Wolf