Eric Blake <ebl...@redhat.com> writes: > A regression in commit 15c2f669e caused us to silently ignore > excess input to the QemuOpts visitor. Later, commit ea4641 > accidentally abused that situation, by removing "qom-type" and > "id" from the corresponding QDict but leaving them defined in > the QemuOpts, when using the pair of containers to create a > user-defined object. Note that since we are already traversing > two separate items (a QDict and a QemuOpts), we are already > able to flag bogus arguments, as in: > > $ ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -nographic -qmp stdio > -object memory-backend-ram,id=mem1,size=4k,bogus=huh > qemu-system-x86_64: -object memory-backend-ram,id=mem1,size=4k,bogus=huh: > Property '.bogus' not found > > So the only real concern is that when we re-enable strict checking > in the QemuOpts visitor, we do not want to start flagging the two > leftover keys as unvisited. Rearrange the code to clean out the > QemuOpts listing in advance, rather than removing items from the > QDict. Since "qom-type" is usually an automatic implicit default, > we don't have to restore it; but "id" has to be put back (requiring > us to cast away a const).
This is a yet another example of how actual configuration can easily diverge from the one in QemuOpts. Discussed recently in: Subject: Re: [PATCH 0/2] add writeconfig command on monitor Message-ID: <87k28qlca9....@dusky.pond.sub.org> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg03476.html Not putting "qom-type" back here is okay. Putting it back would also be okay. I guess what you prefer depends on your level of OCD. > CC: qemu-sta...@nongnu.org > Signed-off-by: Eric Blake <ebl...@redhat.com> > > --- > v2: new patch > --- > qom/object_interfaces.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c > index 03a95c3..cc9a694 100644 > --- a/qom/object_interfaces.c > +++ b/qom/object_interfaces.c > @@ -114,7 +114,7 @@ Object *user_creatable_add_opts(QemuOpts *opts, Error > **errp) > QDict *pdict; > Object *obj; > const char *id = qemu_opts_id(opts); > - const char *type = qemu_opt_get(opts, "qom-type"); > + char *type = qemu_opt_get_del(opts, "qom-type"); > > if (!type) { > error_setg(errp, QERR_MISSING_PARAMETER, "qom-type"); > @@ -125,14 +125,15 @@ Object *user_creatable_add_opts(QemuOpts *opts, Error > **errp) > return NULL; > } > > + qemu_opts_set_id(opts, NULL); > pdict = qemu_opts_to_qdict(opts, NULL); > - qdict_del(pdict, "qom-type"); > - qdict_del(pdict, "id"); > > v = opts_visitor_new(opts); > obj = user_creatable_add_type(type, id, pdict, v, errp); > visit_free(v); > > + qemu_opts_set_id(opts, (char *) id); > + g_free(type); > QDECREF(pdict); > return obj; > } Aside: I dislike how this converts QemuOpts to QDict so user_creatable_add_type() can use the QDict to guide the visit. Awkward, as so much code that uses QemuOpts in not entirely straightforward ways.
