From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35057) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V6u0s-0002wK-8q for qemu-devel@nongnu.org; Tue, 06 Aug 2013 23:02:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V6u0i-0002FG-Nw for qemu-devel@nongnu.org; Tue, 06 Aug 2013 23:02:14 -0400 Received: from e23smtp05.au.ibm.com ([202.81.31.147]:60074) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V6u0i-0002Ex-4A for qemu-devel@nongnu.org; Tue, 06 Aug 2013 23:02:04 -0400 Received: from /spool/local by e23smtp05.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 7 Aug 2013 12:55:06 +1000 Received: from d23relay05.au.ibm.com (d23relay05.au.ibm.com [9.190.235.152]) by d23dlp01.au.ibm.com (Postfix) with ESMTP id D97492CE804D for ; Wed, 7 Aug 2013 13:01:57 +1000 (EST) Received: from d23av01.au.ibm.com (d23av01.au.ibm.com [9.190.234.96]) by d23relay05.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r772k7YD721370 for ; Wed, 7 Aug 2013 12:46:08 +1000 Received: from d23av01.au.ibm.com (localhost [127.0.0.1]) by d23av01.au.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id r7731uLk001973 for ; Wed, 7 Aug 2013 13:01:56 +1000 Message-ID: <5201B898.2080704@linux.vnet.ibm.com> Date: Wed, 07 Aug 2013 11:01:44 +0800 From: Wenchao Xia MIME-Version: 1.0 References: <1374327070-11400-1-git-send-email-xiawenc@linux.vnet.ibm.com> <51F72C1B.6080107@linux.vnet.ibm.com> In-Reply-To: <51F72C1B.6080107@linux.vnet.ibm.com> Content-Type: text/plain; charset=GB2312 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH V6 0/8] add internal snapshot support at block device level List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wenchao Xia Cc: kwolf@redhat.com, phrdina@redhat.com, famz@redhat.com, armbru@redhat.com, qemu-devel@nongnu.org, lcapitulino@redhat.com, stefanha@redhat.com, pbonzini@redhat.com, dietmar@proxmox.com ÓÚ 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, size performance, 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