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>
master
Kevin Wolf 2014-04-03 13:47:50 +02:00
parent cbee81f6de
commit 4c2e5f8f46
3 changed files with 52 additions and 4 deletions

View File

@ -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;
}

View File

@ -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

View File

@ -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