[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 5/5] backup: use copy_bitmap in incremental back
From: |
John Snow |
Subject: |
Re: [Qemu-devel] [PATCH 5/5] backup: use copy_bitmap in incremental backup |
Date: |
Mon, 9 Oct 2017 19:51:38 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 |
On 10/02/2017 10:39 AM, Vladimir Sementsov-Ogievskiy wrote:
> We can use copy_bitmap instead of sync_bitmap. copy_bitmap is
> initialized from sync_bitmap and it is more informative: we will not try
> to process data, that is already in progress (by write notifier).
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> Reviewed-by: Stefan Hajnoczi <address@hidden>
> ---
> block/backup.c | 55 +++++++++++++++++--------------------------------------
Lookin' good.
> 1 file changed, 17 insertions(+), 38 deletions(-)
>
> diff --git a/block/backup.c b/block/backup.c
> index 52df7bb19e..c933cfafc3 100644
> --- a/block/backup.c
> +++ b/block/backup.c
> @@ -362,49 +362,28 @@ static bool coroutine_fn yield_and_check(BackupBlockJob
> *job)
>
> static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
> {
> + int ret;
> bool error_is_read;
> - int ret = 0;
> - int clusters_per_iter;
> - uint32_t granularity;
> - int64_t offset;
> int64_t cluster;
> - int64_t end;
> - BdrvDirtyBitmapIter *dbi;
> -
> - granularity = bdrv_dirty_bitmap_granularity(job->sync_bitmap);
> - clusters_per_iter = MAX((granularity / job->cluster_size), 1);
> - dbi = bdrv_dirty_iter_new(job->sync_bitmap);
> -
> - /* Find the next dirty sector(s) */
> - while ((offset = bdrv_dirty_iter_next(dbi)) >= 0) {
> - cluster = offset / job->cluster_size;
> -
> - for (end = cluster + clusters_per_iter; cluster < end; cluster++) {
> - do {
> - if (yield_and_check(job)) {
> - goto out;
> - }
> - ret = backup_do_cow(job, cluster * job->cluster_size,
> - job->cluster_size, &error_is_read,
> - false);
> - if ((ret < 0) &&
> - backup_error_action(job, error_is_read, -ret) ==
> - BLOCK_ERROR_ACTION_REPORT) {
> - goto out;
> - }
> - } while (ret < 0);
> - }
> + HBitmapIter hbi;
>
> - /* If the bitmap granularity is smaller than the backup granularity,
> - * we need to advance the iterator pointer to the next cluster. */
> - if (granularity < job->cluster_size) {
> - bdrv_set_dirty_iter(dbi, cluster * job->cluster_size);
> - }
> + hbitmap_iter_init(&hbi, job->copy_bitmap, 0);
> + while ((cluster = hbitmap_iter_next(&hbi)) != -1) {
> + do {
> + if (yield_and_check(job)) {
> + return 0;
> + }
> + ret = backup_do_cow(job, cluster * job->cluster_size,
> + job->cluster_size, &error_is_read, false);
> + if (ret < 0 && backup_error_action(job, error_is_read, -ret) ==
> + BLOCK_ERROR_ACTION_REPORT)
> + {
> + return ret;
> + }
> + } while (ret < 0);
> }
>
> -out:
> - bdrv_dirty_iter_free(dbi);
> - return ret;
> + return 0;
> }
>
> /* init copy_bitmap from sync_bitmap */
>
Impressive, you really cleaned this function up considerably.
Reviewed-by: John Snow <address@hidden>