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.