[PULL 08/17] qcow2: do not try to clear the dirty bit on a read-only node

From: "Denis V. Lunev" <[email protected]>

qcow2_do_close() -> qcow2_inactivate() clears the dirty bit with a
plain write to bs->file, unconditionally. A read-only node can still
be dirty, inherited from an earlier writable session, and that write
then hits a missing BLK_PERM_WRITE and asserts in
bdrv_co_write_req_prepare() (block/io.c) on an entirely ordinary
close -- closing is expected, the dirty bit on a read-only node
is not.

Skip the clear for read-only nodes, same as read access already does.
Any other still-dirty node keeps the unguarded write: it is expected
to hold write permission, and a missing one there is a bug worth
seeing.

Signed-off-by: Denis V. Lunev <[email protected]>
CC: Kevin Wolf <[email protected]>
CC: Hanna Reitz <[email protected]>
Message-ID: <[email protected]>
Reviewed-by: Kevin Wolf <[email protected]>
Signed-off-by: Kevin Wolf <[email protected]>
---
 block/qcow2.c              |  6 +++++-
 tests/qemu-iotests/039     | 11 +++++++++++
 tests/qemu-iotests/039.out |  3 +++
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 19271b10a49..7292dd036c9 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2870,7 +2870,11 @@ static int GRAPH_RDLOCK qcow2_inactivate(BlockDriverState *bs)
                      strerror(-ret));
     }
 
-    if (result == 0) {
+    /*
+     * A read-only node cannot resolve an inherited dirty bit here;
+     * leave it dirty, same as plain read access already does.
+     */
+    if (result == 0 && !bdrv_is_read_only(bs)) {
         qcow2_mark_clean(bs);
     }
 
diff --git a/tests/qemu-iotests/039 b/tests/qemu-iotests/039
index e43e7026ce3..94a8bfe754e 100755
--- a/tests/qemu-iotests/039
+++ b/tests/qemu-iotests/039
@@ -84,6 +84,17 @@ $QEMU_IO -r -c "read -P 0x5a 0 512" "$TEST_IMG" | _filter_qemu_io
 # The dirty bit must be set
 _qcow2_dump_header | grep incompatible_features
 
+echo
+echo "== Read-only open must not crash on close =="
+
+# We must not try to write the QCOW2 header to a read-only image.
+$QEMU_IMG info --image-opts \
+    "driver=$IMGFMT,read-only=on,file.driver=file,file.filename=$TEST_IMG,file.read-only=off" \
+    > /dev/null
+
+# The dirty bit must still be set: this open never wrote any guest data
+_qcow2_dump_header | grep incompatible_features
+
 echo
 echo "== Repairing the image file must succeed =="
 
diff --git a/tests/qemu-iotests/039.out b/tests/qemu-iotests/039.out
index 8fdbcc528aa..c66361128f6 100644
--- a/tests/qemu-iotests/039.out
+++ b/tests/qemu-iotests/039.out
@@ -24,6 +24,9 @@ read 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 incompatible_features     [0]
 
+== Read-only open must not crash on close ==
+incompatible_features     [0]
+
 == Repairing the image file must succeed ==
 ERROR cluster 5 refcount=0 reference=1
 Rebuilding refcount structure
-- 
2.55.0