From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:43044) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TGb1Y-0004CK-DW for qemu-devel@nongnu.org; Tue, 25 Sep 2012 15:42:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TGb1X-0005Rg-1c for qemu-devel@nongnu.org; Tue, 25 Sep 2012 15:42:28 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38207) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TGb1W-0005Rc-PG for qemu-devel@nongnu.org; Tue, 25 Sep 2012 15:42:26 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q8PJgPhl026238 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 25 Sep 2012 15:42:26 -0400 Message-ID: <5062091A.8080600@redhat.com> Date: Tue, 25 Sep 2012 13:42:18 -0600 From: Eric Blake MIME-Version: 1.0 References: <75bbfc268498ae3733d7ffb896ab0852527d5b11.1348589526.git.jcody@redhat.com> In-Reply-To: <75bbfc268498ae3733d7ffb896ab0852527d5b11.1348589526.git.jcody@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="------------enig501605F1333EB4F1BFB27C98" Subject: Re: [Qemu-devel] [PATCH v2 5/7] QAPI: add command for live block commit, 'block-commit' List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jeff Cody Cc: kwolf@redhat.com, pbonzini@redhat.com, qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig501605F1333EB4F1BFB27C98 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 09/25/2012 10:29 AM, Jeff Cody wrote: > The command for live block commit is added, which has the following > arguments: >=20 > device: the block device to perform the commit on (mandatory) > base: the base image to commit into; optional (if not specified, > it is the underlying original image) > top: the top image of the commit - all data from inside top down > to base will be committed into base. optional (if not specified= , > it is one below the active image) - see note below This says that for 'base' <- 'mid' <- 'active', omitting 'top' is the same as specifying 'top':'mid'. > speed: maximum speed, in bytes/sec >=20 > note: eventually this will support merging down the active layer, > but that code is not yet complete. If the active layer is passed= > in currently as top, or top is left to the default, then an error= > will be returned. This says that for 'base' <- 'mid' <- 'active', omitting 'top' is an error (because it would be the same as an explicit 'top:'active'). Let's check the code... > + if (top && has_top) { > + /* if we want to allow the active layer, > + * use 'bdrv_find_image()' here */ > + top_bs =3D bdrv_find_backing_image(bs, top); > + } else { > + /* default is one below the active layer, unless that is > + * the base */ > + top_bs =3D bs->backing_hd; Aha - the former is correct as coded (you default to 'top':'mid' in my example), so the 'note' in your commit message needs editing. On the other hand, when we ever DO get around to adding live commit, which is the saner default? That is, am I more likely to want to do live commit from 'active', or more likely to do commit of the layer below 'active'? If live commit is the more desirable case, then that argues that THIS commit should always error out if !has_top, and that the future patch will default top_bs =3D bs, and the rest of your commit message and documentation would need tweaking accordingly. I don't have a preference either way (libvirt can arrange to always pass the top argument, and thus avoid the confusion when top is omitted), but if anyone else cares, now is the time to get the API right before we are locked in to it. > +++ b/qapi-schema.json > @@ -1468,6 +1468,41 @@ > 'returns': 'str' } > =20 > ## > +# @block-commit > +# > +# Live commit of data from overlay image nodes into backing nodes - i.= e., > +# writes data between 'top' and 'base' into 'base'. > +# > +# @device: the name of the device > +# > +# @base: #optional The file name of the backing image to write data = into. > +# If not specified, this is the deepest backing ima= ge > +# > +# @top: #optional The file name of the backing image within the ima= ge chain, > +# which contains the topmost data to be committed d= own. > +# If not specified, this is one layer below the act= ive > +# layer (i.e. active->backing_hd). > +# > +# If top =3D=3D base, that is an error. > +# This documentation of @top matches the first paragraph of your commit message. > +# > +# @speed: #optional the maximum speed, in bytes per second > +# > +# Returns: Nothing on success > +# If commit or stream is already active on this device, Devic= eInUse > +# If @device does not exist, DeviceNotFound > +# If image commit is not supported by this device, NotSupport= ed > +# If @base does not exist, a generic error is returned > +# If @top does not exist, a generic error is returned These two lines could be merged, or even made more generic (for example, based on the rest of this thread, you will also be erroring out if base and top exist, but appear as swapped arguments): If @base or @top is invalid, a generic error is returned --=20 Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --------------enig501605F1333EB4F1BFB27C98 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.12 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://www.enigmail.net/ iQEcBAEBCAAGBQJQYgkbAAoJEKeha0olJ0NqIX0IAJeQBAt9x3eRes+7599r6075 Qw1HId8xsZDgWwaR+Wi+NfnMaP2Rd5w049/j9tgMoZh4ExPMBDIRDsWk/giqzuNK f/cUoxp3PcbvculQqAh0Gwxs6emCY3tjbuu9Xlzxft6YeoLpmzVe36wUR/CANb69 mlkk2N/V+f75isD0oQBkXKCqEG/YRS2kPYLVctJ33sCGHV61Ko6fB4K9zVDjvmsZ ZBSq2fngZQgN8ZjdBMdNDLEeKbcb4rD3C7jKz7/76123OD5C2DEv46jsNFKLU9u4 AoFIRUSZ5fdQvfCR2qIiYUNj5STnc6O+BsCBybQ/FKplB6DH9nNx2ZkuPQrnx74= =EGFL -----END PGP SIGNATURE----- --------------enig501605F1333EB4F1BFB27C98--