On 04/06/2016 12:28 PM, Max Reitz wrote: > Add a new option "address" to the NBD block driver which accepts a > SocketAddress. > > "path", "host" and "port" are still supported as legacy options and are > mapped to their corresponding SocketAddress representation.
The back-compat work is pretty slick. > > Signed-off-by: Max Reitz <mre...@redhat.com> > --- > block/nbd.c | 97 > ++++++++++++++++++++++++++----------------- > tests/qemu-iotests/051.out | 4 +- > tests/qemu-iotests/051.pc.out | 4 +- > 3 files changed, 64 insertions(+), 41 deletions(-) > > diff --git a/block/nbd.c b/block/nbd.c > index 3adf302..9ae66ba 100644 > --- a/block/nbd.c > +++ b/block/nbd.c > @@ -32,6 +32,8 @@ > #include "qemu/uri.h" > #include "block/block_int.h" > #include "qemu/module.h" > +#include "qapi-visit.h" > +#include "qapi/qmp-input-visitor.h" > #include "qapi/qmp/qdict.h" > #include "qapi/qmp/qjson.h" > #include "qapi/qmp/qint.h" > @@ -128,7 +130,9 @@ static bool nbd_has_filename_options_conflict(QDict > *options, Error **errp) > if (!strcmp(e->key, "host") > || !strcmp(e->key, "port") > || !strcmp(e->key, "path") > - || !strcmp(e->key, "export")) > + || !strcmp(e->key, "export") > + || !strcmp(e->key, "address") > + || !strncmp(e->key, "address.", 8)) May need a tweak if you break before '||' > { > error_setg(errp, "Option '%s' cannot be used with a file name", > e->key); > @@ -202,46 +206,66 @@ out: > g_free(file); > } > > +static bool nbd_process_legacy_socket_options(QDict *options, Error **errp) > +{ > + if (qdict_haskey(options, "path") && qdict_haskey(options, "host")) { > + error_setg(errp, "path and host may not be used at the same time"); > + return false; > + } else if (qdict_haskey(options, "path")) { > + if (qdict_haskey(options, "port")) { > + error_setg(errp, "port may not be used without host"); > + return false; > + } > + > + qdict_put(options, "address.type", qstring_from_str("unix")); > + qdict_change_key(options, "path", "address.data.path"); > + } else if (qdict_haskey(options, "host")) { > + qdict_put(options, "address.type", qstring_from_str("inet")); > + qdict_change_key(options, "host", "address.data.host"); > + if (!qdict_change_key(options, "port", "address.data.port")) { > + qdict_put(options, "address.data.port", > + qstring_from_str(stringify(NBD_DEFAULT_PORT))); > + } The rewrite from old to modern is rather nice. I wonder if we could then use a generated qapi_visit_SocketAddress instead of manually handling all the complication of SocketAddress proper. > + } > + > + return true; > +} > + > static SocketAddress *nbd_config(BDRVNBDState *s, QDict *options, char > **export, > Error **errp) > { > - SocketAddress *saddr; > + SocketAddress *saddr = NULL; > + QDict *addr = NULL; > + QObject *crumpled_addr; > + QmpInputVisitor *iv; > + Error *local_err = NULL; > > - if (qdict_haskey(options, "path") == qdict_haskey(options, "host")) { > - if (qdict_haskey(options, "path")) { > - error_setg(errp, "path and host may not be used at the same > time"); > - } else { > - error_setg(errp, "one of path and host must be specified"); > - } > - return NULL; > + if (!nbd_process_legacy_socket_options(options, errp)) { > + goto fail; > } > - if (qdict_haskey(options, "port") && !qdict_haskey(options, "host")) { > - error_setg(errp, "port may not be used without host"); > - return NULL; > + > + qdict_extract_subqdict(options, &addr, "address."); > + if (!qdict_size(addr)) { > + error_setg(errp, "NBD server address missing"); > + goto fail; > } > > - saddr = g_new0(SocketAddress, 1); > + crumpled_addr = qdict_crumple(addr, true, errp); > + if (!crumpled_addr) { > + goto fail; > + } > Ah, so you ARE depending on Dan's qdict_crumple(). > - if (qdict_haskey(options, "path")) { > - UnixSocketAddress *q_unix; > - saddr->type = SOCKET_ADDRESS_KIND_UNIX; > - q_unix = saddr->u.q_unix.data = g_new0(UnixSocketAddress, 1); > - q_unix->path = g_strdup(qdict_get_str(options, "path")); > - qdict_del(options, "path"); > - } else { > - InetSocketAddress *inet; > - saddr->type = SOCKET_ADDRESS_KIND_INET; > - inet = saddr->u.inet.data = g_new0(InetSocketAddress, 1); > - inet->host = g_strdup(qdict_get_str(options, "host")); > - if (!qdict_get_try_str(options, "port")) { > - inet->port = g_strdup_printf("%d", NBD_DEFAULT_PORT); > - } else { > - inet->port = g_strdup(qdict_get_str(options, "port")); > - } > - qdict_del(options, "host"); > - qdict_del(options, "port"); > + iv = qmp_input_visitor_new(crumpled_addr); > + visit_type_SocketAddress(qmp_input_get_visitor(iv), NULL, &saddr, > + &local_err); and you DO use the generated QAPI code. Cool! > + qmp_input_visitor_cleanup(iv); > + if (local_err) { > + error_propagate(errp, local_err); > + goto fail; > } > > + /* TODO: Detect superfluous (unused) options under in addr */ Is this TODO addressed anywhere, or mentioned in the commit message? In fact, I think it's quite easy to address: qmp_input_visitor_new() recently changed signatures (commit fc471c18), so all you have to do is pass strict=true to that function as part of rebasing your work, at which point you will automatically reject any leftover keys in addr that didn't get parsed by SocketAddress. Needs a rebase, but looks promising. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature