[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 02/21] backup: init copy_bitmap from sync_bitmap

From: Vladimir Sementsov-Ogievskiy
Subject: Re: [Qemu-devel] [PATCH 02/21] backup: init copy_bitmap from sync_bitmap for incremental
Date: Tue, 24 Jan 2017 13:16:05 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0

24.01.2017 12:46, Fam Zheng wrote:
On Tue, 01/24 12:00, Vladimir Sementsov-Ogievskiy wrote:
   static void coroutine_fn backup_run(void *opaque)
       BackupBlockJob *job = opaque;
@@ -453,20 +481,22 @@ static void coroutine_fn backup_run(void *opaque)
       end = DIV_ROUND_UP(job->common.len, job->cluster_size);
       job->copy_bitmap = hbitmap_alloc(end, 0);
-    hbitmap_set(job->copy_bitmap, 0, end);
       job->before_write.notify = backup_before_write_notify;
       bdrv_add_before_write_notifier(bs, &job->before_write);
       if (job->sync_mode == MIRROR_SYNC_MODE_NONE) {
+        hbitmap_set(job->copy_bitmap, 0, end);
This is confusing. It seems job->copy_bitmap is actually a superset of clusters
to copy instead of the exact one?  Because "none" mode doesn't need blanket copy
- only overwritten clusters are copied...
We can't guess, what clusters should be copied finally in none mode. None
mode is done by allowing only notifier handling and no linear copying. But
notifier handling will copy only clusters marked in copy_bitmap, so I set it
from 0 to end.
Yes, that's how I understand it too, what I dislike is this bit of inconsistency
with it: "allowed to copy bitmap" here versus "planned to copy" in other modes.
I don't know how to improve that, but I think at least the specialty of none
mode could be mentioned in the comment of copy_bitmap.

Ok, comment will be good. I'll add it.

Best regards,

reply via email to

[Prev in Thread] Current Thread [Next in Thread]