From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44257) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XhGGz-0006OR-Uz for qemu-devel@nongnu.org; Thu, 23 Oct 2014 07:09:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XhGGt-0000HE-Lr for qemu-devel@nongnu.org; Thu, 23 Oct 2014 07:09:41 -0400 Received: from mx1.redhat.com ([209.132.183.28]:52138) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XhGGt-0000Gy-EG for qemu-devel@nongnu.org; Thu, 23 Oct 2014 07:09:35 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s9NB9XR6025037 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Thu, 23 Oct 2014 07:09:34 -0400 Message-ID: <5448E1EB.1060507@redhat.com> Date: Thu, 23 Oct 2014 13:09:31 +0200 From: Max Reitz MIME-Version: 1.0 References: <1413982283-10186-1-git-send-email-mreitz@redhat.com> <1413982283-10186-8-git-send-email-mreitz@redhat.com> <20141023105230.GJ3522@noname.redhat.com> In-Reply-To: <20141023105230.GJ3522@noname.redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit 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: Kevin Wolf Cc: qemu-devel@nongnu.org, Stefan Hajnoczi 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. Max