| Message ID | 1459967330-4573-7-git-send-email-mreitz@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 04/06/2016 12:28 PM, Max Reitz wrote: > Right now, we have four possible options that conflict with specifying > an NBD filename, and a future patch will add another one ("address"). > This future option is a nested QDict that is flattened at this point, > requiring as to test each option whether its key has an "address." > prefix. Therefore, we will then need to iterate through all options. > > Adding this iteration logic now will simplify adding the new option > later. A nice side effect is that the user will not receive a long list > of five options which are not supposed to be specified with a filename, > but we can actually print the problematic option. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > block/nbd.c | 26 ++++++++++++++++++++------ > 1 file changed, 20 insertions(+), 6 deletions(-) > > diff --git a/block/nbd.c b/block/nbd.c > index d12bcc6..1736f68 100644 > --- a/block/nbd.c > +++ b/block/nbd.c > @@ -120,6 +120,25 @@ out: > return ret; > } > > +static bool nbd_has_filename_options_conflict(QDict *options, Error **errp) > +{ > + const QDictEntry *e; > + > + for (e = qdict_first(options); e; e = qdict_next(options, e)) { > + if (!strcmp(e->key, "host") > + || !strcmp(e->key, "port") I know there are already instances of breaking before || in this file, but most of qemu breaks after, as in: if (!strcmp(e->key, "host") || !strcmp(e->key, "port") || ... But choice of formatting is trivial, so: Reviewed-by: Eric Blake <eblake@redhat.com>
diff --git a/block/nbd.c b/block/nbd.c index d12bcc6..1736f68 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -120,6 +120,25 @@ out: return ret; } +static bool nbd_has_filename_options_conflict(QDict *options, Error **errp) +{ + const QDictEntry *e; + + for (e = qdict_first(options); e; e = qdict_next(options, e)) { + if (!strcmp(e->key, "host") + || !strcmp(e->key, "port") + || !strcmp(e->key, "path") + || !strcmp(e->key, "export")) + { + error_setg(errp, "Option '%s' cannot be used with a file name", + e->key); + return true; + } + } + + return false; +} + static void nbd_parse_filename(const char *filename, QDict *options, Error **errp) { @@ -128,12 +147,7 @@ static void nbd_parse_filename(const char *filename, QDict *options, const char *host_spec; const char *unixpath; - if (qdict_haskey(options, "host") - || qdict_haskey(options, "port") - || qdict_haskey(options, "path")) - { - error_setg(errp, "host/port/path and a file name may not be specified " - "at the same time"); + if (nbd_has_filename_options_conflict(options, errp)) { return; }
Right now, we have four possible options that conflict with specifying an NBD filename, and a future patch will add another one ("address"). This future option is a nested QDict that is flattened at this point, requiring as to test each option whether its key has an "address." prefix. Therefore, we will then need to iterate through all options. Adding this iteration logic now will simplify adding the new option later. A nice side effect is that the user will not receive a long list of five options which are not supposed to be specified with a filename, but we can actually print the problematic option. Signed-off-by: Max Reitz <mreitz@redhat.com> --- block/nbd.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-)