From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:60647) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1US80w-0003pd-R4 for qemu-devel@nongnu.org; Tue, 16 Apr 2013 11:41:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1US80t-0005sw-8Y for qemu-devel@nongnu.org; Tue, 16 Apr 2013 11:41:46 -0400 Received: from mx1.redhat.com ([209.132.183.28]:1242) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1US80t-0005s9-1A for qemu-devel@nongnu.org; Tue, 16 Apr 2013 11:41:43 -0400 Message-ID: <516D7112.9000503@redhat.com> Date: Tue, 16 Apr 2013 09:41:06 -0600 From: Eric Blake MIME-Version: 1.0 References: <1365851501-3037-1-git-send-email-xiawenc@linux.vnet.ibm.com> <1365851501-3037-6-git-send-email-xiawenc@linux.vnet.ibm.com> In-Reply-To: <1365851501-3037-6-git-send-email-xiawenc@linux.vnet.ibm.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="----enig2GNOBTWAKEPPWIIDLPBXJ" Subject: Re: [Qemu-devel] [PATCH V2 5/5] block: make all steps in qmp_transaction() as callback List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wenchao Xia Cc: kwolf@redhat.com, pbonzini@redhat.com, stefanha@gmail.com, qemu-devel@nongnu.org, dietmar@proxmox.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) ------enig2GNOBTWAKEPPWIIDLPBXJ Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 04/13/2013 05:11 AM, Wenchao Xia wrote: > Now qmp_transaction() can be extended with other operation, > external snapshot or backing chain creation, is just one case it. This read a bit awkwardly. Might I suggest: block: use callbacks in qmp_transaction() Make it easier to add other operations to qmp_transaction() by using callbacks, with external snapshots serving as an example implementation of the callbacks. >=20 > Signed-off-by: Wenchao Xia > --- > blockdev.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++-= ------- > 1 files changed, 58 insertions(+), 10 deletions(-) >=20 > diff --git a/blockdev.c b/blockdev.c > index 3e69569..9f0acfb 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -779,14 +779,35 @@ void qmp_blockdev_snapshot_sync(const char *devic= e, const char *snapshot_file, > =20 > =20 > /* New and old BlockDriverState structs for group snapshots */ > + > +/* Only in prepare() it may fail, so roll back just need to take care > + what is done in prepare(). */ Better might be: /* Only prepare() may fail. In a single transaction, only one of commit() or rollback() will be called. */ > +typedef struct BdrvActionOps { > + /* Prepare the work, must NOT be NULL. */ > + int (*prepare)(BlockdevAction *action, void **p_opaque, Error **er= rp); Based on the comments on 1/5, should prepare have a void signature and just let errp serve to indicate failure? --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org ------enig2GNOBTWAKEPPWIIDLPBXJ Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJRbXESAAoJEKeha0olJ0Nq7o8H/2I6Wai7oN7RRD1ItS9ijKjv 0Kq5R+kjP4RvoFmx5ikCEzLwHsY/fweKLTnwtq6hYw/qD7BH8d7CTVwwecKWg61U a9H2g3jQ5oz4Ximy4hoP42VWp8qN/9KwG5HS+A/U+vlYQByswttpPjnYsJqX/YwP DUzlOcHXAPWwy5S22a02rf+nxlu0YTxmlS/DDVffxxri5LMqsLbm67vp4w7NndYz Um6NjclLyt85lF/iVeA0ah+qEYUiNRStYQpeqjMDyyJRuTOXgakRlCkoMh3yRjsM LlPUVoZOnYboCOLuhQnGk4+cGGwooVxsXVa7dycVZwsOYQZLQPjZjC9Hdw7DE2U= =j4V8 -----END PGP SIGNATURE----- ------enig2GNOBTWAKEPPWIIDLPBXJ--