From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:46004) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UZAjx-0000Mf-BA for qemu-devel@nongnu.org; Sun, 05 May 2013 22:01:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UZAjw-0000jA-Co for qemu-devel@nongnu.org; Sun, 05 May 2013 22:01:21 -0400 Received: from e28smtp07.in.ibm.com ([122.248.162.7]:55617) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UZAjr-0000il-LE for qemu-devel@nongnu.org; Sun, 05 May 2013 22:01:20 -0400 Received: from /spool/local by e28smtp07.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 6 May 2013 07:25:39 +0530 Received: from d28relay05.in.ibm.com (d28relay05.in.ibm.com [9.184.220.62]) by d28dlp03.in.ibm.com (Postfix) with ESMTP id E880D1258023 for ; Mon, 6 May 2013 07:32:42 +0530 (IST) Received: from d28av04.in.ibm.com (d28av04.in.ibm.com [9.184.220.66]) by d28relay05.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r4620qgh5243348 for ; Mon, 6 May 2013 07:30:53 +0530 Received: from d28av04.in.ibm.com (loopback [127.0.0.1]) by d28av04.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r4620ucY017790 for ; Mon, 6 May 2013 12:00:56 +1000 Message-ID: <51870ECD.6090304@linux.vnet.ibm.com> Date: Mon, 06 May 2013 10:00:45 +0800 From: Wenchao Xia MIME-Version: 1.0 References: <1367461606-7554-1-git-send-email-xiawenc@linux.vnet.ibm.com> <1367461606-7554-6-git-send-email-xiawenc@linux.vnet.ibm.com> <5183CDCE.70005@redhat.com> In-Reply-To: <5183CDCE.70005@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH V4 5/5] block: make all steps in qmp_transaction() as callback List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: kwolf@redhat.com, pbonzini@redhat.com, stefanha@gmail.com, qemu-devel@nongnu.org, dietmar@proxmox.com 于 2013-5-3 22:46, Eric Blake 写道: > On 05/01/2013 08:26 PM, Wenchao Xia wrote: >> Make it easier to add other operations to qmp_transaction() by using >> callbacks, with external snapshots serving as an example implementation >> of the callbacks. >> >> Signed-off-by: Wenchao Xia >> --- >> blockdev.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++------------- >> 1 files changed, 71 insertions(+), 21 deletions(-) >> > >> +/* >> + * This structure must be arranged as first member in parent type, assuming > > To me, "parent type" seems like the wrong relationship - in C++, the > parent type is the smaller type, and the child type is the larger type > that includes the parent type as its first member. That is, > BlkTransationStates IS the parent type, and the comment would read > better as "first member in child type". > >> + * that compiler will also arrange it to the same address with parent instance. >> + * Later it will be used in free(). >> + */ >> +struct BlkTransactionStates { >> + BlockdevAction *action; >> + const BdrvActionOps *ops; >> + QSIMPLEQ_ENTRY(BlkTransactionStates) entry; >> +}; >> > >> >> /* Now we are going to do the actual pivot. Everything up to this point >> * is reversible, but we are committed at this point */ > > Is this comment still correct, now that things are extensible, or should > you s/pivot/commit/? > > If you do respin, I agree that s/rollback/abort/ might be a nicer name > for that particular callback. But I'm also quite okay with using the > patch as-is without a respin (while I pointed out two possible comment > wording changes, they don't make or break the patch for me). > > Reviewed-by: Eric Blake > I'll respin to fix comments and use "abort" as v5, thanks for your reviewing! -- Best Regards Wenchao Xia