On 09/30/2016 09:45 AM, Daniel P. Berrange wrote: > Some of the historical command line opts that had their > keys in in a completely flat namespace are now represented > by QAPI schemas that use a nested structs. When converting
singular/plural mismatch; either s/a // or s/structs/struct/ > the QemuOpts to QObject, there is no information about > compound types available, so the QObject will be completely > flat, even after the qdict_crumple() call. So when starting > a struct, we may not have a QDict available in the input > data, so we auto-create a QDict containing all the currently > unvisited input data keys. Not all historical command line > opts require this, so the behaviour is opt-in, by specifying > how many levels of structs are permitted to be auto-created. > > Note that this only works if the child struct is the last > field to the visited in the parent struct. This is always > the case for currently existing legacy command line options. > > The example is the NetLegacy type which has 3 levels of > structs. The modern way to represent this in QemuOpts would > be the dot-separated component approach > > -net vlan=1,id=foo,name=bar,opts.type=tap,\ > opts.data.fd=3,opts.data.script=ifup > > The legacy syntax will just be presenting > > -net vlan=1,id=foo,name=bar,type=tap,fd=3,script=ifup So basically, any key=values that were not consumed as known keys in the parent struct are reparsed as the (presumably lone) remaining QDict child, recursing as needed to follow the QAPI nesting. Does this still work if the command line is presented in a different order: -net script=ifup,vlan=1,type=tap,fd=3,id=foo,name=bar My guess is yes, but if the testsuite doesn't cover it, you may want to add a test. > > So we need to auto-create 3 levels of struct when visiting. > > The implementation here will enable visiting in both the > modern and legacy syntax, compared to OptsVisitor which > only allows the legacy syntax. > > Signed-off-by: Daniel P. Berrange <berra...@redhat.com> > --- > include/qapi/qobject-input-visitor.h | 22 +++++- > qapi/qobject-input-visitor.c | 59 ++++++++++++++-- > tests/test-qobject-input-visitor.c | 130 > ++++++++++++++++++++++++++++++++--- > 3 files changed, 194 insertions(+), 17 deletions(-) > > diff --git a/include/qapi/qobject-input-visitor.h > b/include/qapi/qobject-input-visitor.h > index 1809f48..94051f3 100644 > --- a/include/qapi/qobject-input-visitor.h > +++ b/include/qapi/qobject-input-visitor.h > @@ -45,7 +45,7 @@ Visitor *qobject_input_visitor_new(QObject *obj, bool > strict); > * If @autocreate_list is true, then as an alternative to a normal QList, > * list values can be stored as a QString or QDict instead, which will > * be interpreted as representing single element lists. This should only > - * by used if compatibility is required with the OptsVisitor which allowed > + * be used if compatibility is required with the OptsVisitor which allowed This typo fix should be hoisted earlier in the series when it was introduced. > * repeated keys, without list indexes, to represent lists. e.g. set this > * to true if you have compatibility requirements for > * > @@ -55,6 +55,22 @@ Visitor *qobject_input_visitor_new(QObject *obj, bool > strict); > * > * -arg foo.0=hello,foo.1=world > * > + * If @autocreate_struct_levels is non-zero, then when visiting structs, > + * if the corresponding QDict is not found, it will automatically create > + * a QDict containing all remaining unvisited options. This should only > + * be used if compatibility is required with the OptsVisitor which flatten > + * structs so that all keys were at the same level. e.g. set this to a > + * non-zero number if you compatibility requirements for > + * > + * -arg type=person,surname=blogs,forename=fred > + * > + * to be treated as equivalent to the perferred syntax s/perferred/preferred/ > +static void test_visitor_in_struct_autocreate(TestInputVisitorData *data, > + const void *unused) > +{ > + Visitor *v; > + int64_t vlan; > + char *id = NULL; > + char *type; > + int64_t fd; > + char *script = NULL; > + > + v = visitor_input_test_init_internal( > + data, true, true, false, 3, > + "{ 'vlan': '1', 'id': 'foo', 'type': 'tap', 'fd': '3', " > + "'script': 'ifup' }", NULL); > + This is where intentionally mixing up the parameter orders might be worthwhile. Overall, the idea looks reasonable. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature