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

[Wenchao Xia <xiawenc@linux.vnet.ibm.com>]

于 2013/10/2 20:28, 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.
>
Thanks for review. Since Max have already added some error path code in 
upstream,
I rebased and skip cluster freeing when fail in image header update.