qcow2: Flush metadata during read-only reopen
PATCH 1/1 qcow2: Flush metadata during read-only reopen
If lazy refcounts are enabled for a backing file, committing to this backing file may leave it in a dirty state even if the commit succeeds. The reason is that the bdrv_flush() call in bdrv_commit() doesn't flush refcount updates with lazy refcounts enabled, and qcow2_reopen_prepare() doesn't take care to flush metadata.
In order to fix this, this patch also fixes qcow2_mark_clean(), which contains another ineffective bdrv_flush() call beause lazy refcounts are disabled only afterwards. All existing callers of qcow2_mark_clean() either don't modify refcounts or already flush manually, so that this fixes only a latent, but not yet actually triggerable bug.
Another instance of the same problem is live snapshots. Again, a real corruption is prevented by an explicit flush for non-read-only images in external_snapshot_prepare(), but images using lazy refcounts stay dirty.
--- block/qcow2.c | 25 +++++++++++++++++++++---- tests/qemu-iotests/039 | 20 ++++++++++++++++++++ tests/qemu-iotests/039.out | 11 +++++++++++ 3 files changed, 52 insertions(+), 4 deletions(-)
block/qcow2.c +21 -4
@@ -269,12 +269,15 @@ static int qcow2_mark_clean(BlockDriverState *bs)BDRVQcowState *s = bs->opaque;if (s->incompatible_features &QCOW2_INCOMPAT_DIRTY ) { -int ret =bdrv_flush (bs); +int ret; + + s->incompatible_features &= ~QCOW2_INCOMPAT_DIRTY ; + + ret =bdrv_flush (bs);if (ret <0 ) {return ret; } - s->incompatible_features &= ~QCOW2_INCOMPAT_DIRTY ;return qcow2_update_header (bs); }return 0 ;
@@ -900,11 +903,25 @@ static int qcow2_set_key(BlockDriverState *bs, const char *key)return 0 ; } -/* We have nothing to do for QCOW2 reopen, stubs just return -* success */ +/* We have no actual commit/abort logic for qcow2, but we need to write out any +* unwritten data if we reopen read-only. */ static int qcow2_reopen_prepare (BDRVReopenState *state,BlockReopenQueue *queue,Error **errp) { +int ret; + +if ((state->flags &BDRV_O_RDWR ) ==0 ) { + ret =bdrv_flush (state->bs); +if (ret <0 ) { +return ret; + } + + ret =qcow2_mark_clean (state->bs); +if (ret <0 ) { +return ret; + } + } +return 0 ; }
tests/qemu-iotests/039 +20 -0
@@ -131,6 +131,26 @@ ulimit -c "$old_ulimit" ./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features _check_test_img +echo +echo "== Committing to a backing file with lazy_refcounts=on ==" + +IMGOPTS="compat=1.1,lazy_refcounts=on" +TEST_IMG="$TEST_IMG".base _make_test_img $size + +IMGOPTS="compat=1.1,lazy_refcounts=on,backing_file=$TEST_IMG.base" +_make_test_img $size + +$QEMU_IO -c "write 0 512" "$TEST_IMG" | _filter_qemu_io +$QEMU_IMG commit "$TEST_IMG" + +# The dirty bit must not be set +./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features +./qcow2.py "$TEST_IMG".base dump-header | grep incompatible_features + +_check_test_img +TEST_IMG="$TEST_IMG".base _check_test_img + + # success, all done echo "*** done" rm -f $seq.full
tests/qemu-iotests/039.out +11 -0
@@ -54,4 +54,15 @@ wrote 512/512 bytes at offset 0 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) incompatible_features 0x0 No errors were found on the image. + +== Committing to a backing file with lazy_refcounts=on == +Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=134217728 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file='TEST_DIR/t.IMGFMT.base' +wrote 512/512 bytes at offset 0 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +Image committed. +incompatible_features 0x0 +incompatible_features 0x0 +No errors were found on the image. +No errors were found on the image. *** done
Signature · 1 line
1.8.3.1
Quoted 16 lines · Kevin Wolf
If lazy refcounts are enabled for a backing file, committing to this backing file may leave it in a dirty state even if the commit succeeds. The reason is that the
bdrv_flush()call inbdrv_commit()doesn't flush refcount updates with lazy refcounts enabled, andqcow2_reopen_prepare()doesn't take care to flush metadata.In order to fix this, this patch also fixes
qcow2_mark_clean(), which contains another ineffectivebdrv_flush()call beause lazy refcounts are disabled only afterwards. All existing callers ofqcow2_mark_clean()either don't modify refcounts or already flush manually, so that this fixes only a latent, but not yet actually triggerable bug.Another instance of the same problem is live snapshots. Again, a real corruption is prevented by an explicit flush for non-read-only images in
external_snapshot_prepare(), but images using lazy refcounts stay dirty.
Quoted 7 lines
Signed-off-by Kevin Wolf <[email protected]>--- block/qcow2.c | 25 +++++++++++++++++++++---- tests/qemu-iotests/039 | 20 ++++++++++++++++++++ tests/qemu-iotests/039.out | 11 +++++++++++ 3 files changed, 52 insertions(+), 4 deletions(-)