On 09/07/2016 08:40 AM, Markus Armbruster wrote: >>>>>> Yet another option is to add 'ifdef' keys to the definitions >>>>>> themselves, i.e. >>>>>> >>>>>> { 'command': 'query-gic-capabilities', 'returns': ['GICCapability'], >>>>>> 'ifdef': 'TARGET_ARM' }
Seems like it might be useful; could even be expanded to: { 'command': 'query-arm-x86', 'returns': ['Foo'], 'ifdef': [ 'TARGET_ARM', 'TARGET_X86' ] } unless you are okay with it instead being: { 'command': 'query-arm-x86', 'returns': ['Foo'], 'ifdef': 'TARGET_ARM || TARGET_X86' } Either way, we need to make sure that whatever we put here is easy to translate into the appropriate generated code; and keeping in mind that while '#if A || B' makes sense to the preprocessor, '#ifdef A || B' does not. >>>> Fixable the same way as always when a definition needs to grow >>>> additional properties, but the syntax only provides a single value: make >>>> that single value an object, and the old non-object value syntactic >>>> sugar for the equivalent object value. We've previously discussed this >>>> technique in the context of giving command arguments default values. >>>> I'm not saying this is what we should do here, only pointing out it's >>>> possible. >>>> >>> >>> Ok, but let's find something, if possible simple and convenient, no? >> >> I agree it needs to be simple, both the interface (QAPI language) and >> the implementation. However, I don't like "first past the post". I >> prefer to explore the design space a bit. Another nice thing about expanding it by changing 'name':'typestr' to be sugar for 'name':{'type':'typestr'} is that QMP introspection is already prepared to expose a dictionary of attributes (type being one, and ifdef'ness being another) per member, so we already would have a nice mapping between qapi JSON files (a member name tied to a dictionary) and the generated output. >> >> So let me explore the "add 'ifdef' keys to definitions" corner of the >> QAPI language design space. >> >> Easily done for top-level definitions, because they're all JSON objects. >> Could even add it to the include directive if we wanted to. >> >> Less easily done for enumeration, struct, union and alternate members. >> Note that command and event arguments specified inline are a special >> case of struct members. >> and ideally, whatever we can do for a named struct, we can also do for an anonymous inlined struct for the command and event arguments. > > I ported qmp-commands.hx's #if to qapi-schema.json. The TARGET_FOO are > poisoned, so I commented them out. There's a CONFIG_SPICE left, which > will do for testing. Obviously, we'd have to find a way to work around the poisoned ifdef names, but the idea makes some sense to me. > > I also turned key 'gen': false into 'if': false. Possibly a bad idea. Maybe, maybe not. We have so few that it's easy to convert them all in one go, at which point you are just giving directives to the code generator: if the 'if' key is present, it controls what gets omitted (either the omitted code is gated by #if (or #ifdef) of the gate string, or 'if':false means the generator omits the code altogether). > @@ -475,16 +475,18 @@ arguments for the user's function out of an input > QDict, calls the > user's function, and if it succeeded, builds an output QObject from > its return value. > > +FIXME document 'if' > + And in particular, what the string must look like (will it be fed to generated #if or to generated #ifdef? What if you have more than one conditional?) > +++ b/qapi-schema.json > @@ -1269,7 +1269,9 @@ > # > # Since: 0.14.0 > ## > -{ 'command': 'query-spice', 'returns': 'SpiceInfo' } > +{ 'command': 'query-spice', > + 'if': 'defined(CONFIG_SPICE)', > + 'returns': 'SpiceInfo' } So the idea here is that qemu built without SPICE support would completely lack the query-spice command. This is currently detectable during 'query-commands' but NOT detectable during 'query-qmp-schema'; but after the patch, we could begin to make the introspection likewise hide the unused command. Sounds useful. > > ## > # @BalloonInfo: > @@ -2355,6 +2357,7 @@ > # Since: 2.5 > ## > { 'command': 'dump-skeys', > +# 'if': 'defined(TARGET_S390X)', > 'data': { 'filename': 'str' } } And here you ran into the poisoned variable problem. Obviously something to be solved if we like the overall idea. > +++ b/scripts/qapi-introspect.py > @@ -30,8 +30,6 @@ def to_json(obj, level=0): > ret = '{' + ', '.join(elts) + '}' > else: > assert False # not implemented > - if level == 1: > - ret = '\n' + ret > return ret > > > @@ -69,8 +67,15 @@ class QAPISchemaGenIntrospectVisitor(QAPISchemaVisitor): > extern const char %(c_name)s[]; > ''', > c_name=c_name(name)) > - lines = to_json(jsons).split('\n') > - c_string = '\n '.join([to_c_string(line) for line in lines]) > + c_string = '"["' > + for i in jsons: > + js, genif = i > + # FIXME blank lines are off > + c_string += gen_pp_if(genif or True) > + c_string += '\n ' + to_c_string(to_json(js) + ', ') > + c_string += gen_pp_endif(genif or True) And this change to introspection output shows the true power - by having the conditional expression be part of the .json QAPI file, we can now reflect the conditions through ALL parts of the generated code, instead of having discrepancies between query-commands vs. query-qmp-schema. At any rate, for a quick day's hack, I like this approach for feeling like it fits in with the QAPI language, rather than being bolted on as additional non-JSON syntax. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature