From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:56294) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uc96v-0006lA-E9 for qemu-devel@nongnu.org; Tue, 14 May 2013 02:53:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Uc96u-0005cE-6y for qemu-devel@nongnu.org; Tue, 14 May 2013 02:53:21 -0400 Received: from e23smtp02.au.ibm.com ([202.81.31.144]:47236) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uc96t-0005c4-L8 for qemu-devel@nongnu.org; Tue, 14 May 2013 02:53:20 -0400 Received: from /spool/local by e23smtp02.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 14 May 2013 16:44:50 +1000 Received: from d23relay04.au.ibm.com (d23relay04.au.ibm.com [9.190.234.120]) by d23dlp03.au.ibm.com (Postfix) with ESMTP id D1E123578019 for ; Tue, 14 May 2013 16:53:11 +1000 (EST) Received: from d23av01.au.ibm.com (d23av01.au.ibm.com [9.190.234.96]) by d23relay04.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r4E6dCmM6684694 for ; Tue, 14 May 2013 16:39:13 +1000 Received: from d23av01.au.ibm.com (loopback [127.0.0.1]) by d23av01.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r4E6rALP002053 for ; Tue, 14 May 2013 16:53:10 +1000 Message-ID: <5191DF01.7060008@linux.vnet.ibm.com> Date: Tue, 14 May 2013 14:51:45 +0800 From: Wenchao Xia MIME-Version: 1.0 References: <1368008716-9632-1-git-send-email-xiawenc@linux.vnet.ibm.com> <20130508112104.GF3093@dhcp-200-207.str.redhat.com> In-Reply-To: <20130508112104.GF3093@dhcp-200-207.str.redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH V5 0/5] block: make qmp_transaction extendable List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: stefanha@gmail.com, pbonzini@redhat.com, qemu-devel@nongnu.org, dietmar@proxmox.com 于 2013-5-8 19:21, Kevin Wolf 写道: > Am 08.05.2013 um 12:25 hat Wenchao Xia geschrieben: >> This serial will package backing chain snapshot code as one case, to make it >> possible adding more operations later. >> >> v2: >> Address Kevin's comments: >> Use the same prototype prepare, commit, rollback model in original code, >> commit should never fail. >> >> v3: >> Address Stefan's comments: >> 3/5, 4/5: remove *action parameter since later only BlkTransactionStates* is >> needed. >> 5/5: embbed BlkTransactionStates in ExternalSnapshotStates, *opaque is >> removed, related call back function format change for external snapshot. >> Address Kevin's comments: >> removed all indention in commit message. >> 1/5: return void for prepare() function, *errp plays the role as error >> checker. >> 5/5: mark *commit callback must exist, *rollback callback can be NULL. Align >> "callback =" in "const BdrvActionOps external_snapshot_ops" to the same colum. >> Address Eric's comments: >> 1/5: better commit message. >> 5/5: better commit message and comments in code that only one of rollback() >> or commit() will be called. >> >> v4: >> 5/5: document clean() callback will always be called if it present, declare >> static for global variable "actions", use array plus .instance_size to remove >> "switch" checking code according to caller input. >> >> v5: >> better commit message and comments, switch callback function name "rollback" >> to "abort". >> >> Wenchao Xia (5): >> 1 block: package preparation code in qmp_transaction() >> 2 block: move input parsing code in qmp_transaction() >> 3 block: package committing code in qmp_transaction() >> 4 block: package rollback code in qmp_transaction() >> 5 block: make all steps in qmp_transaction() as callback >> >> blockdev.c | 266 ++++++++++++++++++++++++++++++++++++++---------------------- >> 1 files changed, 169 insertions(+), 97 deletions(-) > > Thanks, applied all to block-next for 1.6. > > Kevin > Hi, Kevin I am looking at adding internal snapshot into qmp_transaction(). Although it have been discussed before, but there are possible two ways to do it, want to know you opinion. Methods: 1) in block.c, break bdrv_snapshot_create() into bdrv_snapshot_create_prepare() and bdrv_snapshot_create_commit(), add bdrv_snapshot_create_abort(). In block/qcow2-snapshot.c, do related changing, adding qcow2_snapshot_create_abort(). Currently the abort() action can't fail, so just deallocate the cluster, but can't correct the refs, maybe put a dirty flag to let qemu-img correct it? 2) like external backing chain, on fail qemu don't delete the new created one, instead it just guarentee qemu works normal, that means nothing need to be done now. In creation, if it found a snapshot existing have same name, just fail. User should delete the existing one himself, and clean up the created one in fail. 1) deployed the commit/abort() conception in the block level, 2) deployed the conception in qemu level similar to external snapshot chain, I am not sure which is better. -- Best Regards Wenchao Xia