From nobody Fri Oct 3 15:49:19 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1552077294155114.25799952871387; Fri, 8 Mar 2019 12:34:54 -0800 (PST) Received: from localhost ([127.0.0.1]:49521 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h2MCg-0004fB-1b for importer@patchew.org; Fri, 08 Mar 2019 15:34:50 -0500 Received: from eggs.gnu.org ([209.51.188.92]:44775) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h2M7E-0000nP-W3 for qemu-devel@nongnu.org; Fri, 08 Mar 2019 15:29:14 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1h2M7B-0006ov-E5 for qemu-devel@nongnu.org; Fri, 08 Mar 2019 15:29:11 -0500 Received: from mx1.redhat.com ([209.132.183.28]:38295) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1h2M78-0006j9-A4 for qemu-devel@nongnu.org; Fri, 08 Mar 2019 15:29:07 -0500 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 4B7A43082133; Fri, 8 Mar 2019 20:29:04 +0000 (UTC) Received: from probe.bos.redhat.com (dhcp-17-160.bos.redhat.com [10.18.17.160]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4A89A1FA; Fri, 8 Mar 2019 20:29:02 +0000 (UTC) From: John Snow To: qemu-devel@nongnu.org Date: Fri, 8 Mar 2019 15:28:43 -0500 Message-Id: <20190308202858.26636-3-jsnow@redhat.com> In-Reply-To: <20190308202858.26636-1-jsnow@redhat.com> References: <20190308202858.26636-1-jsnow@redhat.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.42]); Fri, 08 Mar 2019 20:29:04 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PULL 02/17] block/dirty-bitmaps: rename frozen predicate helper X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: peter.maydell@linaro.org, jsnow@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" "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 bdrv_enable_dirty_bitmap_locked and bdrv_disable_dirty_bitmap_locked, it doesn't make sense to prohibit QEMU internals from performing this action when we only wished to prohibit QMP users from issuing these commands. All of the QMP API commands for bitmap manipulation already check against user_locked() to prohibit these actions. Several other assertions really want to check that the bitmap isn't in-use by another operation -- use the bitmap_user_locked function for this instea= d, which presently also checks for has_successor. This leaves some redundant checks of has_successor through different helpers that are addressed in forthcoming patches. Signed-off-by: John Snow Reviewed-by: Eric Blake Reviewed-by: Vladimir Sementsov-Ogievskiy Message-id: 20190223000614.13894-3-jsnow@redhat.com Signed-off-by: John Snow --- include/block/dirty-bitmap.h | 2 +- block/dirty-bitmap.c | 34 +++++++++++++++++++--------------- migration/block-dirty-bitmap.c | 2 +- 3 files changed, 21 insertions(+), 17 deletions(-) diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h index 04a117fc81..cdbb4dfefd 100644 --- a/include/block/dirty-bitmap.h +++ b/include/block/dirty-bitmap.h @@ -36,7 +36,7 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverS= tate *bs); uint32_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs); uint32_t bdrv_dirty_bitmap_granularity(const BdrvDirtyBitmap *bitmap); bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap); -bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap); +bool bdrv_dirty_bitmap_has_successor(BdrvDirtyBitmap *bitmap); const char *bdrv_dirty_bitmap_name(const BdrvDirtyBitmap *bitmap); int64_t bdrv_dirty_bitmap_size(const BdrvDirtyBitmap *bitmap); DirtyBitmapStatus bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap); diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c index 101383b3af..c8fa21b526 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 st= ate */ char *name; /* Optional non-empty unique ID */ int64_t size; /* Size of the bitmap, in bytes */ bool disabled; /* Bitmap is disabled. It ignores all writ= es to @@ -183,14 +183,14 @@ const char *bdrv_dirty_bitmap_name(const BdrvDirtyBit= map *bitmap) } =20 /* Called with BQL taken. */ -bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap) +bool bdrv_dirty_bitmap_has_successor(BdrvDirtyBitmap *bitmap) { return bitmap->successor; } =20 /* 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); } =20 @@ -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) =20 /** * Create a successor bitmap destined to replace this bitmap after an oper= ation. - * Requires that the bitmap is not frozen and has no successor. + * Requires that the bitmap is not user_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(BlockDriverSta= te *bs, uint64_t granularity; BdrvDirtyBitmap *child; =20 - 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 i= n-use " + "by an operation"); + return -1; + } + if (bdrv_dirty_bitmap_has_successor(bitmap)) { + error_setg(errp, "Cannot create a successor for a bitmap that alre= ady " + "has one"); return -1; } - assert(!bitmap->successor); =20 /* Create an anonymous successor */ granularity =3D bdrv_dirty_bitmap_granularity(bitmap); @@ -268,7 +272,6 @@ int bdrv_dirty_bitmap_create_successor(BlockDriverState= *bs, =20 void bdrv_enable_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap) { - assert(!bdrv_dirty_bitmap_frozen(bitmap)); bitmap->disabled =3D false; } =20 @@ -285,7 +288,8 @@ void bdrv_dirty_bitmap_enable_successor(BdrvDirtyBitmap= *bitmap) static void bdrv_release_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap) { assert(!bitmap->active_iterators); - assert(!bdrv_dirty_bitmap_frozen(bitmap)); + assert(!bdrv_dirty_bitmap_user_locked(bitmap)); + assert(!bdrv_dirty_bitmap_has_successor(bitmap)); assert(!bitmap->meta); QLIST_REMOVE(bitmap, list); hbitmap_free(bitmap->bitmap); @@ -325,7 +329,7 @@ BdrvDirtyBitmap *bdrv_dirty_bitmap_abdicate(BlockDriver= State *bs, /** * In cases of failure where we can no longer safely delete the parent, * we may wish to re-join the parent and child/successor. - * The merged parent will be un-frozen, but not explicitly re-enabled. + * The merged parent will not be user_locked, nor explicitly re-enabled. * Called within bdrv_dirty_bitmap_lock..unlock and with BQL taken. */ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs, @@ -373,7 +377,8 @@ void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, i= nt64_t bytes) =20 bdrv_dirty_bitmaps_lock(bs); QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) { - assert(!bdrv_dirty_bitmap_frozen(bitmap)); + assert(!bdrv_dirty_bitmap_user_locked(bitmap)); + assert(!bdrv_dirty_bitmap_has_successor(bitmap)); assert(!bitmap->active_iterators); hbitmap_truncate(bitmap->bitmap, bytes); bitmap->size =3D bytes; @@ -391,7 +396,7 @@ void bdrv_release_dirty_bitmap(BlockDriverState *bs, Bd= rvDirtyBitmap *bitmap) =20 /** * Release all named dirty bitmaps attached to a BDS (for use in bdrv_clos= e()). - * There must not be any frozen bitmaps attached. + * There must not be any user_locked bitmaps attached. * This function does not remove persistent bitmaps from the storage. * Called with BQL taken. */ @@ -428,7 +433,6 @@ void bdrv_remove_persistent_dirty_bitmap(BlockDriverSta= te *bs, void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap) { bdrv_dirty_bitmap_lock(bitmap); - assert(!bdrv_dirty_bitmap_frozen(bitmap)); bitmap->disabled =3D true; bdrv_dirty_bitmap_unlock(bitmap); } diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c index 6426151e4f..ac6954142f 100644 --- a/migration/block-dirty-bitmap.c +++ b/migration/block-dirty-bitmap.c @@ -542,7 +542,7 @@ static void dirty_bitmap_load_complete(QEMUFile *f, Dir= tyBitmapLoadState *s) } } =20 - if (bdrv_dirty_bitmap_frozen(s->bitmap)) { + if (bdrv_dirty_bitmap_has_successor(s->bitmap)) { bdrv_dirty_bitmap_lock(s->bitmap); if (enabled_bitmaps =3D=3D NULL) { /* in postcopy */ --=20 2.17.2