Re: [Qemu-devel] [PATCH V6 0/8] add internal snapshot support at block device level
δΊ 2013-7-30 10:59, Wenchao Xia ει: This series brings internal snapshot support at block devices level, now we have two three methods to do block snapshot lively: 1) backing chain, 2) internal one and 3) drive-back up approach. Comparation: Advantages:Disadvantages: 1)delta data, taken fast, export, sizeperformance, delete slow. 2) taken fast, delete fast, performance, size delta data, format 3) performance, export, format taken slow, delta data, size I think in most case, saving vmstate in an standalone file is better than saving it inside qcow2, So suggest treat internal snapshot as block level methods and not encourage user to savevm in qcow2 any more. Implemention details: To avoid trouble, this serial have hide ID in create interfaces, this make sure no chaos of ID and name will be introduced by these interfaces. There is one patch may be common to Pavel's savvm transaction, patch 1/11, others are not quite related. Patch 1/11 will not set errp when no snapshot find, since patch 3/11 need to distinguish real error case. Next steps to better full VM snapshot: Improve internal snapshot's export capability. Better vmstate saving. Thanks Kevin to give advisement about how add it in qmp_transaction, oldest version comes drom Dietmar Maurer. v3: General: Rebased after Stenfan's driver-backup patch V6. Address Eric's comments: 4/9: grammar fix and better doc. 5/9: parameter name is mandatory now. grammar fix. 6/9: redesiged interface: take both id and name as optional parameter, return the deleted snapshot's info. Address Stefan's comments: 4/9: add '' around %s in message. drop code comments about vm_clock. 9/9: better doc, refined the code and add more test case. v4: Address Stefan's comments: 4/9: use error_setg_errno() to show error reason for bdrv_snapshot_create(), spell fix and better doc. 5/9: better doc. 6/9: remove spurious ';' in code, spell fix and better doc. v5: Address Kevin's comments: 3/8, 4/8, 8/8: remove the limit of numeric snapshot name. General change: 4/8: use existing type as parameter in qapi schema. v6: Address Stefan's comments: 2/8: macro STR_PRINT_CHAR was renamed as STR_OR_NULL, and moved into patch 5, since implement function in this patch do not printf snapshot id any more, as Kevin's suggestion. Address Kevin's comments: 2/8: remove device, id, name info in the error message, use error message in existing caller. A new function bdrv_snapshot_delete_by_id_or_name() is added to make the usage clear while keep logic unchanged. 3/8: remove device info in error message when name is empty. Use else if after call of bdrv_snapshot_find_by_id_and_name(). Other: 2/8: refined the comments in code for bdrv_snapshot_delete(). 3/8: in error reporting, change format from reason is: '%s' to reason is: %s. Wenchao Xia (8): 1 snapshot: new function bdrv_snapshot_find_by_id_and_name() 2 snapshot: distinguish id and name in snapshot delete 3 qmp: add internal snapshot support in qmp_transaction 4 qmp: add interface blockdev-snapshot-internal-sync 5 qmp: add interface blockdev-snapshot-delete-internal-sync 6 hmp: add interface hmp_snapshot_blkdev_internal 7 hmp: add interface hmp_snapshot_delete_blkdev_internal 8 qemu-iotests: add 056 internal snapshot for block device test case block/qcow2-snapshot.c | 55 +++--- block/qcow2.h |5 +- block/rbd.c| 21 - block/sheepdog.c |5 +- block/snapshot.c | 131 ++- blockdev.c | 190 hmp-commands.hx| 37 ++- hmp.c | 22 hmp.h |2 + include/block/block_int.h |5 +- include/block/snapshot.h | 14 +++- include/qemu-common.h |3 + qapi-schema.json | 65 +++- qemu-img.c | 11 ++- qmp-commands.hx| 104 -- savevm.c | 32 +++--- tests/qemu-iotests/056 | 259 tests/qemu-iotests/056.out |5 + tests/qemu-iotests/group |1 + 19 files changed, 913 insertions(+), 54 deletions(-) create mode 100755 tests/qemu-iotests/056 create mode 100644 tests/qemu-iotests/056.out Hello, any comments for this version? I just found 056 is taken in upstream this morning, may respin if you like. I guess it is hard to catch 1.6, so rebased on upstream and re-target to 1.7. -- Best Regards Wenchao Xia
Re: [Qemu-devel] [PATCH V6 0/8] add internal snapshot support at block device level
This series brings internal snapshot support at block devices level, now we have two three methods to do block snapshot lively: 1) backing chain, 2) internal one and 3) drive-back up approach. Comparation: Advantages:Disadvantages: 1)delta data, taken fast, export, sizeperformance, delete slow. 2) taken fast, delete fast, performance, size delta data, format 3) performance, export, format taken slow, delta data, size I think in most case, saving vmstate in an standalone file is better than saving it inside qcow2, So suggest treat internal snapshot as block level methods and not encourage user to savevm in qcow2 any more. Implemention details: To avoid trouble, this serial have hide ID in create interfaces, this make sure no chaos of ID and name will be introduced by these interfaces. There is one patch may be common to Pavel's savvm transaction, patch 1/11, others are not quite related. Patch 1/11 will not set errp when no snapshot find, since patch 3/11 need to distinguish real error case. Next steps to better full VM snapshot: Improve internal snapshot's export capability. Better vmstate saving. Thanks Kevin to give advisement about how add it in qmp_transaction, oldest version comes drom Dietmar Maurer. v3: General: Rebased after Stenfan's driver-backup patch V6. Address Eric's comments: 4/9: grammar fix and better doc. 5/9: parameter name is mandatory now. grammar fix. 6/9: redesiged interface: take both id and name as optional parameter, return the deleted snapshot's info. Address Stefan's comments: 4/9: add '' around %s in message. drop code comments about vm_clock. 9/9: better doc, refined the code and add more test case. v4: Address Stefan's comments: 4/9: use error_setg_errno() to show error reason for bdrv_snapshot_create(), spell fix and better doc. 5/9: better doc. 6/9: remove spurious ';' in code, spell fix and better doc. v5: Address Kevin's comments: 3/8, 4/8, 8/8: remove the limit of numeric snapshot name. General change: 4/8: use existing type as parameter in qapi schema. v6: Address Stefan's comments: 2/8: macro STR_PRINT_CHAR was renamed as STR_OR_NULL, and moved into patch 5, since implement function in this patch do not printf snapshot id any more, as Kevin's suggestion. Address Kevin's comments: 2/8: remove device, id, name info in the error message, use error message in existing caller. A new function bdrv_snapshot_delete_by_id_or_name() is added to make the usage clear while keep logic unchanged. 3/8: remove device info in error message when name is empty. Use else if after call of bdrv_snapshot_find_by_id_and_name(). Other: 2/8: refined the comments in code for bdrv_snapshot_delete(). 3/8: in error reporting, change format from reason is: '%s' to reason is: %s. Wenchao Xia (8): 1 snapshot: new function bdrv_snapshot_find_by_id_and_name() 2 snapshot: distinguish id and name in snapshot delete 3 qmp: add internal snapshot support in qmp_transaction 4 qmp: add interface blockdev-snapshot-internal-sync 5 qmp: add interface blockdev-snapshot-delete-internal-sync 6 hmp: add interface hmp_snapshot_blkdev_internal 7 hmp: add interface hmp_snapshot_delete_blkdev_internal 8 qemu-iotests: add 056 internal snapshot for block device test case block/qcow2-snapshot.c | 55 +++--- block/qcow2.h |5 +- block/rbd.c| 21 - block/sheepdog.c |5 +- block/snapshot.c | 131 ++- blockdev.c | 190 hmp-commands.hx| 37 ++- hmp.c | 22 hmp.h |2 + include/block/block_int.h |5 +- include/block/snapshot.h | 14 +++- include/qemu-common.h |3 + qapi-schema.json | 65 +++- qemu-img.c | 11 ++- qmp-commands.hx| 104 -- savevm.c | 32 +++--- tests/qemu-iotests/056 | 259 tests/qemu-iotests/056.out |5 + tests/qemu-iotests/group |1 + 19 files changed, 913 insertions(+), 54 deletions(-) create mode 100755 tests/qemu-iotests/056 create mode 100644 tests/qemu-iotests/056.out Hello, any comments for this version? I just found 056 is taken in upstream this morning, may respin if you like. -- Best Regards Wenchao Xia
[Qemu-devel] [PATCH V6 0/8] add internal snapshot support at block device level
This series brings internal snapshot support at block devices level, now we have two three methods to do block snapshot lively: 1) backing chain, 2) internal one and 3) drive-back up approach. Comparation: Advantages:Disadvantages: 1)delta data, taken fast, export, sizeperformance, delete slow. 2) taken fast, delete fast, performance, size delta data, format 3) performance, export, format taken slow, delta data, size I think in most case, saving vmstate in an standalone file is better than saving it inside qcow2, So suggest treat internal snapshot as block level methods and not encourage user to savevm in qcow2 any more. Implemention details: To avoid trouble, this serial have hide ID in create interfaces, this make sure no chaos of ID and name will be introduced by these interfaces. There is one patch may be common to Pavel's savvm transaction, patch 1/11, others are not quite related. Patch 1/11 will not set errp when no snapshot find, since patch 3/11 need to distinguish real error case. Next steps to better full VM snapshot: Improve internal snapshot's export capability. Better vmstate saving. Thanks Kevin to give advisement about how add it in qmp_transaction, oldest version comes drom Dietmar Maurer. v3: General: Rebased after Stenfan's driver-backup patch V6. Address Eric's comments: 4/9: grammar fix and better doc. 5/9: parameter name is mandatory now. grammar fix. 6/9: redesiged interface: take both id and name as optional parameter, return the deleted snapshot's info. Address Stefan's comments: 4/9: add '' around %s in message. drop code comments about vm_clock. 9/9: better doc, refined the code and add more test case. v4: Address Stefan's comments: 4/9: use error_setg_errno() to show error reason for bdrv_snapshot_create(), spell fix and better doc. 5/9: better doc. 6/9: remove spurious ';' in code, spell fix and better doc. v5: Address Kevin's comments: 3/8, 4/8, 8/8: remove the limit of numeric snapshot name. General change: 4/8: use existing type as parameter in qapi schema. v6: Address Stefan's comments: 2/8: macro STR_PRINT_CHAR was renamed as STR_OR_NULL, and moved into patch 5, since implement function in this patch do not printf snapshot id any more, as Kevin's suggestion. Address Kevin's comments: 2/8: remove device, id, name info in the error message, use error message in existing caller. A new function bdrv_snapshot_delete_by_id_or_name() is added to make the usage clear while keep logic unchanged. 3/8: remove device info in error message when name is empty. Use else if after call of bdrv_snapshot_find_by_id_and_name(). Other: 2/8: refined the comments in code for bdrv_snapshot_delete(). 3/8: in error reporting, change format from reason is: '%s' to reason is: %s. Wenchao Xia (8): 1 snapshot: new function bdrv_snapshot_find_by_id_and_name() 2 snapshot: distinguish id and name in snapshot delete 3 qmp: add internal snapshot support in qmp_transaction 4 qmp: add interface blockdev-snapshot-internal-sync 5 qmp: add interface blockdev-snapshot-delete-internal-sync 6 hmp: add interface hmp_snapshot_blkdev_internal 7 hmp: add interface hmp_snapshot_delete_blkdev_internal 8 qemu-iotests: add 056 internal snapshot for block device test case block/qcow2-snapshot.c | 55 +++--- block/qcow2.h |5 +- block/rbd.c| 21 - block/sheepdog.c |5 +- block/snapshot.c | 131 ++- blockdev.c | 190 hmp-commands.hx| 37 ++- hmp.c | 22 hmp.h |2 + include/block/block_int.h |5 +- include/block/snapshot.h | 14 +++- include/qemu-common.h |3 + qapi-schema.json | 65 +++- qemu-img.c | 11 ++- qmp-commands.hx| 104 -- savevm.c | 32 +++--- tests/qemu-iotests/056 | 259 tests/qemu-iotests/056.out |5 + tests/qemu-iotests/group |1 + 19 files changed, 913 insertions(+), 54 deletions(-) create mode 100755 tests/qemu-iotests/056 create mode 100644 tests/qemu-iotests/056.out
