From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:44336) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TkWVW-00022j-Oa for qemu-devel@nongnu.org; Mon, 17 Dec 2012 04:01:03 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TkWS0-0006bL-SP for qemu-devel@nongnu.org; Mon, 17 Dec 2012 03:57:06 -0500 Received: from e23smtp05.au.ibm.com ([202.81.31.147]:33736) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TkWS0-0006b8-4H for qemu-devel@nongnu.org; Mon, 17 Dec 2012 03:53:28 -0500 Received: from /spool/local by e23smtp05.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 17 Dec 2012 18:50:37 +1000 Received: from d23relay05.au.ibm.com (d23relay05.au.ibm.com [9.190.235.152]) by d23dlp01.au.ibm.com (Postfix) with ESMTP id CC6782CE8050 for ; Mon, 17 Dec 2012 19:53:18 +1100 (EST) Received: from d23av04.au.ibm.com (d23av04.au.ibm.com [9.190.235.139]) by d23relay05.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id qBH8g65O42008768 for ; Mon, 17 Dec 2012 19:42:07 +1100 Received: from d23av04.au.ibm.com (loopback [127.0.0.1]) by d23av04.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id qBH8rHRY013828 for ; Mon, 17 Dec 2012 19:53:17 +1100 Message-ID: <50CEDD39.6010803@linux.vnet.ibm.com> Date: Mon, 17 Dec 2012 16:52:09 +0800 From: Wenchao Xia MIME-Version: 1.0 References: <1355725509-5429-1-git-send-email-xiawenc@linux.vnet.ibm.com> <1355725509-5429-5-git-send-email-xiawenc@linux.vnet.ibm.com> <24E144B8C0207547AD09C467A8259F75578B461B@lisa.maurer-it.com> <50CECBFA.6090000@linux.vnet.ibm.com> <24E144B8C0207547AD09C467A8259F75578B46F4@lisa.maurer-it.com> In-Reply-To: <24E144B8C0207547AD09C467A8259F75578B46F4@lisa.maurer-it.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 4/6] snapshot: implemention of common API to take snapshots List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Dietmar Maurer Cc: "kwolf@redhat.com" , "aliguori@us.ibm.com" , "stefanha@gmail.com" , "qemu-devel@nongnu.org" , "blauwirbel@gmail.com" , "pbonzini@redhat.com" 于 2012-12-17 15:52, Dietmar Maurer 写道: >> 于 2012-12-17 14:36, Dietmar Maurer 写道: >>>> This patch is the implemention of transaction based snapshot internal >> API. >>> >>> What exactly is the advantage of using qmp transactions (as opposed to the >> pve snapshot patches)? >> Which pve snapshot patches you referring? > > I refer to: > > https://git.proxmox.com/?p=pve-qemu-kvm.git;a=blob;f=debian/patches/internal-snapshot-async.patch;h=6c86de3a6160c58d77baa41a7774c4a80e63639e;hb=HEAD > >> In group case a check should be >> done for all snapshot first, and I need to support qmp transaction interface >> which is already there, so I build an internal transaction layer below to make >> the interface unified and relative easier to add in qmp transaction. > > IMHO qmp transactions are a bit clumsy and inflexible. And as you can see with my patches, > there is no real need for them. > There are two layers, qmp transaction and BlkTransactionStates, user can use qmp_transaction interface or simply qmp_snapshot_create interface, which are both consumer of BlkTransactionStates. Compared to the pre patch, I think what different is for group use case and integration with external snapshot code. mainly reason are: 1 in group case, it need a queue with element records each one's states, so it can revert on fail, that date structure is BlkTransactionStatesList. 2 in group case check should be done first, so separate check, execution, cancel functions should be there. And there are two type of snapshot there with two type of operation(create and delete), so implement code will be fragment, need to packaged them in unified way, then upper level function such as savevm can use it gracefully. BlkTransactionStates can hide the implement details. 3 group internal and external snapshot have same logic, they can be merged with callback functions, which is embbed in BlkTransactionStates, otherwise there will be "if" in many place makes code ugly which way I have tried and falled back from. some other reason not related to pre patch: 4 qmp_transaction interface is already there for external snapshot, so I think internal snapshot should also support it. BlkTransactionStates make it easier. 5 BlkTransactionStatesList use qemu internal data types instead data exposed in BlockDevAction, this allows other code inside qemu pass in data not observable to user and hide some functions to user. >>> Seems this makes it impossible to use external commands to make >> snapshots? >>> >> what command? can you tip more about the case? > > For example, nexenta storage provides an API to create snapshots. We want to use > that. Another example would be to use lvcreate to create lvm snapshots. > I am not familar with those tools, Patch 5 and 6 provides hmp and qmp API, hope you can enlight more what is missing if it can't work with external tool. > - Dietmar > -- Best Regards Wenchao Xia