block/mirror: fix and improve do_sync_target_write
Use bdrv_dirty_bitmap_next_dirty_area() instead of bdrv_dirty_iter_next_area(), because of the following problems of bdrv_dirty_iter_next_area(): 1. Using HBitmap iterators we should carefully handle unaligned offset, as first call to hbitmap_iter_next() may return a value less than original offset (actually, it will be original offset rounded down to bitmap granularity). This handling is not done in do_sync_target_write(). 2. bdrv_dirty_iter_next_area() handles unaligned max_offset incorrectly: look at the code: if (max_offset == iter->bitmap->size) { /* If max_offset points to the image end, round it up by the * bitmap granularity */ gran_max_offset = ROUND_UP(max_offset, granularity); } else { gran_max_offset = max_offset; } ret = hbitmap_iter_next(&iter->hbi, false); if (ret < 0 || ret + granularity > gran_max_offset) { return false; } and assume that max_offset != iter->bitmap->size but still unaligned. if 0 < ret < max_offset we found dirty area, but the function can return false in this case (if ret + granularity > max_offset). 3. bdrv_dirty_iter_next_area() uses inefficient loop to find the end of the dirty area. Let's use more efficient hbitmap_next_zero instead (bdrv_dirty_bitmap_next_dirty_area() do so) Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
This commit is contained in:
parent
bb6a0ec10e
commit
1eaf1b0fdf
@ -1185,25 +1185,23 @@ do_sync_target_write(MirrorBlockJob *job, MirrorMethod method,
|
||||
uint64_t offset, uint64_t bytes,
|
||||
QEMUIOVector *qiov, int flags)
|
||||
{
|
||||
BdrvDirtyBitmapIter *iter;
|
||||
QEMUIOVector target_qiov;
|
||||
uint64_t dirty_offset;
|
||||
int dirty_bytes;
|
||||
uint64_t dirty_offset = offset;
|
||||
uint64_t dirty_bytes;
|
||||
|
||||
if (qiov) {
|
||||
qemu_iovec_init(&target_qiov, qiov->niov);
|
||||
}
|
||||
|
||||
iter = bdrv_dirty_iter_new(job->dirty_bitmap);
|
||||
bdrv_set_dirty_iter(iter, offset);
|
||||
|
||||
while (true) {
|
||||
bool valid_area;
|
||||
int ret;
|
||||
|
||||
bdrv_dirty_bitmap_lock(job->dirty_bitmap);
|
||||
valid_area = bdrv_dirty_iter_next_area(iter, offset + bytes,
|
||||
&dirty_offset, &dirty_bytes);
|
||||
dirty_bytes = MIN(offset + bytes - dirty_offset, INT_MAX);
|
||||
valid_area = bdrv_dirty_bitmap_next_dirty_area(job->dirty_bitmap,
|
||||
&dirty_offset,
|
||||
&dirty_bytes);
|
||||
if (!valid_area) {
|
||||
bdrv_dirty_bitmap_unlock(job->dirty_bitmap);
|
||||
break;
|
||||
@ -1259,9 +1257,10 @@ do_sync_target_write(MirrorBlockJob *job, MirrorMethod method,
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
dirty_offset += dirty_bytes;
|
||||
}
|
||||
|
||||
bdrv_dirty_iter_free(iter);
|
||||
if (qiov) {
|
||||
qemu_iovec_destroy(&target_qiov);
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user