All of lore.kernel.org
 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 17/19] qcow2-bitmap: move bitmap reopen-rw code to qcow2_reopen_commit
Date: Fri, 11 Oct 2019 17:25:48 -0400	[thread overview]
Message-ID: <20191011212550.27269-18-jsnow@redhat.com> (raw)
In-Reply-To: <20191011212550.27269-1-jsnow@redhat.com>

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

The only reason I can imagine for this strange code at the very-end of
bdrv_reopen_commit is the fact that bs->read_only updated after
calling drv->bdrv_reopen_commit in bdrv_reopen_commit. And in the same
time, prior to previous commit, qcow2_reopen_bitmaps_rw did a wrong
check for being writable, when actually it only need writable file
child not self.

So, as it's fixed, let's move things to correct place.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Acked-by: Max Reitz <mreitz@redhat.com>
Message-id: 20190927122355.7344-10-vsementsov@virtuozzo.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
 include/block/block_int.h |  6 ------
 block.c                   | 19 -------------------
 block/qcow2.c             | 15 ++++++++++++++-
 3 files changed, 14 insertions(+), 26 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 1e54486ad14..9ceca23ef75 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -546,12 +546,6 @@ struct BlockDriver {
                              uint64_t parent_perm, uint64_t parent_shared,
                              uint64_t *nperm, uint64_t *nshared);
 
-    /**
-     * Bitmaps should be marked as 'IN_USE' in the image on reopening image
-     * as rw. This handler should realize it. It also should unset readonly
-     * field of BlockDirtyBitmap's in case of success.
-     */
-    int (*bdrv_reopen_bitmaps_rw)(BlockDriverState *bs, Error **errp);
     bool (*bdrv_co_can_store_new_dirty_bitmap)(BlockDriverState *bs,
                                                const char *name,
                                                uint32_t granularity,
diff --git a/block.c b/block.c
index c548885608d..ba09d97e0a2 100644
--- a/block.c
+++ b/block.c
@@ -3935,16 +3935,12 @@ void bdrv_reopen_commit(BDRVReopenState *reopen_state)
     BlockDriver *drv;
     BlockDriverState *bs;
     BdrvChild *child;
-    bool old_can_write, new_can_write;
 
     assert(reopen_state != NULL);
     bs = reopen_state->bs;
     drv = bs->drv;
     assert(drv != NULL);
 
-    old_can_write =
-        !bdrv_is_read_only(bs) && !(bdrv_get_flags(bs) & BDRV_O_INACTIVE);
-
     /* If there are any driver level actions to take */
     if (drv->bdrv_reopen_commit) {
         drv->bdrv_reopen_commit(reopen_state);
@@ -3988,21 +3984,6 @@ void bdrv_reopen_commit(BDRVReopenState *reopen_state)
     }
 
     bdrv_refresh_limits(bs, NULL);
-
-    new_can_write =
-        !bdrv_is_read_only(bs) && !(bdrv_get_flags(bs) & BDRV_O_INACTIVE);
-    if (!old_can_write && new_can_write && drv->bdrv_reopen_bitmaps_rw) {
-        Error *local_err = NULL;
-        if (drv->bdrv_reopen_bitmaps_rw(bs, &local_err) < 0) {
-            /* This is not fatal, bitmaps just left read-only, so all following
-             * writes will fail. User can remove read-only bitmaps to unblock
-             * writes.
-             */
-            error_reportf_err(local_err,
-                              "%s: Failed to make dirty bitmaps writable: ",
-                              bdrv_get_node_name(bs));
-        }
-    }
 }
 
 /*
diff --git a/block/qcow2.c b/block/qcow2.c
index 7ab1aad9779..b652bf884e5 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1834,6 +1834,20 @@ fail:
 static void qcow2_reopen_commit(BDRVReopenState *state)
 {
     qcow2_update_options_commit(state->bs, state->opaque);
+    if (state->flags & BDRV_O_RDWR) {
+        Error *local_err = NULL;
+
+        if (qcow2_reopen_bitmaps_rw(state->bs, &local_err) < 0) {
+            /*
+             * This is not fatal, bitmaps just left read-only, so all following
+             * writes will fail. User can remove read-only bitmaps to unblock
+             * writes or retry reopen.
+             */
+            error_reportf_err(local_err,
+                              "%s: Failed to make dirty bitmaps writable: ",
+                              bdrv_get_node_name(state->bs));
+        }
+    }
     g_free(state->opaque);
 }
 
@@ -5262,7 +5276,6 @@ BlockDriver bdrv_qcow2 = {
     .bdrv_detach_aio_context  = qcow2_detach_aio_context,
     .bdrv_attach_aio_context  = qcow2_attach_aio_context,
 
-    .bdrv_reopen_bitmaps_rw = qcow2_reopen_bitmaps_rw,
     .bdrv_co_can_store_new_dirty_bitmap = qcow2_co_can_store_new_dirty_bitmap,
     .bdrv_co_remove_persistent_dirty_bitmap =
             qcow2_co_remove_persistent_dirty_bitmap,
-- 
2.21.0



  parent reply	other threads:[~2019-10-11 21:43 UTC|newest]

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

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=20191011212550.27269-18-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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.