qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v20 13/30] block: new bdrv_reopen_bitmaps_rw int


From: Vladimir Sementsov-Ogievskiy
Subject: Re: [Qemu-devel] [PATCH v20 13/30] block: new bdrv_reopen_bitmaps_rw interface
Date: Wed, 14 Jun 2017 12:03:05 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1

13.06.2017 18:28, Max Reitz wrote:
On 2017-06-13 12:25, Vladimir Sementsov-Ogievskiy wrote:
09.06.2017 16:27, Max Reitz wrote:
On 2017-06-02 13:21, Vladimir Sementsov-Ogievskiy wrote:
Add format driver handler, which should mark loaded read-only
bitmaps as 'IN_USE' in the image and unset read_only field in
corresponding BdrvDirtyBitmap's.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
---
   block.c                   | 17 +++++++++++++++++
   include/block/block_int.h |  7 +++++++
   2 files changed, 24 insertions(+)

diff --git a/block.c b/block.c
index 04af7697dc..161db9e32a 100644
--- a/block.c
+++ b/block.c
@@ -2946,12 +2946,16 @@ void bdrv_reopen_commit(BDRVReopenState
*reopen_state)
   {
       BlockDriver *drv;
       BlockDriverState *bs;
+    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);
@@ -2965,6 +2969,19 @@ void bdrv_reopen_commit(BDRVReopenState
*reopen_state)
       bs->read_only = !(reopen_state->flags & BDRV_O_RDWR);
         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.
+             */
In a sense, it pretty much is fatal. We were asked to make the image
non-read-only but we failed because it effectively still is read-only.

But I can't think of anything better, and you're right, removing the
bitmaps would resolve the situation. This would require the user to know
that updating the bitmaps was the issue, and local_err may not actually
reflect that.

+            error_report_err(local_err);
So I'd prepend this with something like "$node_name: Failed to make
dirty bitmaps writable", and maybe append a hint like "Removing all
persistent dirty bitmaps from this node will allow writing to it".
Ok for prepending, but I don't want to add last note, as for the user it
may better to retry an operation, leading reopening image rw..
Which operation do you mean? The reopening? Because that operation
already "succeeded" at this point, so you can't retry it...

So, firstly, reopen r-o again and then reopen r-w? Is it possible?


Max


--
Best regards,
Vladimir




reply via email to

[Prev in Thread] Current Thread [Next in Thread]