Re: [Qemu-devel] [PATCH V3 0/7] qcow2: rollback the modification on fail in snapshot creation
于 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.
Re: [Qemu-devel] [PATCH V3 0/7] qcow2: rollback the modification on fail in snapshot creation
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.
Re: [Qemu-devel] [PATCH V3 0/7] qcow2: rollback the modification on fail in snapshot creation
于 2013/9/9 10:57, Wenchao Xia 写道: 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 Hello, any comments for it?
