From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:35704)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <vsementsov@parallels.com>) id 1YMK8F-0005Lo-Bz
	for qemu-devel@nongnu.org; Fri, 13 Feb 2015 12:34:28 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <vsementsov@parallels.com>) id 1YMK8A-0003LB-8f
	for qemu-devel@nongnu.org; Fri, 13 Feb 2015 12:34:23 -0500
Received: from mx2.parallels.com ([199.115.105.18]:57238)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <vsementsov@parallels.com>) id 1YMK8A-0003Am-2n
	for qemu-devel@nongnu.org; Fri, 13 Feb 2015 12:34:18 -0500
Message-ID: <54DE3562.3020405@parallels.com>
Date: Fri, 13 Feb 2015 20:33:22 +0300
From: Vladimir Sementsov-Ogievskiy <vsementsov@parallels.com>
MIME-Version: 1.0
References: <1423532117-14490-1-git-send-email-jsnow@redhat.com>
	<1423532117-14490-8-git-send-email-jsnow@redhat.com>
In-Reply-To: <1423532117-14490-8-git-send-email-jsnow@redhat.com>
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v12 07/17] qmp: Add support of
 "dirty-bitmap" sync mode for drive-backup
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: John Snow <jsnow@redhat.com>, qemu-devel@nongnu.org
Cc: kwolf@redhat.com, famz@redhat.com, armbru@redhat.com, mreitz@redhat.com, stefanha@redhat.com

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 = true;
>           }
> +    } else if (job->sync_mode == 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 = -1;
> +        bool polyrhythmic;
> +
> +        bdrv_dirty_iter_init(bs, job->sync_bitmap, &hbi);
> +        /* Does the granularity happen to match our backup cluster size? */
> +        polyrhythmic = (bdrv_dirty_bitmap_granularity(job->sync_bitmap) !=
> +                        BACKUP_CLUSTER_SIZE);
let it be false, i.e. granularity == cluster
> +
> +        /* Find the next dirty /sector/ and copy that /cluster/ */
> +        while ((sector = hbitmap_iter_next(&hbi)) != -1) {
then, don't we skip here the very first cluster, if it is dirty?
> +            if (yield_and_check(job)) {
> +                goto leave;
> +            }
> +            cluster = sector / BACKUP_SECTORS_PER_CLUSTER;
> +
> +            /* Play some catchup with the progress meter */
> +            if (cluster != last_cluster + 1) {
> +                job->common.offset += ((cluster - last_cluster - 1) *
> +                                       BACKUP_CLUSTER_SIZE);
> +            }
> +
> +            do {
> +                ret = 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) ==
> +                    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 = cluster;
> +        }
> +
> +        /* Play some final catchup with the progress meter */
> +        if (last_cluster + 1 < end) {
> +            job->common.offset += ((end - last_cluster - 1) *
> +                                   BACKUP_CLUSTER_SIZE);
> +        }
> +
>       } else {
>   

-- 
Best regards,
Vladimir