From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47750) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YML5M-00020W-LL for qemu-devel@nongnu.org; Fri, 13 Feb 2015 13:35:29 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YML5I-0000H8-M9 for qemu-devel@nongnu.org; Fri, 13 Feb 2015 13:35:28 -0500 Received: from mx1.redhat.com ([209.132.183.28]:49443) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YML5I-0000Gx-EY for qemu-devel@nongnu.org; Fri, 13 Feb 2015 13:35:24 -0500 Message-ID: <54DE43E9.6000508@redhat.com> Date: Fri, 13 Feb 2015 13:35:21 -0500 From: John Snow MIME-Version: 1.0 References: <1423532117-14490-1-git-send-email-jsnow@redhat.com> <1423532117-14490-8-git-send-email-jsnow@redhat.com> <54DE3562.3020405@parallels.com> In-Reply-To: <54DE3562.3020405@parallels.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v12 07/17] qmp: Add support of "dirty-bitmap" sync mode for drive-backup List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org, vladimir Sementsov-Ogievskiy On 02/13/2015 12:33 PM, Vladimir Sementsov-Ogievskiy wrote: > On 10.02.2015 04:35, John Snow wrote: >> ...... >> @@ -278,28 +305,61 @@ static void coroutine_fn backup_run(void *opaque= ) >> qemu_coroutine_yield(); >> job->common.busy =3D true; >> } >> + } else if (job->sync_mode =3D=3D MIRROR_SYNC_MODE_DIRTY_BITMAP) { >> + /* Dirty Bitmap sync has a slightly different iteration >> method */ >> + HBitmapIter hbi; >> + int64_t sector; >> + int64_t cluster; >> + int64_t last_cluster =3D -1; >> + bool polyrhythmic; >> + >> + bdrv_dirty_iter_init(bs, job->sync_bitmap, &hbi); >> + /* Does the granularity happen to match our backup cluster >> size? */ >> + polyrhythmic =3D >> (bdrv_dirty_bitmap_granularity(job->sync_bitmap) !=3D >> + BACKUP_CLUSTER_SIZE); > let it be false, i.e. granularity =3D=3D cluster >> + >> + /* Find the next dirty /sector/ and copy that /cluster/ */ >> + while ((sector =3D hbitmap_iter_next(&hbi)) !=3D -1) { > then, don't we skip here the very first cluster, if it is dirty? I don't think so. The HBitmapIterator is still in its initial state=20 here, and it can and will return 0 if the first sector is dirty. The iotest submitted in v12 tests writes to the very first sector, and I=20 just verified it quickly that this function *does* return 0 for the=20 first go-around. >> + if (yield_and_check(job)) { >> + goto leave; >> + } >> + cluster =3D sector / BACKUP_SECTORS_PER_CLUSTER; >> + >> + /* Play some catchup with the progress meter */ >> + if (cluster !=3D last_cluster + 1) { >> + job->common.offset +=3D ((cluster - last_cluster - 1)= * >> + BACKUP_CLUSTER_SIZE); >> + } >> + >> + do { >> + ret =3D backup_do_cow(bs, cluster * >> BACKUP_SECTORS_PER_CLUSTER, >> + BACKUP_SECTORS_PER_CLUSTER, >> &error_is_read); >> + if ((ret < 0) && >> + backup_error_action(job, error_is_read, -ret) =3D= =3D >> + BLOCK_ERROR_ACTION_REPORT) { >> + goto leave; >> + } >> + } while (ret < 0); >> + >> + /* Advance (or rewind) our iterator if we need to. */ >> + if (polyrhythmic) { >> + bdrv_set_dirty_iter(&hbi, >> + (cluster + 1) * >> BACKUP_SECTORS_PER_CLUSTER); >> + } >> + >> + last_cluster =3D cluster; >> + } >> + >> + /* Play some final catchup with the progress meter */ >> + if (last_cluster + 1 < end) { >> + job->common.offset +=3D ((end - last_cluster - 1) * >> + BACKUP_CLUSTER_SIZE); >> + } >> + >> } else { > --=20 =E2=80=94js