From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:56609) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1USf5w-0007B4-AI for qemu-devel@nongnu.org; Wed, 17 Apr 2013 23:01:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1USf5v-00068M-5D for qemu-devel@nongnu.org; Wed, 17 Apr 2013 23:01:08 -0400 Received: from e28smtp01.in.ibm.com ([122.248.162.1]:48977) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1USf5u-00065V-KV for qemu-devel@nongnu.org; Wed, 17 Apr 2013 23:01:07 -0400 Received: from /spool/local by e28smtp01.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 18 Apr 2013 08:26:20 +0530 Received: from d28relay02.in.ibm.com (d28relay02.in.ibm.com [9.184.220.59]) by d28dlp02.in.ibm.com (Postfix) with ESMTP id BD9F43940053 for ; Thu, 18 Apr 2013 08:30:59 +0530 (IST) Received: from d28av01.in.ibm.com (d28av01.in.ibm.com [9.184.220.63]) by d28relay02.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r3I30sbK66977948 for ; Thu, 18 Apr 2013 08:30:55 +0530 Received: from d28av01.in.ibm.com (loopback [127.0.0.1]) by d28av01.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r3I30vCQ000740 for ; Thu, 18 Apr 2013 03:00:57 GMT Message-ID: <516F61B4.6020100@linux.vnet.ibm.com> Date: Thu, 18 Apr 2013 11:00:04 +0800 From: Wenchao Xia MIME-Version: 1.0 References: <1364810491-21404-1-git-send-email-xiawenc@linux.vnet.ibm.com> <1364810491-21404-3-git-send-email-xiawenc@linux.vnet.ibm.com> <20130417144238.GA9390@stefanha-thinkpad.muc.redhat.com> In-Reply-To: <20130417144238.GA9390@stefanha-thinkpad.muc.redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 2/3] block: adjust qmp_transaction to be extendable List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: kwolf@redhat.com, pbonzini@redhat.com, qemu-devel@nongnu.org, dietmar@proxmox.com 于 2013-4-17 22:42, Stefan Hajnoczi 写道: > On Mon, Apr 01, 2013 at 06:01:30PM +0800, Wenchao Xia wrote: >> /* New and old BlockDriverState structs for group snapshots */ >> -typedef struct BlkTransactionStates { >> +typedef struct BdrvActionOps { >> + int (*commit)(BlockdevAction *action, void **p_opaque, Error **errp); >> + void (*rollback)(BlockdevAction *action, void *opaque); >> + void (*clean)(BlockdevAction *action, void *opaque); > > Please document these functions. > OK. >> +const BdrvActionOps external_snapshot_ops = { >> + .commit = external_snapshot_commit, >> + .rollback = external_snapshot_rollback, >> + .clean = external_snapshot_clean, >> +}; >> + >> +typedef struct BlkTransactionStates { >> + BlockdevAction *action; >> + void *opaque; >> + const BdrvActionOps *ops; >> QSIMPLEQ_ENTRY(BlkTransactionStates) entry; >> } BlkTransactionStates; > > The relationship between BlkTransactionStates and ExternalSnapshotState > can be simplified: > > typedef struct { > int (*commit)(BlkTransactionState *state, Error **errp); > void (*rollback)(BlkTransactionState *state); > void (*clean)(BlkTransactionState *state); > size_t instance_size; > } BdrvActionOps; > > typedef struct BlkTransactionState { > BlockDevAction *action; > const BdrvActionOps *ops; > QSIMPLEQ_ENTRY(BlkTransactionState) entry; > } BlkTransactionState; > > typedef struct { > BlkTransactionState common; > BlockDriverState *old_bs; > BlockDriverState *new_bs; > } ExternalSnapshotState; > > const BdrvActionOps external_snapshot_ops = { > .commit = external_snapshot_commit, > .rollback = external_snapshot_rollback, > .clean = external_snapshot_clean, > .instance_size = sizeof(ExternalSnapshotState); > }; > > This eliminates the opaque pointer and g_free(state) can be handled by > qmp_transaction(). This way action types no longer need to free opaque. > Where should be ExternalSnapshotState* placed, insert it into BlkTransactionState, or BdrvActionOps? -- Best Regards Wenchao Xia