[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v3 07/14] block/nbd: "address" in nbd_refresh_fi
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [PATCH v3 07/14] block/nbd: "address" in nbd_refresh_filename() |
Date: |
Tue, 14 Jun 2016 17:03:47 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
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.
>
> 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.
>
> Signed-off-by: Max Reitz <address@hidden>
> ---
> block/nbd.c | 84
> ++++++++++++++++++++++++++++++++++++++++++++-----------------
> 1 file changed, 61 insertions(+), 23 deletions(-)
>
> 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(BlockDriverState *bs,
> static void nbd_refresh_filename(BlockDriverState *bs, QDict *options)
> {
> QDict *opts = qdict_new();
> - const char *path = qdict_get_try_str(options, "path");
> - const char *host = qdict_get_try_str(options, "host");
> - const char *port = qdict_get_try_str(options, "port");
> + bool can_generate_filename = true;
> + const char *path = NULL, *host = NULL, *port = NULL;
> const char *export = qdict_get_try_str(options, "export");
> const char *tlscreds = qdict_get_try_str(options, "tls-creds");
>
> - if (host && !port) {
> - port = 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 = 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 = qdict_get_str(options, "address.data.path");
> + } else if (!strcmp(type, "inet")) {
> + host = qdict_get_str(options, "address.data.host");
> + port = 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 = !qdict_haskey(options, "address.data.to")
> + && !qdict_haskey(options,
> "address.data.ipv4")
> + && !qdict_haskey(options,
> "address.data.ipv6");
> + } else {
> + can_generate_filename = false;
> + }
> + } else {
> + path = qdict_get_try_str(options, "path");
> + host = qdict_get_try_str(options, "host");
> + port = qdict_get_try_str(options, "port");
> +
> + if (host && !port) {
> + port = 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 = qdict_first(options); e; e = 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));
>
At this point, I'll reserve giving R-b until I've seen the whole series
(it may need rebasing anyways...)
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-block] [PATCH v3 07/14] block/nbd: "address" in nbd_refresh_filename(),
Eric Blake <=