On 07/09/2013 03:53 AM, Kevin Wolf wrote: > The discriminator for anonymous unions is the data type. This allows to > have a union type that allows both of these: > > { 'file': 'my_existing_block_device_id' } > { 'file': { 'filename': '/tmp/mydisk.qcow2', 'read-only': true } } > > Unions like this are specified in the schema with an empty dict as > discriminator. For this example you could take: > > { 'union': 'BlockRef', > 'discriminator': {}, > 'data': { 'definition': 'BlockOptions' > 'reference': 'str' } } > { 'type': 'ExampleObject', > 'data: { 'file': 'BlockRef' } }
Yay - a commit message that shows the new QMP wire format, and the qapi-schema.json that generated it. [Without reading the patch yet] I take it the 'data' of such a union must be completely distinguishable by type - the visitor for the 'file' key tries each subtype in turn ('BlockOptions' or 'str') until one does not have a parse error. And this changes the earlier patch that required 'discriminator' to be used only when 'base' was also present. Can it distinguish between two structs, or is the union only allowed to have one struct option and all other options are primitive types? Should we consider the use of arrays as a union member type? Hmm, I wonder if this new anonymous union can be useful in helping us write the self-description of introspection in Amos' patches. > > Signed-off-by: Kevin Wolf <kw...@redhat.com> > --- > include/qapi/qmp/qobject.h | 1 + > include/qapi/visitor-impl.h | 2 ++ > include/qapi/visitor.h | 3 +++ > qapi/qapi-visit-core.c | 9 +++++++++ > qapi/qmp-input-visitor.c | 14 ++++++++++++++ > qobject/qjson.c | 2 ++ > scripts/qapi-types.py | 42 ++++++++++++++++++++++++++++++++++++++++ > scripts/qapi-visit.py | 47 > +++++++++++++++++++++++++++++++++++++++++++++ > scripts/qapi.py | 28 +++++++++++++++++++++++++++ > 9 files changed, 148 insertions(+) > +++ b/scripts/qapi-visit.py > @@ -172,6 +172,49 @@ void visit_type_%(name)s(Visitor *m, %(name)s * obj, > const char *name, Error **e > ''', > name=name) > > +def generate_visit_anon_union(name, members): > + ret = mcgen(''' > + > +void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, > Error **errp) > +{ > + Error *err = NULL; > + > + if (!error_is_set(errp)) { > + visit_start_implicit_struct(m, (void**) obj, sizeof(%(name)s), &err); > + visit_get_next_type(m, (int*) &(*obj)->kind, %(name)s_qtypes, name, > &err); > + switch ((*obj)->kind) { > +''', > + name=name) > + > + for key in members: > + assert (members[key] in builtin_types > + or find_struct(members[key]) > + or find_union(members[key])), "Invalid anonymous union member" > + > + ret += mcgen(''' > + case %(abbrev)s_KIND_%(enum)s: > + visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, name, &err); > + break; > +''', > + abbrev = de_camel_case(name).upper(), > + enum = c_fun(de_camel_case(key),False).upper(), > + c_type=type_name(members[key]), > + c_name=c_fun(key)) Inconsistent spacing around '=' > + > + ret += mcgen(''' > + default: > + abort(); Does this mean I can cause qemu to abort if I pass bogus information on the wire? Using your commit message example, { 'file': false } would hit the default case, right? > +++ b/scripts/qapi.py > @@ -17,6 +17,21 @@ builtin_types = [ > 'uint8', 'uint16', 'uint32', 'uint64' > ] > > +builtin_type_qtypes = { > + 'str': 'QTYPE_QSTRING', > + 'int': 'QTYPE_QINT', > + 'number': 'QTYPE_QFLOAT', > + 'bool': 'QTYPE_QBOOL', > + 'int8': 'QTYPE_QINT', > + 'int16': 'QTYPE_QINT', > + 'int32': 'QTYPE_QINT', > + 'int64': 'QTYPE_QINT', > + 'uint8': 'QTYPE_QINT', > + 'uint16': 'QTYPE_QINT', > + 'uint32': 'QTYPE_QINT', > + 'uint64': 'QTYPE_QINT', What happens if I try to write a union in qapi-schema.json that has both a 'int8' and 'uint32' branch? Since both of those resolve to the QTYPE_QINT visitor, does that end up causing the C code to generate a switch statement with duplicate labels? > +} > + > def tokenize(data): > while len(data): > ch = data[0] > @@ -105,6 +120,7 @@ def parse_schema(fp): > if expr_eval.has_key('enum'): > add_enum(expr_eval['enum']) > elif expr_eval.has_key('union'): > + add_union(expr_eval) > add_enum('%sKind' % expr_eval['union']) > elif expr_eval.has_key('type'): > add_struct(expr_eval) > @@ -188,6 +204,7 @@ def type_name(name): > > enum_types = [] > struct_types = [] > +union_types = [] Is it worth splitting the tracking of the union_types of qapi-schema.json into an independent patch? Does it make sense to allow a union type (possibly only an anonymous union type) in place of a struct type as a top-level type reference? That is, just as we now allow { 'command':'foo', 'data': 'StructType' }, would it make sense to allow { 'command':bar', 'data': 'AnonUnion' }? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature