Re: [libvirt] [PATCH v2 2/6] block/dirty-bitmaps: rename frozen predicate helper

"Frozen" was a good description a long time ago, but it isn't adequate now. Rename the frozen predicate to has_successor to make the semantics of the predicate more clear to outside callers. In the process, remove some calls to frozen() that no longer semantically make sense. For enabled and disabled in particular, it's actually okay for the internals to do this but only forbidden for users to invoke them, and

all of the QMP entry uses already check against qmp_locked. Several other assertions really want to check that the bitmap isn't in-use by another operation -- use the qmp_locked function for this instead, which presently also checks for has_successor.

Signed-off-by: John Snow <jsnow(a)redhat.com&gt; --- block/dirty-bitmap.c | 32 +++++++++++++++++--------------- include/block/dirty-bitmap.h | 2 +- migration/block-dirty-bitmap.c | 2 +- 3 files changed, 19 insertions(+), 17 deletions(-) diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c index 101383b3af..639ebc0645 100644 --- a/block/dirty-bitmap.c +++ b/block/dirty-bitmap.c @@ -50,7 +50,7 @@ struct BdrvDirtyBitmap { HBitmap *meta; /* Meta dirty bitmap */ bool qmp_locked; /* Bitmap is locked, it can't be modified through QMP */ - BdrvDirtyBitmap *successor; /* Anonymous child; implies frozen status */ + BdrvDirtyBitmap *successor; /* Anonymous child; implies user_locked state */

char *name; /* Optional non-empty unique ID */ int64_t size; /* Size of the bitmap, in bytes */ bool disabled; /* Bitmap is disabled. It ignores all writes to @@ -183,14 +183,14 @@ const char *bdrv_dirty_bitmap_name(const BdrvDirtyBitmap *bitmap) } /* Called with BQL taken. */ -bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap) +bool bdrv_dirty_bitmap_has_successor(BdrvDirtyBitmap *bitmap) { return bitmap->successor; } /* Both conditions disallow user-modification via QMP. */ bool bdrv_dirty_bitmap_user_locked(BdrvDirtyBitmap *bitmap) { - return bdrv_dirty_bitmap_frozen(bitmap) || + return bdrv_dirty_bitmap_has_successor(bitmap) || bdrv_dirty_bitmap_qmp_locked(bitmap); } @@ -215,7 +215,7 @@ bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap) /* Called with BQL taken. */ DirtyBitmapStatus bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap) { - if (bdrv_dirty_bitmap_frozen(bitmap)) { + if (bdrv_dirty_bitmap_has_successor(bitmap)) { return DIRTY_BITMAP_STATUS_FROZEN; } else if (bdrv_dirty_bitmap_qmp_locked(bitmap)) { return DIRTY_BITMAP_STATUS_LOCKED; @@ -235,7 +235,7 @@ static bool bdrv_dirty_bitmap_recording(BdrvDirtyBitmap *bitmap) /** * Create a successor bitmap destined to replace this bitmap after an operation. - * Requires that the bitmap is not frozen and has no successor. + * Requires that the bitmap is not locked and has no successor.

* Called with BQL taken. */ int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs, @@ -244,12 +244,16 @@ int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs, uint64_t granularity; BdrvDirtyBitmap *child; - if (bdrv_dirty_bitmap_frozen(bitmap)) { - error_setg(errp, "Cannot create a successor for a bitmap that is " - "currently frozen"); + if (bdrv_dirty_bitmap_user_locked(bitmap)) { + error_setg(errp, "Cannot create a successor for a bitmap that is in-use " + "by an operation"); + return -1; + } + if (bdrv_dirty_bitmap_has_successor(bitmap)) { + error_setg(errp, "Cannot create a successor for a bitmap that already " + "has one");

return -1; } - assert(!bitmap->successor); /* Create an anonymous successor */ granularity = bdrv_dirty_bitmap_granularity(bitmap);