diff options
author | Kevin Wolf <kwolf@redhat.com> | 2014-04-03 13:47:50 +0200 |
---|---|---|
committer | Kevin Wolf <kwolf@redhat.com> | 2014-04-04 14:12:26 +0200 |
commit | 4c2e5f8f46a17966dc45b5a3e07b97434c0eabdf (patch) | |
tree | aa545165b570b6c297d75dd0ae623abd3861e4f1 | |
parent | cbee81f6de57ddc1b21ba28f01f6a3b5d87428a5 (diff) | |
download | qemu-4c2e5f8f46a17966dc45b5a3e07b97434c0eabdf.zip qemu-4c2e5f8f46a17966dc45b5a3e07b97434c0eabdf.tar.gz qemu-4c2e5f8f46a17966dc45b5a3e07b97434c0eabdf.tar.bz2 |
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.
Cc: qemu-stable@nongnu.org
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
-rw-r--r-- | block/qcow2.c | 25 | ||||
-rwxr-xr-x | tests/qemu-iotests/039 | 20 | ||||
-rw-r--r-- | tests/qemu-iotests/039.out | 11 |
3 files changed, 52 insertions, 4 deletions
diff --git a/block/qcow2.c b/block/qcow2.c index 333e26d..e903d97 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -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; } diff --git a/tests/qemu-iotests/039 b/tests/qemu-iotests/039 index 9b355c0..b9cbe99 100755 --- a/tests/qemu-iotests/039 +++ b/tests/qemu-iotests/039 @@ -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 diff --git a/tests/qemu-iotests/039.out b/tests/qemu-iotests/039.out index 077fa64..fb31ae0 100644 --- a/tests/qemu-iotests/039.out +++ b/tests/qemu-iotests/039.out @@ -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 |