qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: John Snow <jsnow@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>, qemu-devel@nongnu.org
Cc: Fam Zheng <fam@euphon.net>, Kevin Wolf <kwolf@redhat.com>,
	Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
	qemu-block@nongnu.org, Juan Quintela <quintela@redhat.com>,
	libvir-list@redhat.com, John Snow <jsnow@redhat.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	Max Reitz <mreitz@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	Markus Armbruster <armbru@redhat.com>
Subject: [PULL v2 12/19] block/qcow2-bitmap: get rid of bdrv_has_changed_persistent_bitmaps
Date: Mon, 14 Oct 2019 15:29:02 -0400	[thread overview]
Message-ID: <20191014192909.16044-13-jsnow@redhat.com> (raw)
In-Reply-To: <20191014192909.16044-1-jsnow@redhat.com>

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Firstly, no reason to optimize failure path. Then, function name is
ambiguous: it checks for readonly and similar things, but someone may
think that it will ignore normal bitmaps which was just unchanged, and
this is in bad relation with the fact that we should drop IN_USE flag
for unchanged bitmaps in the image.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 20190927122355.7344-5-vsementsov@virtuozzo.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
 include/block/dirty-bitmap.h |  1 -
 block/dirty-bitmap.c         | 12 ------------
 block/qcow2-bitmap.c         | 23 +++++++++++++----------
 3 files changed, 13 insertions(+), 23 deletions(-)

diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 257f0f6704..958e7474fb 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -95,7 +95,6 @@ bool bdrv_has_readonly_bitmaps(BlockDriverState *bs);
 bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap);
 bool bdrv_dirty_bitmap_get_persistence(BdrvDirtyBitmap *bitmap);
 bool bdrv_dirty_bitmap_inconsistent(const BdrvDirtyBitmap *bitmap);
-bool bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs);
 
 BdrvDirtyBitmap *bdrv_dirty_bitmap_first(BlockDriverState *bs);
 BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BdrvDirtyBitmap *bitmap);
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 6065db8094..4bbb251b2c 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -839,18 +839,6 @@ bool bdrv_dirty_bitmap_inconsistent(const BdrvDirtyBitmap *bitmap)
     return bitmap->inconsistent;
 }
 
-bool bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs)
-{
-    BdrvDirtyBitmap *bm;
-    QLIST_FOREACH(bm, &bs->dirty_bitmaps, list) {
-        if (bm->persistent && !bm->readonly && !bm->skip_store) {
-            return true;
-        }
-    }
-
-    return false;
-}
-
 BdrvDirtyBitmap *bdrv_dirty_bitmap_first(BlockDriverState *bs)
 {
     return QLIST_FIRST(&bs->dirty_bitmaps);
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index 99812b418b..6dfc083548 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -1464,16 +1464,7 @@ void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp)
     Qcow2Bitmap *bm;
     QSIMPLEQ_HEAD(, Qcow2BitmapTable) drop_tables;
     Qcow2BitmapTable *tb, *tb_next;
-
-    if (!bdrv_has_changed_persistent_bitmaps(bs)) {
-        /* nothing to do */
-        return;
-    }
-
-    if (!can_write(bs)) {
-        error_setg(errp, "No write access");
-        return;
-    }
+    bool need_write = false;
 
     QSIMPLEQ_INIT(&drop_tables);
 
@@ -1499,6 +1490,8 @@ void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp)
             continue;
         }
 
+        need_write = true;
+
         if (check_constraints_on_bitmap(bs, name, granularity, errp) < 0) {
             error_prepend(errp, "Bitmap '%s' doesn't satisfy the constraints: ",
                           name);
@@ -1537,6 +1530,15 @@ void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp)
         bm->dirty_bitmap = bitmap;
     }
 
+    if (!need_write) {
+        goto success;
+    }
+
+    if (!can_write(bs)) {
+        error_setg(errp, "No write access");
+        goto fail;
+    }
+
     /* allocate clusters and store bitmaps */
     QSIMPLEQ_FOREACH(bm, bm_list, entry) {
         if (bm->dirty_bitmap == NULL) {
@@ -1578,6 +1580,7 @@ void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp)
         bdrv_release_dirty_bitmap(bm->dirty_bitmap);
     }
 
+success:
     bitmap_list_free(bm_list);
     return;
 
-- 
2.21.0



  parent reply	other threads:[~2019-10-14 19:47 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-14 19:28 [PULL v2 00/19] Bitmaps patches John Snow
2019-10-14 19:28 ` [PULL v2 01/19] util/hbitmap: strict hbitmap_reset John Snow
2019-10-14 19:28 ` [PULL v2 02/19] block: move bdrv_can_store_new_dirty_bitmap to block/dirty-bitmap.c John Snow
2019-10-14 19:28 ` [PULL v2 03/19] block/dirty-bitmap: return int from bdrv_remove_persistent_dirty_bitmap John Snow
2019-10-14 19:28 ` [PULL v2 04/19] block/qcow2: proper locking on bitmap add/remove paths John Snow
2019-10-14 19:28 ` [PULL v2 05/19] block/dirty-bitmap: drop meta John Snow
2019-10-14 19:28 ` [PULL v2 06/19] block/dirty-bitmap: add bs link John Snow
2019-10-14 19:28 ` [PULL v2 07/19] block/dirty-bitmap: drop BdrvDirtyBitmap.mutex John Snow
2019-10-14 19:28 ` [PULL v2 08/19] block/dirty-bitmap: refactor bdrv_dirty_bitmap_next John Snow
2019-10-14 19:28 ` [PULL v2 09/19] block: switch reopen queue from QSIMPLEQ to QTAILQ John Snow
2019-10-14 19:29 ` [PULL v2 10/19] block: reverse order for reopen commits John Snow
2019-10-14 19:29 ` [PULL v2 11/19] iotests: add test-case to 165 to test reopening qcow2 bitmaps to RW John Snow
2019-10-14 19:29 ` John Snow [this message]
2019-10-14 19:29 ` [PULL v2 13/19] block/qcow2-bitmap: drop qcow2_reopen_bitmaps_rw_hint() John Snow
2019-10-14 19:29 ` [PULL v2 14/19] block/qcow2-bitmap: do not remove bitmaps on reopen-ro John Snow
2019-10-14 19:29 ` [PULL v2 15/19] iotests: add test 260 to check bitmap life after snapshot + commit John Snow
2019-10-14 19:29 ` [PULL v2 16/19] block/qcow2-bitmap: fix and improve qcow2_reopen_bitmaps_rw John Snow
2019-10-14 19:29 ` [PULL v2 17/19] qcow2-bitmap: move bitmap reopen-rw code to qcow2_reopen_commit John Snow
2019-10-14 19:29 ` [PULL v2 18/19] MAINTAINERS: Add Vladimir as a reviewer for bitmaps John Snow
2019-10-14 19:29 ` [PULL v2 19/19] dirty-bitmaps: remove deprecated autoload parameter John Snow
2019-10-17 11:07 ` [PULL v2 00/19] Bitmaps patches Peter Maydell
2019-10-17 19:34   ` John Snow

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191014192909.16044-13-jsnow@redhat.com \
    --to=jsnow@redhat.com \
    --cc=armbru@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=fam@euphon.net \
    --cc=kwolf@redhat.com \
    --cc=libvir-list@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    --cc=stefanha@redhat.com \
    --cc=vsementsov@virtuozzo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).