Re: [Qemu-devel] [PATCH V3 0/7] qcow2: rollback the modification on fail in snapshot creation

[Stefan Hajnoczi]

On Mon, Sep 09, 2013 at 10:57:55AM +0800, Wenchao Xia wrote:
> V2:
>   1: all fail case will goto fail section.
>   2: add the goto code.
> v3:
>   Address Stefan's comments:
>   2: don't goto fail after allocation failure.
>   3: use sn->l1size correctly in qcow2_free_cluster().
>   4-7: add test case to verify the error paths.
>   Other:
>   1: new patch fix a existing bug, which will be exposed in error path test.
> 
> 
> Wenchao Xia (7):
>   1 qcow2: restore nb_snapshots when fail in snapshot creation
>   2 qcow2: free allocated cluster on fail in qcow2_write_snapshots()
>   3 qcow2: cancel the modification on fail in qcow2_snapshot_create()
>   4 blkdebug: add debug events for snapshot
>   5 qcow2: use debug events for snapshot
>   6 qcow2: print message for error path in snapshot creation
>   7 qemu-iotests: add test for qcow2 snapshot
> 
>  block/blkdebug.c           |    4 +
>  block/qcow2-snapshot.c     |   80 ++++++++++++++--
>  include/block/block.h      |    4 +
>  tests/qemu-iotests/063     |  229 
> ++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/063.out |   37 +++++++
>  tests/qemu-iotests/group   |    1 +
>  6 files changed, 348 insertions(+), 7 deletions(-)
>  create mode 100755 tests/qemu-iotests/063
>  create mode 100644 tests/qemu-iotests/063.out

Makes sense but keep in mind that it's better to leak clusters than to
corrupt the image further.  We have to be very careful in these error
paths, especially when a big operation could have failed partway
through.  I left comments where I thought the patches need to be
changed.