block: simplify handling of try to merge different sized bitmaps
We have too much logic to simply check that bitmaps are of the same size. Let's just define that hbitmap_merge() and bdrv_dirty_bitmap_merge_internal() require their argument bitmaps be of same size, this simplifies things. Let's look through the callers: For backup_init_bcs_bitmap() we already assert that merge can't fail. In bdrv_reclaim_dirty_bitmap_locked() we gracefully handle the error that can't happen: successor always has same size as its parent, drop this logic. In bdrv_merge_dirty_bitmap() we already has assertion and separate check. Make the check explicit and improve error message. Signed-off-by: Vladimir Sementsov-Ogievskiy <v.sementsov-og@mail.ru> Reviewed-by: Nikita Lapshin <nikita.lapshin@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20220517111206.23585-4-v.sementsov-og@mail.ru> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
This commit is contained in:
parent
58cbfbdf73
commit
618af89e55
@ -228,15 +228,13 @@ out:
|
||||
|
||||
static void backup_init_bcs_bitmap(BackupBlockJob *job)
|
||||
{
|
||||
bool ret;
|
||||
uint64_t estimate;
|
||||
BdrvDirtyBitmap *bcs_bitmap = block_copy_dirty_bitmap(job->bcs);
|
||||
|
||||
if (job->sync_mode == MIRROR_SYNC_MODE_BITMAP) {
|
||||
bdrv_clear_dirty_bitmap(bcs_bitmap, NULL);
|
||||
ret = bdrv_dirty_bitmap_merge_internal(bcs_bitmap, job->sync_bitmap,
|
||||
NULL, true);
|
||||
assert(ret);
|
||||
bdrv_dirty_bitmap_merge_internal(bcs_bitmap, job->sync_bitmap, NULL,
|
||||
true);
|
||||
} else if (job->sync_mode == MIRROR_SYNC_MODE_TOP) {
|
||||
/*
|
||||
* We can't hog the coroutine to initialize this thoroughly.
|
||||
|
@ -309,10 +309,7 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BdrvDirtyBitmap *parent,
|
||||
return NULL;
|
||||
}
|
||||
|
||||
if (!hbitmap_merge(parent->bitmap, successor->bitmap, parent->bitmap)) {
|
||||
error_setg(errp, "Merging of parent and successor bitmap failed");
|
||||
return NULL;
|
||||
}
|
||||
hbitmap_merge(parent->bitmap, successor->bitmap, parent->bitmap);
|
||||
|
||||
parent->disabled = successor->disabled;
|
||||
parent->busy = false;
|
||||
@ -912,13 +909,15 @@ bool bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src,
|
||||
goto out;
|
||||
}
|
||||
|
||||
if (!hbitmap_can_merge(dest->bitmap, src->bitmap)) {
|
||||
error_setg(errp, "Bitmaps are incompatible and can't be merged");
|
||||
if (bdrv_dirty_bitmap_size(src) != bdrv_dirty_bitmap_size(dest)) {
|
||||
error_setg(errp, "Bitmaps are of different sizes (destination size is %"
|
||||
PRId64 ", source size is %" PRId64 ") and can't be merged",
|
||||
bdrv_dirty_bitmap_size(dest), bdrv_dirty_bitmap_size(src));
|
||||
goto out;
|
||||
}
|
||||
|
||||
ret = bdrv_dirty_bitmap_merge_internal(dest, src, backup, false);
|
||||
assert(ret);
|
||||
bdrv_dirty_bitmap_merge_internal(dest, src, backup, false);
|
||||
ret = true;
|
||||
|
||||
out:
|
||||
bdrv_dirty_bitmaps_unlock(dest->bs);
|
||||
@ -932,17 +931,16 @@ out:
|
||||
/**
|
||||
* bdrv_dirty_bitmap_merge_internal: merge src into dest.
|
||||
* Does NOT check bitmap permissions; not suitable for use as public API.
|
||||
* @dest, @src and @backup (if not NULL) must have same size.
|
||||
*
|
||||
* @backup: If provided, make a copy of dest here prior to merge.
|
||||
* @lock: If true, lock and unlock bitmaps on the way in/out.
|
||||
* returns true if the merge succeeded; false if unattempted.
|
||||
*/
|
||||
bool bdrv_dirty_bitmap_merge_internal(BdrvDirtyBitmap *dest,
|
||||
void bdrv_dirty_bitmap_merge_internal(BdrvDirtyBitmap *dest,
|
||||
const BdrvDirtyBitmap *src,
|
||||
HBitmap **backup,
|
||||
bool lock)
|
||||
{
|
||||
bool ret;
|
||||
IO_CODE();
|
||||
|
||||
assert(!bdrv_dirty_bitmap_readonly(dest));
|
||||
@ -959,9 +957,9 @@ bool bdrv_dirty_bitmap_merge_internal(BdrvDirtyBitmap *dest,
|
||||
if (backup) {
|
||||
*backup = dest->bitmap;
|
||||
dest->bitmap = hbitmap_alloc(dest->size, hbitmap_granularity(*backup));
|
||||
ret = hbitmap_merge(*backup, src->bitmap, dest->bitmap);
|
||||
hbitmap_merge(*backup, src->bitmap, dest->bitmap);
|
||||
} else {
|
||||
ret = hbitmap_merge(dest->bitmap, src->bitmap, dest->bitmap);
|
||||
hbitmap_merge(dest->bitmap, src->bitmap, dest->bitmap);
|
||||
}
|
||||
|
||||
if (lock) {
|
||||
@ -970,6 +968,4 @@ bool bdrv_dirty_bitmap_merge_internal(BdrvDirtyBitmap *dest,
|
||||
bdrv_dirty_bitmaps_unlock(src->bs);
|
||||
}
|
||||
}
|
||||
|
||||
return ret;
|
||||
}
|
||||
|
@ -102,7 +102,7 @@ bool blk_dev_is_tray_open(BlockBackend *blk);
|
||||
void bdrv_set_dirty(BlockDriverState *bs, int64_t offset, int64_t bytes);
|
||||
|
||||
void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap **out);
|
||||
bool bdrv_dirty_bitmap_merge_internal(BdrvDirtyBitmap *dest,
|
||||
void bdrv_dirty_bitmap_merge_internal(BdrvDirtyBitmap *dest,
|
||||
const BdrvDirtyBitmap *src,
|
||||
HBitmap **backup, bool lock);
|
||||
|
||||
|
@ -76,20 +76,9 @@ void hbitmap_truncate(HBitmap *hb, uint64_t size);
|
||||
*
|
||||
* Store result of merging @a and @b into @result.
|
||||
* @result is allowed to be equal to @a or @b.
|
||||
*
|
||||
* Return true if the merge was successful,
|
||||
* false if it was not attempted.
|
||||
* All bitmaps must have same size.
|
||||
*/
|
||||
bool hbitmap_merge(const HBitmap *a, const HBitmap *b, HBitmap *result);
|
||||
|
||||
/**
|
||||
* hbitmap_can_merge:
|
||||
*
|
||||
* hbitmap_can_merge(a, b) && hbitmap_can_merge(a, result) is sufficient and
|
||||
* necessary for hbitmap_merge will not fail.
|
||||
*
|
||||
*/
|
||||
bool hbitmap_can_merge(const HBitmap *a, const HBitmap *b);
|
||||
void hbitmap_merge(const HBitmap *a, const HBitmap *b, HBitmap *result);
|
||||
|
||||
/**
|
||||
* hbitmap_empty:
|
||||
|
@ -873,11 +873,6 @@ void hbitmap_truncate(HBitmap *hb, uint64_t size)
|
||||
}
|
||||
}
|
||||
|
||||
bool hbitmap_can_merge(const HBitmap *a, const HBitmap *b)
|
||||
{
|
||||
return (a->orig_size == b->orig_size);
|
||||
}
|
||||
|
||||
/**
|
||||
* hbitmap_sparse_merge: performs dst = dst | src
|
||||
* works with differing granularities.
|
||||
@ -901,28 +896,24 @@ static void hbitmap_sparse_merge(HBitmap *dst, const HBitmap *src)
|
||||
* Given HBitmaps A and B, let R := A (BITOR) B.
|
||||
* Bitmaps A and B will not be modified,
|
||||
* except when bitmap R is an alias of A or B.
|
||||
*
|
||||
* @return true if the merge was successful,
|
||||
* false if it was not attempted.
|
||||
* Bitmaps must have same size.
|
||||
*/
|
||||
bool hbitmap_merge(const HBitmap *a, const HBitmap *b, HBitmap *result)
|
||||
void hbitmap_merge(const HBitmap *a, const HBitmap *b, HBitmap *result)
|
||||
{
|
||||
int i;
|
||||
uint64_t j;
|
||||
|
||||
if (!hbitmap_can_merge(a, b) || !hbitmap_can_merge(a, result)) {
|
||||
return false;
|
||||
}
|
||||
assert(hbitmap_can_merge(b, result));
|
||||
assert(a->orig_size == result->orig_size);
|
||||
assert(b->orig_size == result->orig_size);
|
||||
|
||||
if ((!hbitmap_count(a) && result == b) ||
|
||||
(!hbitmap_count(b) && result == a)) {
|
||||
return true;
|
||||
return;
|
||||
}
|
||||
|
||||
if (!hbitmap_count(a) && !hbitmap_count(b)) {
|
||||
hbitmap_reset_all(result);
|
||||
return true;
|
||||
return;
|
||||
}
|
||||
|
||||
if (a->granularity != b->granularity) {
|
||||
@ -935,7 +926,7 @@ bool hbitmap_merge(const HBitmap *a, const HBitmap *b, HBitmap *result)
|
||||
if (b != result) {
|
||||
hbitmap_sparse_merge(result, b);
|
||||
}
|
||||
return true;
|
||||
return;
|
||||
}
|
||||
|
||||
/* This merge is O(size), as BITS_PER_LONG and HBITMAP_LEVELS are constant.
|
||||
@ -951,8 +942,6 @@ bool hbitmap_merge(const HBitmap *a, const HBitmap *b, HBitmap *result)
|
||||
|
||||
/* Recompute the dirty count */
|
||||
result->count = hb_count_between(result, 0, result->size - 1);
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
char *hbitmap_sha256(const HBitmap *bitmap, Error **errp)
|
||||
|
Loading…
Reference in New Issue
Block a user