From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54207) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZWVKU-00020A-Lh for qemu-devel@nongnu.org; Mon, 31 Aug 2015 16:05:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZWVKT-0003Pt-3I for qemu-devel@nongnu.org; Mon, 31 Aug 2015 16:05:22 -0400 References: <457103c2204e849aea3b83ffd78fad049d8d923d.1441014844.git.berto@igalia.com> <55E4B0CC.6070906@redhat.com> From: Eric Blake Message-ID: <55E4B374.70800@redhat.com> Date: Mon, 31 Aug 2015 14:05:08 -0600 MIME-Version: 1.0 In-Reply-To: <55E4B0CC.6070906@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="UjNwvjxLcRhc7r3BXwaIf9qUC653mKu2W" Subject: Re: [Qemu-devel] [PATCH 1/1] block: Allow passing BlockdevOptions to blockdev-snapshot-sync List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz , Alberto Garcia , qemu-devel@nongnu.org Cc: Kevin Wolf , qemu-block@nongnu.org, Stefan Hajnoczi This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --UjNwvjxLcRhc7r3BXwaIf9qUC653mKu2W Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 08/31/2015 01:53 PM, Max Reitz wrote: > Design question: Would it make sense to instead add a "reference" mode > to blockdev-snapshot-sync where you can specify a BDS's node-name > instead of snapshot-file to use an existing BDS as the new top layer, > ideally an empty one? Indeed - then blockdev-add can be used to create an unattached BDS (with all appropriate options), and blockdev-snapshot-sync would then attach that BDS as the snapshot-file that wraps an existing BDS (without needing to worry about options). >=20 > What we'd then need is a QMP command for creating images. But as far as= > I know we'll need that anyway sooner or later... Can't blockdev-add already be used for that (at least, for supported file types)? If not, what would it take to get it there? >=20 >=20 > Comments on the patch itself below. >=20 >=20 > With has_format =3D=3D false, format will be set to "qcow2" by default.= So, > if the user does not specify the format explicitly, the "driver" field > has to be set to "qcow2". >=20 > I'd rather make specifying @format and @options exclusive, and if > @options has been specified, its "driver" field should override the > "format" default. >=20 >> +++ b/qapi/block-core.json >> @@ -697,11 +697,18 @@ >> # >> # @mode: #optional whether and how QEMU should create a new image, de= fault is >> # 'absolute-paths'. >> +# >> +# @options: #optional options for the new device, with the following >> +# restrictions for the fields: 'driver' must match the valu= e >> +# of @format, >=20 > As said above, I'd rather make specifying both @options and @format > exclusive. >=20 > Maybe there is even some QAPI magic to enforce that (and for > 'node-name', too), I don't know... Not that I know of at the moment, but not to say we can't add some. The closest we can get is with a flat union, but that requires a non-optional discriminator field. Maybe we can tweak qapi to make the discriminator optional (with a default value). Thankfully, it sounds like Markus' work on introspection would at least let management apps learn about a new 'options' argument. >> 'node-name' and @snapshot-node-name cannot be= >> +# specified at the same time, and 'file' can only contain a= n >> +# empty string (Since 2.5) I really don't like the idea of requiring the user to pass in an empty string. Can we make the field optional instead, and require it to be omitted in the context where it would otherwise need to be empty, while still requiring it to be present in existing clients that weren't prepared for it to be optional? It also feels a bit ad hoc that you describe that driver/format must be identical, but that other fields must be mutually exclusive or the empty string; consistency would argue that since you already handle duplication of driver/format by requiring them to be the same, that you should also allow duplication and validation of identical other fields that overlap. (But even better would be finding a way to not allow overlapping fields to appear in the first place). --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --UjNwvjxLcRhc7r3BXwaIf9qUC653mKu2W Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJV5LN0AAoJEKeha0olJ0Nqg9MH/2VyuKovdRvBp81zVJzyxZSU XX4hlS60WwIqcWSqqAy/wAEf+tQkwh1Wif4WpT0br3FZIF90w7iqAskTW51kYKGE LwG5Mt8vyIiFzQcfoLn1DvdayZLBYCiwWQxLYVkvTL2DLFqM+V9d2bBU6kOTlPTp 2scaNyaOwh7SOA+ZiOiDpKwT3M6mvitzVNzNVec4rU2Z6Pzo0XaP37YwS3ECuz/9 reEPCKJNSa+JzndigVsvsMKyAh8BlUXt0ZzoztG0ZG8ENKsuf8QZawQzT9fSA8wE hHhx7hs8tdeUJ2Mlfeh+3n/ptoUKPiXMuJHxfr9QRosYeFcqu536WiJoYP1Rf/w= =XFfR -----END PGP SIGNATURE----- --UjNwvjxLcRhc7r3BXwaIf9qUC653mKu2W--