| Message ID | 20220215175310.68058-2-vsementsov@virtuozzo.com |
|---|---|
| State | New |
| Headers | show |
| Series | block/dirty-bitmaps: fix and improve bitmap merge | expand |
I forget that I already sent it in other series: [PATCH v3 02/19] block/dirty-bitmap: bdrv_merge_dirty_bitmap(): add return value "[PATCH v3 02/19] block/dirty-bitmap: bdrv_merge_dirty_bitmap(): add return value" is a bit better as it adds a comment. And has Hanna's r-b 15.02.2022 20:53, Vladimir Sementsov-Ogievskiy wrote: > Add return value to bdrv_merge_dirty_bitmap() and use it to reduce > error propagation. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > include/block/dirty-bitmap.h | 2 +- > block/dirty-bitmap.c | 6 ++++-- > block/monitor/bitmap-qmp-cmds.c | 5 +---- > 3 files changed, 6 insertions(+), 7 deletions(-) > > diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h > index 40950ae3d5..f95d350b70 100644 > --- a/include/block/dirty-bitmap.h > +++ b/include/block/dirty-bitmap.h > @@ -77,7 +77,7 @@ void bdrv_dirty_bitmap_set_persistence(BdrvDirtyBitmap *bitmap, > bool persistent); > void bdrv_dirty_bitmap_set_inconsistent(BdrvDirtyBitmap *bitmap); > void bdrv_dirty_bitmap_set_busy(BdrvDirtyBitmap *bitmap, bool busy); > -void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src, > +bool bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src, > HBitmap **backup, Error **errp); > void bdrv_dirty_bitmap_skip_store(BdrvDirtyBitmap *bitmap, bool skip); > bool bdrv_dirty_bitmap_get(BdrvDirtyBitmap *bitmap, int64_t offset); > diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c > index 0ef46163e3..d16b96ee62 100644 > --- a/block/dirty-bitmap.c > +++ b/block/dirty-bitmap.c > @@ -881,10 +881,10 @@ bool bdrv_dirty_bitmap_next_dirty_area(BdrvDirtyBitmap *bitmap, > * > * @backup: If provided, make a copy of dest here prior to merge. > */ > -void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src, > +bool bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src, > HBitmap **backup, Error **errp) > { > - bool ret; > + bool ret = false; > > bdrv_dirty_bitmaps_lock(dest->bs); > if (src->bs != dest->bs) { > @@ -912,6 +912,8 @@ out: > if (src->bs != dest->bs) { > bdrv_dirty_bitmaps_unlock(src->bs); > } > + > + return ret; > } > > /** > diff --git a/block/monitor/bitmap-qmp-cmds.c b/block/monitor/bitmap-qmp-cmds.c > index 9f11deec64..83970b22fa 100644 > --- a/block/monitor/bitmap-qmp-cmds.c > +++ b/block/monitor/bitmap-qmp-cmds.c > @@ -259,7 +259,6 @@ BdrvDirtyBitmap *block_dirty_bitmap_merge(const char *node, const char *target, > BlockDriverState *bs; > BdrvDirtyBitmap *dst, *src, *anon; > BlockDirtyBitmapMergeSourceList *lst; > - Error *local_err = NULL; > > dst = block_dirty_bitmap_lookup(node, target, &bs, errp); > if (!dst) { > @@ -297,9 +296,7 @@ BdrvDirtyBitmap *block_dirty_bitmap_merge(const char *node, const char *target, > abort(); > } > > - bdrv_merge_dirty_bitmap(anon, src, NULL, &local_err); > - if (local_err) { > - error_propagate(errp, local_err); > + if (!bdrv_merge_dirty_bitmap(anon, src, NULL, errp)) { > dst = NULL; > goto out; > } >
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h index 40950ae3d5..f95d350b70 100644 --- a/include/block/dirty-bitmap.h +++ b/include/block/dirty-bitmap.h @@ -77,7 +77,7 @@ void bdrv_dirty_bitmap_set_persistence(BdrvDirtyBitmap *bitmap, bool persistent); void bdrv_dirty_bitmap_set_inconsistent(BdrvDirtyBitmap *bitmap); void bdrv_dirty_bitmap_set_busy(BdrvDirtyBitmap *bitmap, bool busy); -void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src, +bool bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src, HBitmap **backup, Error **errp); void bdrv_dirty_bitmap_skip_store(BdrvDirtyBitmap *bitmap, bool skip); bool bdrv_dirty_bitmap_get(BdrvDirtyBitmap *bitmap, int64_t offset); diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c index 0ef46163e3..d16b96ee62 100644 --- a/block/dirty-bitmap.c +++ b/block/dirty-bitmap.c @@ -881,10 +881,10 @@ bool bdrv_dirty_bitmap_next_dirty_area(BdrvDirtyBitmap *bitmap, * * @backup: If provided, make a copy of dest here prior to merge. */ -void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src, +bool bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src, HBitmap **backup, Error **errp) { - bool ret; + bool ret = false; bdrv_dirty_bitmaps_lock(dest->bs); if (src->bs != dest->bs) { @@ -912,6 +912,8 @@ out: if (src->bs != dest->bs) { bdrv_dirty_bitmaps_unlock(src->bs); } + + return ret; } /** diff --git a/block/monitor/bitmap-qmp-cmds.c b/block/monitor/bitmap-qmp-cmds.c index 9f11deec64..83970b22fa 100644 --- a/block/monitor/bitmap-qmp-cmds.c +++ b/block/monitor/bitmap-qmp-cmds.c @@ -259,7 +259,6 @@ BdrvDirtyBitmap *block_dirty_bitmap_merge(const char *node, const char *target, BlockDriverState *bs; BdrvDirtyBitmap *dst, *src, *anon; BlockDirtyBitmapMergeSourceList *lst; - Error *local_err = NULL; dst = block_dirty_bitmap_lookup(node, target, &bs, errp); if (!dst) { @@ -297,9 +296,7 @@ BdrvDirtyBitmap *block_dirty_bitmap_merge(const char *node, const char *target, abort(); } - bdrv_merge_dirty_bitmap(anon, src, NULL, &local_err); - if (local_err) { - error_propagate(errp, local_err); + if (!bdrv_merge_dirty_bitmap(anon, src, NULL, errp)) { dst = NULL; goto out; }
Add return value to bdrv_merge_dirty_bitmap() and use it to reduce error propagation. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- include/block/dirty-bitmap.h | 2 +- block/dirty-bitmap.c | 6 ++++-- block/monitor/bitmap-qmp-cmds.c | 5 +---- 3 files changed, 6 insertions(+), 7 deletions(-)