From nobody Sat Nov 8 10:38:51 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 1551468114829569.8413518508329; Fri, 1 Mar 2019 11:21:54 -0800 (PST) Received: from localhost ([127.0.0.1]:42811 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gznjB-0006Ub-Jy for importer@patchew.org; Fri, 01 Mar 2019 14:21:50 -0500 Received: from eggs.gnu.org ([209.51.188.92]:34792) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gzndm-0002GQ-NX for qemu-devel@nongnu.org; Fri, 01 Mar 2019 14:16:17 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gzndj-0008Gu-1Q for qemu-devel@nongnu.org; Fri, 01 Mar 2019 14:16:12 -0500 Received: from mx1.redhat.com ([209.132.183.28]:47894) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gzndW-00081R-Pi; Fri, 01 Mar 2019 14:16:00 -0500 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D3981306D33A; Fri, 1 Mar 2019 19:15:55 +0000 (UTC) Received: from probe.bos.redhat.com (dhcp-17-206.bos.redhat.com [10.18.17.206]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9FD1D1001DE2; Fri, 1 Mar 2019 19:15:54 +0000 (UTC) From: John Snow To: qemu-devel@nongnu.org, qemu-block@nongnu.org Date: Fri, 1 Mar 2019 14:15:41 -0500 Message-Id: <20190301191545.8728-4-jsnow@redhat.com> In-Reply-To: <20190301191545.8728-1-jsnow@redhat.com> References: <20190301191545.8728-1-jsnow@redhat.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.47]); Fri, 01 Mar 2019 19:15:55 +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] [PATCH v3 3/7] block/dirty-bitmaps: add block_dirty_bitmap_check function 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: Fam Zheng , Kevin Wolf , vsementsov@virtuozzo.com, Juan Quintela , Markus Armbruster , Max Reitz , Stefan Hajnoczi , John Snow , "Dr. David Alan Gilbert" 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" Instead of checking against busy, inconsistent, or read only directly, use a check function with permissions bits that let us streamline the checks without reproducing them in many places. Included in this patch are permissions changes that simply add the inconsistent check to existing permissions call spots, without addressing existing bugs. In general, this means that busy+readonly checks become BDRV_BITMAP_DEFAULT, which checks against all three conditions. busy-only checks become BDRV_BITMAP_ALLOW_RO. Notably, remove allows inconsistent bitmaps, so it doesn't follow the patte= rn. Signed-off-by: John Snow Reviewed-by: Eric Blake Reviewed-by: Vladimir Sementsov-Ogievskiy --- include/block/dirty-bitmap.h | 13 ++++++++- block/dirty-bitmap.c | 38 +++++++++++++++++++------- blockdev.c | 49 +++++++--------------------------- migration/block-dirty-bitmap.c | 12 +++------ nbd/server.c | 3 +-- 5 files changed, 54 insertions(+), 61 deletions(-) diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h index bd1b6479df..2a78243954 100644 --- a/include/block/dirty-bitmap.h +++ b/include/block/dirty-bitmap.h @@ -5,6 +5,16 @@ #include "qapi/qapi-types-block-core.h" #include "qemu/hbitmap.h" =20 +typedef enum BitmapCheckFlags { + BDRV_BITMAP_BUSY =3D 1, + BDRV_BITMAP_RO =3D 2, + BDRV_BITMAP_INCONSISTENT =3D 4, +} BitmapCheckFlags; + +#define BDRV_BITMAP_DEFAULT (BDRV_BITMAP_BUSY | BDRV_BITMAP_RO | \ + BDRV_BITMAP_INCONSISTENT) +#define BDRV_BITMAP_ALLOW_RO (BDRV_BITMAP_BUSY | BDRV_BITMAP_INCONSISTENT) + BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, uint32_t granularity, const char *name, @@ -24,6 +34,8 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverSta= te *bs, void bdrv_dirty_bitmap_enable_successor(BdrvDirtyBitmap *bitmap); BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs, const char *name); +int bdrv_dirty_bitmap_check(const BdrvDirtyBitmap *bitmap, uint32_t flags, + Error **errp); void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitm= ap); void bdrv_release_named_dirty_bitmaps(BlockDriverState *bs); void bdrv_remove_persistent_dirty_bitmap(BlockDriverState *bs, @@ -93,7 +105,6 @@ bool bdrv_has_readonly_bitmaps(BlockDriverState *bs); bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap); bool bdrv_dirty_bitmap_get_persistance(BdrvDirtyBitmap *bitmap); bool bdrv_dirty_bitmap_inconsistent(const BdrvDirtyBitmap *bitmap); -bool bdrv_dirty_bitmap_busy(BdrvDirtyBitmap *bitmap); bool bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs); BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BlockDriverState *bs, BdrvDirtyBitmap *bitmap); diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c index 71e0098396..769668ccdc 100644 --- a/block/dirty-bitmap.c +++ b/block/dirty-bitmap.c @@ -174,7 +174,7 @@ bool bdrv_dirty_bitmap_has_successor(BdrvDirtyBitmap *b= itmap) return bitmap->successor; } =20 -bool bdrv_dirty_bitmap_busy(BdrvDirtyBitmap *bitmap) { +static bool bdrv_dirty_bitmap_busy(const BdrvDirtyBitmap *bitmap) { return bitmap->busy; } =20 @@ -235,6 +235,33 @@ static bool bdrv_dirty_bitmap_recording(BdrvDirtyBitma= p *bitmap) !bitmap->successor->disabled); } =20 +int bdrv_dirty_bitmap_check(const BdrvDirtyBitmap *bitmap, uint32_t flags, + Error **errp) +{ + if ((flags & BDRV_BITMAP_BUSY) && bdrv_dirty_bitmap_busy(bitmap)) { + error_setg(errp, "Bitmap '%s' is currently in use by another" + " operation and cannot be used", bitmap->name); + return -1; + } + + if ((flags & BDRV_BITMAP_RO) && bdrv_dirty_bitmap_readonly(bitmap)) { + error_setg(errp, "Bitmap '%s' is readonly and cannot be modified", + bitmap->name); + return -1; + } + + if ((flags & BDRV_BITMAP_INCONSISTENT) && + bdrv_dirty_bitmap_inconsistent(bitmap)) { + error_setg(errp, "Bitmap '%s' is inconsistent and cannot be used", + bitmap->name); + error_append_hint(errp, "Try block-dirty-bitmap-remove to delete " + "this bitmap from disk"); + return -1; + } + + return 0; +} + /** * Create a successor bitmap destined to replace this bitmap after an oper= ation. * Requires that the bitmap is not marked busy and has no successor. @@ -794,17 +821,10 @@ void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, c= onst BdrvDirtyBitmap *src, =20 qemu_mutex_lock(dest->mutex); =20 - if (bdrv_dirty_bitmap_busy(dest)) { - error_setg(errp, "Bitmap '%s' is currently in use by another" - " operation and cannot be modified", dest->name); + if (bdrv_dirty_bitmap_check(dest, BDRV_BITMAP_DEFAULT, errp)) { goto out; } =20 - if (bdrv_dirty_bitmap_readonly(dest)) { - error_setg(errp, "Bitmap '%s' is readonly and cannot be modified", - dest->name); - goto out; - } =20 if (!hbitmap_can_merge(dest->bitmap, src->bitmap)) { error_setg(errp, "Bitmaps are incompatible and can't be merged"); diff --git a/blockdev.c b/blockdev.c index cbce44877d..5d74479ba7 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2007,11 +2007,7 @@ static void block_dirty_bitmap_clear_prepare(BlkActi= onState *common, return; } =20 - if (bdrv_dirty_bitmap_busy(state->bitmap)) { - error_setg(errp, "Cannot modify a bitmap in use by another operati= on"); - return; - } else if (bdrv_dirty_bitmap_readonly(state->bitmap)) { - error_setg(errp, "Cannot clear a readonly bitmap"); + if (bdrv_dirty_bitmap_check(state->bitmap, BDRV_BITMAP_DEFAULT, errp))= { return; } =20 @@ -2056,10 +2052,7 @@ static void block_dirty_bitmap_enable_prepare(BlkAct= ionState *common, return; } =20 - if (bdrv_dirty_bitmap_busy(state->bitmap)) { - error_setg(errp, - "Bitmap '%s' is currently in use by another operation" - " and cannot be enabled", action->name); + if (bdrv_dirty_bitmap_check(state->bitmap, BDRV_BITMAP_ALLOW_RO, errp)= ) { return; } =20 @@ -2097,10 +2090,7 @@ static void block_dirty_bitmap_disable_prepare(BlkAc= tionState *common, return; } =20 - if (bdrv_dirty_bitmap_busy(state->bitmap)) { - error_setg(errp, - "Bitmap '%s' is currently in use by another operation" - " and cannot be disabled", action->name); + if (bdrv_dirty_bitmap_check(state->bitmap, BDRV_BITMAP_ALLOW_RO, errp)= ) { return; } =20 @@ -2891,10 +2881,7 @@ void qmp_block_dirty_bitmap_remove(const char *node,= const char *name, return; } =20 - if (bdrv_dirty_bitmap_busy(bitmap)) { - error_setg(errp, - "Bitmap '%s' is currently in use by another operation a= nd" - " cannot be removed", name); + if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_BUSY, errp)) { return; } =20 @@ -2930,13 +2917,7 @@ void qmp_block_dirty_bitmap_clear(const char *node, = const char *name, return; } =20 - if (bdrv_dirty_bitmap_busy(bitmap)) { - error_setg(errp, - "Bitmap '%s' is currently in use by another operation" - " and cannot be cleared", name); - return; - } else if (bdrv_dirty_bitmap_readonly(bitmap)) { - error_setg(errp, "Bitmap '%s' is readonly and cannot be cleared", = name); + if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_DEFAULT, errp)) { return; } =20 @@ -2954,10 +2935,7 @@ void qmp_block_dirty_bitmap_enable(const char *node,= const char *name, return; } =20 - if (bdrv_dirty_bitmap_busy(bitmap)) { - error_setg(errp, - "Bitmap '%s' is currently in use by another operation" - " and cannot be enabled", name); + if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_ALLOW_RO, errp)) { return; } =20 @@ -2975,10 +2953,7 @@ void qmp_block_dirty_bitmap_disable(const char *node= , const char *name, return; } =20 - if (bdrv_dirty_bitmap_busy(bitmap)) { - error_setg(errp, - "Bitmap '%s' is currently in use by another operation" - " and cannot be disabled", name); + if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_ALLOW_RO, errp)) { return; } =20 @@ -3551,10 +3526,7 @@ static BlockJob *do_drive_backup(DriveBackup *backup= , JobTxn *txn, bdrv_unref(target_bs); goto out; } - if (bdrv_dirty_bitmap_busy(bmap)) { - error_setg(errp, - "Bitmap '%s' is currently in use by another operati= on" - " and cannot be used for backup", backup->bitmap); + if (bdrv_dirty_bitmap_check(bmap, BDRV_BITMAP_ALLOW_RO, errp)) { goto out; } } @@ -3664,10 +3636,7 @@ BlockJob *do_blockdev_backup(BlockdevBackup *backup,= JobTxn *txn, error_setg(errp, "Bitmap '%s' could not be found", backup->bit= map); goto out; } - if (bdrv_dirty_bitmap_busy(bmap)) { - error_setg(errp, - "Bitmap '%s' is currently in use by another operati= on" - " and cannot be used for backup", backup->bitmap); + if (bdrv_dirty_bitmap_check(bmap, BDRV_BITMAP_ALLOW_RO, errp)) { goto out; } } diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c index 7057fff242..0fcf897f32 100644 --- a/migration/block-dirty-bitmap.c +++ b/migration/block-dirty-bitmap.c @@ -274,6 +274,7 @@ static int init_dirty_bitmap_migration(void) BdrvDirtyBitmap *bitmap; DirtyBitmapMigBitmapState *dbms; BdrvNextIterator it; + Error *local_err =3D NULL; =20 dirty_bitmap_mig_state.bulk_completed =3D false; dirty_bitmap_mig_state.prev_bs =3D NULL; @@ -301,15 +302,8 @@ static int init_dirty_bitmap_migration(void) goto fail; } =20 - if (bdrv_dirty_bitmap_busy(bitmap)) { - error_report("Can't migrate a bitmap that is in use by ano= ther operation: '%s'", - bdrv_dirty_bitmap_name(bitmap)); - goto fail; - } - - if (bdrv_dirty_bitmap_readonly(bitmap)) { - error_report("Can't migrate read-only dirty bitmap: '%s", - bdrv_dirty_bitmap_name(bitmap)); + if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_DEFAULT, &loca= l_err)) { + error_report_err(local_err); goto fail; } =20 diff --git a/nbd/server.c b/nbd/server.c index 02773e2836..9b87c7f243 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -1510,8 +1510,7 @@ NBDExport *nbd_export_new(BlockDriverState *bs, uint6= 4_t dev_offset, goto fail; } =20 - if (bdrv_dirty_bitmap_busy(bm)) { - error_setg(errp, "Bitmap '%s' is in use", bitmap); + if (bdrv_dirty_bitmap_check(bm, BDRV_BITMAP_ALLOW_RO, errp)) { goto fail; } =20 --=20 2.17.2