From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:50857) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UG3RM-0001k0-Qm for qemu-devel@nongnu.org; Thu, 14 Mar 2013 04:23:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UG3RI-0004mQ-Je for qemu-devel@nongnu.org; Thu, 14 Mar 2013 04:23:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:51190) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UG3RI-0004mI-AP for qemu-devel@nongnu.org; Thu, 14 Mar 2013 04:23:04 -0400 Date: Thu, 14 Mar 2013 09:22:56 +0100 From: Kevin Wolf Message-ID: <20130314082256.GA2485@dhcp-200-207.str.redhat.com> References: <50EE33B2.4040504@linux.vnet.ibm.com> <20130110124109.GD30946@stefanha-thinkpad.redhat.com> <50EFAFA4.1030705@linux.vnet.ibm.com> <20130111091253.GA31400@stefanha-thinkpad.muc.redhat.com> <50F373DE.4060709@linux.vnet.ibm.com> <20130114100604.GH11260@stefanha-thinkpad.redhat.com> <50F4FF3C.9000706@linux.vnet.ibm.com> <513EE7B1.6060501@linux.vnet.ibm.com> <20130313101838.GD2309@dhcp-200-207.str.redhat.com> <51415B52.1040409@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <51415B52.1040409@linux.vnet.ibm.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH V2 07/10] snapshot: qmp use new internal API for external snapshot transaction List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wenchao Xia Cc: aliguori@us.ibm.com, quintela@redhat.com, Stefan Hajnoczi , qemu-devel@nongnu.org, lcapitulino@redhat.com, pbonzini@redhat.com, dietmar@proxmox.com Am 14.03.2013 um 06:08 hat Wenchao Xia geschrieben: > =E4=BA=8E 2013-3-13 18:18, Kevin Wolf =E5=86=99=E9=81=93: > >Am 12.03.2013 um 09:30 hat Wenchao Xia geschrieben: > >> I redesigned the structure, Following is the fake code: > >> > >>typedef struct BdrvActionOps { > >> /* check the request's validation, allocate p_opaque if needed *= / > >> int (*check)(BlockdevAction *action, void **p_opaque, Error **er= rp); > >> /* take the action */ > >> int (*submit)(BlockdevAction *action, void *opaque, Error **errp= ); > >> /* update emulator */ > >> int (*commit)(BlockdevAction *action, void *opaque, Error **errp= ); > >> /* cancel the action */ > >> int (*rollback)(BlockdevAction *action, void *opaque, Error **er= rp); > >>} BdrvActionOps; > > > >Why do you need the split of prepare into check/submit? > > > >If you have prepare/commit/abort, everybody will recognise this as the > >standard transaction pattern because this is just how it's done. > >Deviating from it needs a good justification in my opinion. > > > >Kevin > > >=20 > My thought is rejecting the request in *check if parameter invalid > before take any action, while submit do the real action, to reduce > the chance to of rolling back when some request not valid in the batch. Okay, so it's not strictly needed, but an optimisation of the error case? Does it work well when the transaction includes an operation that depends on the previous one, like create a snapshot and then do something with this snapshot? Anyway, even if we think it works and is worth the effort to optimise such error cases, please use names that are consistent with the transactions used for reopening: (check/)prepare/commit/abort. Kevin