From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37970) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bwXxW-0004Jx-1f for qemu-devel@nongnu.org; Tue, 18 Oct 2016 13:13:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bwXxV-0008UK-1T for qemu-devel@nongnu.org; Tue, 18 Oct 2016 13:13:50 -0400 From: Markus Armbruster References: <1475246744-29302-1-git-send-email-berrange@redhat.com> <1475246744-29302-15-git-send-email-berrange@redhat.com> Date: Tue, 18 Oct 2016 19:13:39 +0200 In-Reply-To: <1475246744-29302-15-git-send-email-berrange@redhat.com> (Daniel P. Berrange's message of "Fri, 30 Sep 2016 15:45:37 +0100") Message-ID: <878ttlinr0.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v14 14/21] qapi: allow repeated opts with qobject_input_visitor_new_opts List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org, Max Reitz , Paolo Bonzini , Andreas =?utf-8?Q?F=C3=A4rber?= "Daniel P. Berrange" writes: > The qobject_input_visitor_new_opts() method gains a new > parameter to control whether it allows repeated option > keys in the input QemuOpts or not. > > Signed-off-by: Daniel P. Berrange > --- > include/qapi/qobject-input-visitor.h | 6 ++++++ > qapi/qobject-input-visitor.c | 5 ++++- > 2 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/include/qapi/qobject-input-visitor.h b/include/qapi/qobject-input-visitor.h > index 242b767..bc5062a 100644 > --- a/include/qapi/qobject-input-visitor.h > +++ b/include/qapi/qobject-input-visitor.h > @@ -112,6 +112,11 @@ Visitor *qobject_input_visitor_new_autocast(QObject *obj, > * qobject_input_visitor_new_autocast() method. See the docs > * of that method for further details on processing behaviour. > * > + * If the @permit_repeated_opts parameter is true, then the input > + * @opts is allowed to contain repeated keys and they will be > + * turned into a QList. If it is false, then repeated keys will > + * result in an error being reported. > + * > * The returned input visitor should be released by calling > * visit_free() when no longer required. > */ > @@ -119,6 +124,7 @@ Visitor *qobject_input_visitor_new_opts(const QemuOpts *opts, > bool autocreate_list, > size_t autocreate_struct_levels, > bool permit_int_ranges, > + bool permit_repeated_opts, > Error **errp); > > #endif > diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c > index 2287d11..5a3872c 100644 > --- a/qapi/qobject-input-visitor.c > +++ b/qapi/qobject-input-visitor.c > @@ -753,6 +753,7 @@ Visitor *qobject_input_visitor_new_opts(const QemuOpts *opts, > bool autocreate_list, > size_t autocreate_struct_levels, > bool permit_int_ranges, > + bool permit_repeated_opts, > Error **errp) > { > QDict *pdict; > @@ -760,7 +761,9 @@ Visitor *qobject_input_visitor_new_opts(const QemuOpts *opts, > Visitor *v = NULL; > > pdict = qemu_opts_to_qdict(opts, NULL, > - QEMU_OPTS_REPEAT_POLICY_LAST, > + permit_repeated_opts ? > + QEMU_OPTS_REPEAT_POLICY_ALL : > + QEMU_OPTS_REPEAT_POLICY_ERROR, > errp); > if (!pdict) { > goto cleanup; @permit_repeated_opts applies to the whole @opts. However, the "repeat to build a list" feature is actually used more selectively. For instance, -spice wants it for "tls-channel" and "plaintext-channel", but not for its other keys. There, the code uses qemu_opt_get() or similar, which gets the last value. The options visitor works the same. Internally, it stores only lists. When the visit is for a scalar, it gets the last value from the list. When the visit is for a list, it sets up things so that the list element visits get the values in order. As long as no key is list-valued, you can keep @permit_repeated_opts false, and have duplicate keys rejected, for whatever that's worth. As soon as you have a list-valued key, you need to set @permit_repeated_opts to true. But then *no* duplicate keys are rejected. If consistency with @permit_repeated_opts = false is needed, then something else needs to detect unwanted lists and reject them. What use cases for @permit_repeated_opts = false do you envisage? To serve as replacement for the options visitor, this needs to treat duplicates the same. As we saw in PATCH 12, QEMU_OPTS_REPEAT_POLICY_ALL makes a list when the key is repeated, while the options visitor makes a list always. I guess some other patch hides this difference.