qcow2: Fix cluster leaks on write error
cover qcow2: Fix cluster leaks on write error
Kevin Wolf (3):
qemu-iotests: Update 026.out.nocache reference output
qcow2: Free allocated clusters on write error
qemu-iotests: Test qcow2 not leaking clusters on write error
block/qcow2.h | 1 + block/qcow2-cluster.c | 11 +++++++++++ block/qcow2.c | 2 ++ tests/qemu-iotests/026 | 17 +++++++++++++++++ tests/qemu-iotests/026.out | 8 ++++++++ tests/qemu-iotests/026.out.nocache | 14 +++++++++++--- 6 files changed, 50 insertions(+), 3 deletions(-)
Signature · 1 line
2.13.6
PATCH 1/3 qemu-iotests: Update 026.out.nocache reference output
Commit abf754fe406 updated 026.out, but forgot to also update 026.out.nocache.
--- tests/qemu-iotests/026.out.nocache | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
tests/qemu-iotests/026.out.nocache +3 -3
@@ -541,7 +541,7 @@ Failed to flush the L2 table cache: No space left on device Failed to flush the refcount block cache: No space left on device write failed: No space left on device -11 leaked clusters were found on the image. +10 leaked clusters were found on the image. This means waste of disk space, but no harm to data. Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
@@ -569,7 +569,7 @@ Failed to flush the L2 table cache: No space left on device Failed to flush the refcount block cache: No space left on device write failed: No space left on device -11 leaked clusters were found on the image. +10 leaked clusters were found on the image. This means waste of disk space, but no harm to data. Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
@@ -597,7 +597,7 @@ Failed to flush the L2 table cache: No space left on device Failed to flush the refcount block cache: No space left on device write failed: No space left on device -11 leaked clusters were found on the image. +10 leaked clusters were found on the image. This means waste of disk space, but no harm to data. Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
Signature · 1 line
2.13.6
PATCH 2/3 qcow2: Free allocated clusters on write error
In reply to Kevin WolfIf we managed to allocate the clusters, but then failed to write the data, there's a good chance that we'll still be able to free the clusters again in order to avoid cluster leaks (the refcounts are cached, so even if we can't write them out right now, we may be able to do so when the VM is resumed after a werror=stop/enospc pause).
--- block/qcow2.h | 1 + block/qcow2-cluster.c | 11 +++++++++++ block/qcow2.c | 2 ++ 3 files changed, 14 insertions(+)
block/qcow2.h +1 -0
@@ -614,6 +614,7 @@ uint64_t qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs, int compressed_size);int qcow2_alloc_cluster_link_l2 (BlockDriverState *bs,QCowL2Meta *m); +void qcow2_alloc_cluster_abort (BlockDriverState *bs,QCowL2Meta *m);int qcow2_cluster_discard (BlockDriverState *bs,uint64_t offset,uint64_t bytes,enum qcow2_discard_type type,bool full_discard);
block/qcow2-cluster.c +11 -0
@@ -994,6 +994,17 @@ err:return ret; } +/** +* Frees the allocated clusters because the request failed and they won't +* actually be linked. +*/ +void qcow2_alloc_cluster_abort (BlockDriverState *bs,QCowL2Meta *m) +{ +BDRVQcow2State *s = bs->opaque; +qcow2_free_clusters (bs, m->alloc_offset, m->nb_clusters << s->cluster_bits, +QCOW2_DISCARD_NEVER ); +} +/* * Returns the number of contiguous clusters that can be used for an allocating * write, but require COW to be performed (this includes yet unallocated space,
block/qcow2.c +2 -0
@@ -1777,6 +1777,8 @@ static coroutine_fn int qcow2_handle_l2meta(BlockDriverState *bs,if (ret) { goto out; } + }else { +qcow2_alloc_cluster_abort (bs, l2meta); }/* Take the request off the list of running requests */
Signature · 1 line
2.13.6
Re: [Qemu-devel] [PATCH 2/3] qcow2: Free allocated clusters on write error
Quoted 6 lines · Kevin Wolf
If we managed to allocate the clusters, but then failed to write the data, there's a good chance that we'll still be able to free the clusters again in order to avoid cluster leaks (the refcounts are cached, so even if we can't write them out right now, we may be able to do so when the VM is resumed after a werror=stop/enospc pause).
Quoted 6 lines
Signed-off-by Kevin Wolf <[email protected]>--- block/qcow2.h | 1 + block/qcow2-cluster.c | 11 +++++++++++ block/qcow2.c | 2 ++ 3 files changed, 14 insertions(+)
Hmm, I wonder if this interacts poorly with my proposed test addition for HUGE images, which is still pending review:
https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg05488.html
https://lists.gnu.org/archive/html/qemu-devel/2018-04/msg04542.html
(time for me to go testing...)
block/qcow2.c static coroutine_fn int qcow2_handle_l2meta(BlockDriverState *bs, · line 1782
Quoted 8 lines
+++
b/block/qcow2.cblock/qcow2.c +2
-0@@ -1777,6 +1777,8 @@ static coroutine_fn int qcow2_handle_l2meta(BlockDriverState *bs,if (ret) { goto out; } + }else { +qcow2_alloc_cluster_abort (bs, l2meta); }
None of our existing tests caught this? But it looks right to me.
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Eric Blake
Tested-byRe: [Qemu-devel] [PATCH 2/3] qcow2: Free allocated clusters on write error
Quoted 14 lines · Eric Blake
Quoted 6 lines · Kevin Wolf
If we managed to allocate the clusters, but then failed to write the data, there's a good chance that we'll still be able to free the clusters again in order to avoid cluster leaks (the refcounts are cached, so even if we can't write them out right now, we may be able to do so when the VM is resumed after a werror=stop/enospc pause).
Quoted 6 lines
Signed-off-by Kevin Wolf <[email protected]>---
block/qcow2.h | 1 +
block/qcow2-cluster.c| 11 +++++++++++
block/qcow2.c | 2 ++3 files changed, 14 insertions(+)
Quoted 5 lines
Hmm, I wonder if this interacts poorly with my proposed test addition for HUGE images, which is still pending review:
https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg05488.html
https://lists.gnu.org/archive/html/qemu-devel/2018-04/msg04542.html(time for me to go testing...)
Phew, my test didn't change (but I will go ahead and repost a v7 to fix the minor conflict in iotest numbering).
And that means I can also add for this series:
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
PATCH 3/3 qemu-iotests: Test qcow2 not leaking clusters on write error
In reply to Kevin WolfThis adds a test for a temporary write failure, which simulates the situation after werror=stop/enospc has stopped the VM. We shouldn't leave leaked clusters behind in such cases.
--- tests/qemu-iotests/026 | 17 +++++++++++++++++ tests/qemu-iotests/026.out | 8 ++++++++ tests/qemu-iotests/026.out.nocache | 8 ++++++++ 3 files changed, 33 insertions(+)
tests/qemu-iotests/026 +17 -0
@@ -200,6 +200,23 @@ done done done +echo +echo === Avoid cluster leaks after temporary failure === +echo + +cat > "$TEST_DIR/blkdebug.conf" <<EOF +[inject-error] +event = "write_aio" +errno = "5" +once = "on" +EOF + +# After the failed first write, do a second write so that the updated refcount +# block is actually written back +_make_test_img 64M +$QEMU_IO -c "write 0 1M" -c "write 0 1M" "$BLKDBG_TEST_IMG" | _filter_qemu_io +_check_test_img + # success, all done echo "*** done" rm -f $seq.full
tests/qemu-iotests/026.out +8 -0
@@ -675,4 +675,12 @@ write failed: No space left on device 96 leaked clusters were found on the image. This means waste of disk space, but no harm to data. + +=== Avoid cluster leaks after temporary failure === + +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 +write failed: Input/output error +wrote 1048576/1048576 bytes at offset 0 +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +No errors were found on the image. *** done
tests/qemu-iotests/026.out.nocache +8 -0
@@ -683,4 +683,12 @@ write failed: No space left on device 96 leaked clusters were found on the image. This means waste of disk space, but no harm to data. + +=== Avoid cluster leaks after temporary failure === + +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 +write failed: Input/output error +wrote 1048576/1048576 bytes at offset 0 +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +No errors were found on the image. *** done
Signature · 1 line
2.13.6
Quoted 5 lines · Kevin Wolf
Kevin Wolf (3):
qemu-iotests: Update 026.out.nocache reference output
qcow2: Free allocated clusters on write error
qemu-iotests: Test qcow2 not leaking clusters on write error
Quoted 7 lines
block/qcow2.h | 1 + block/qcow2-cluster.c | 11 +++++++++++ block/qcow2.c | 2 ++ tests/qemu-iotests/026 | 17 +++++++++++++++++ tests/qemu-iotests/026.out | 8 ++++++++ tests/qemu-iotests/026.out.nocache | 14 +++++++++++--- 6 files changed, 50 insertions(+), 3 deletions(-)
Am 28.06.2018 um 17:39 hat Kevin Wolf geschrieben:
Quoted 4 lines
Kevin Wolf (3):
qemu-iotests: Update 026.out.nocache reference output
qcow2: Free allocated clusters on write error
qemu-iotests: Test qcow2 not leaking clusters on write error
Thanks for the review, applied to the block branch.
Kevin