From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54041) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bCxNG-0002cT-AM for qemu-devel@nongnu.org; Tue, 14 Jun 2016 19:03:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bCxND-0003AW-PR for qemu-devel@nongnu.org; Tue, 14 Jun 2016 19:03:57 -0400 References: <1459967330-4573-1-git-send-email-mreitz@redhat.com> <1459967330-4573-8-git-send-email-mreitz@redhat.com> From: Eric Blake Message-ID: <57608D53.9040808@redhat.com> Date: Tue, 14 Jun 2016 17:03:47 -0600 MIME-Version: 1.0 In-Reply-To: <1459967330-4573-8-git-send-email-mreitz@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="EuduPkIg6bE1Gn1b2i63XrId62eg1SjoH" Subject: Re: [Qemu-devel] [PATCH v3 07/14] block/nbd: "address" in nbd_refresh_filename() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz , qemu-block@nongnu.org Cc: qemu-devel@nongnu.org, Kevin Wolf , Paolo Bonzini , "Daniel P . Berrange" , Markus Armbruster , Luiz Capitulino This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --EuduPkIg6bE1Gn1b2i63XrId62eg1SjoH Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 04/06/2016 12:28 PM, Max Reitz wrote: > As of a future patch, the NBD block driver will accept a SocketAddress > structure for a new "address" option. In order to support this, > nbd_refresh_filename() needs some changes. >=20 > The two TODOs introduced by this patch will be removed in the very next= > one. They exist to explain that it is currently impossible for > nbd_refresh_filename() to emit an "address.*" option (which the NBD > block driver does not handle yet). The next patch will arm these code > paths, but it will also enable handling of these options. >=20 > Signed-off-by: Max Reitz > --- > block/nbd.c | 84 ++++++++++++++++++++++++++++++++++++++++++++---------= -------- > 1 file changed, 61 insertions(+), 23 deletions(-) >=20 > diff --git a/block/nbd.c b/block/nbd.c > index 1736f68..3adf302 100644 > --- a/block/nbd.c > +++ b/block/nbd.c > @@ -441,37 +441,75 @@ static void nbd_attach_aio_context(BlockDriverSta= te *bs, > static void nbd_refresh_filename(BlockDriverState *bs, QDict *options)= > { > QDict *opts =3D qdict_new(); > - const char *path =3D qdict_get_try_str(options, "path"); > - const char *host =3D qdict_get_try_str(options, "host"); > - const char *port =3D qdict_get_try_str(options, "port"); > + bool can_generate_filename =3D true; > + const char *path =3D NULL, *host =3D NULL, *port =3D NULL; > const char *export =3D qdict_get_try_str(options, "export"); > const char *tlscreds =3D qdict_get_try_str(options, "tls-creds"); > =20 > - if (host && !port) { > - port =3D stringify(NBD_DEFAULT_PORT); > + if (qdict_get_try_str(options, "address.type")) { > + /* This path will only be possible as of a future patch; > + * TODO: Remove this note once it is */ > + > + const char *type =3D qdict_get_str(options, "address.type"); > + Oh, I'm sooooo tempted to teach the QAPI generator how to make a discriminated union have a default 'type' value (thus making the discriminator optional), so that we don't need a layer of nesting behind 'address.'. > + if (!strcmp(type, "unix")) { > + path =3D qdict_get_str(options, "address.data.path"); > + } else if (!strcmp(type, "inet")) { > + host =3D qdict_get_str(options, "address.data.host"); > + port =3D qdict_get_str(options, "address.data.port"); It's especially annoying that because SocketAddress is not flat, we have to expose the 'data.' layer of nesting, even if we could avoid the 'address.' layer. > + > + can_generate_filename =3D !qdict_haskey(options, "address.= data.to") > + && !qdict_haskey(options, "address.da= ta.ipv4") > + && !qdict_haskey(options, "address.da= ta.ipv6"); > + } else { > + can_generate_filename =3D false; > + } > + } else { > + path =3D qdict_get_try_str(options, "path"); > + host =3D qdict_get_try_str(options, "host"); > + port =3D qdict_get_try_str(options, "port"); > + > + if (host && !port) { > + port =3D stringify(NBD_DEFAULT_PORT); > + } > } Looks clean given the constraints of what you are able to use from QAPI. > + > + if (qdict_get_try_str(options, "address.type")) { > + /* This path will only be possible as of a future patch; > + * TODO: Remove this note once it is */ > + > + const QDictEntry *e; > + for (e =3D qdict_first(options); e; e =3D qdict_next(options, = e)) { > + if (!strncmp(e->key, "address.", 8)) { > + qobject_incref(e->value); > + qdict_put_obj(opts, e->key, e->value); > + } > + } This part makes me wonder if we want Dan's qdict_crumple() working first.= > } else { > - qdict_put(opts, "host", qstring_from_str(host)); > - qdict_put(opts, "port", qstring_from_str(port)); > + if (path) { > + qdict_put(opts, "path", qstring_from_str(path)); > + } else { > + qdict_put(opts, "host", qstring_from_str(host)); > + qdict_put(opts, "port", qstring_from_str(port)); > + } > } > if (export) { > qdict_put(opts, "export", qstring_from_str(export)); >=20 At this point, I'll reserve giving R-b until I've seen the whole series (it may need rebasing anyways...) --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --EuduPkIg6bE1Gn1b2i63XrId62eg1SjoH 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/ iQEcBAEBCAAGBQJXYI1TAAoJEKeha0olJ0Nq5MYH/0jKbG9mD0i1sc9JUptVKD8Q 0F0tX1atI3oWhGDDuzlV1JLPATEhGrNybuWikaXTIoZM7h6CFeaBNoonUpwG4Er0 6mmVU4HEYK0Anps6CRBM/qyraSDAme8gQhDduSm1D4dwISEtDdJZY0p8zpRDQbBl kBNRsgeaDMzEIrWtme+ByB7FTBVrLmDMxrcZOXmqFJ8484ED57IPfzdGDFrHlM8m 2h9syL8UuRBZQC/nlwStii5+JMqfXfi8I4vtGJ6OuAT1xEMmXu9jqPqW3hNCJv/4 5o2CMrx9H5cpm4Gck/XDMcsRx6DndkuzPqCOGxcsqP0tTCNqayPo3fQCmEoRuQI= =9KqY -----END PGP SIGNATURE----- --EuduPkIg6bE1Gn1b2i63XrId62eg1SjoH--