Message ID | 1350076268-18461-25-git-send-email-mdroth@linux.vnet.ibm.com |
---|---|
State | New |
Headers | show |
Il 12/10/2012 23:11, Michael Roth ha scritto: > + elif field['type'].startswith('enum '): > + typename = 'int' Note that there is support for enum properties in qdev. Please consider adding it, though it can be done as a follow-up. I'm going to play a bit with the series and convert 1 or 2 devices myself to see how it looks, then I'll give my acked-by. Paolo
Il 15/10/2012 10:12, Paolo Bonzini ha scritto: > Il 12/10/2012 23:11, Michael Roth ha scritto: >> + elif field['type'].startswith('enum '): >> + typename = 'int' > > Note that there is support for enum properties in qdev. Please consider > adding it, though it can be done as a follow-up. > > I'm going to play a bit with the series and convert 1 or 2 devices > myself to see how it looks, then I'll give my acked-by. Ok, so now I played with it a bit. My main comments, which can all be tackled as a follow-up, are: - immutable/derived/broken/elsewhere (and the default, let's call it serialized) are really five cases of the same QIDL property. Perhaps this could be enforced in the extended syntax like this: #define q_immutable QIDL(serialize(immutable)) #define q_derived QIDL(serialize(derived)) #define q_broken QIDL(serialize(broken)) #define q_elsewhere QIDL(serialize(elsewhere)) I would also make it possible to explicitly specify the fifth state, if only for symmetry. I'm not sure what your plans are for q_derived vs. VMState. If a field X is set in pre_save hooks based on field Y, how should the fields be set? X is usually not up-to-date, so it should be q_derived. But Y cannot be serialized as is, so it should be q_elsewhere. One of the two is wrong, which one? :) - q_properties are also always q_immutable. I think this should be enforced in the code generator. - it would be _much_ better if you could automatically derive properties information for embedded structs. For example, Notifiers and qemu_irqs are always q_immutable. NotifierLists probably are always q_elsewhere, because the owner of the notifiers should add themselves back. In general, if struct X is QIDL_DECLAREd and only has q_immutable fields, it can be taken as q_immutable. Hence for example the base class should not need any decoration; ISADevice will be seen as q_immutable, but PCIDevice will be seen as serialized. But even if a struct is not QIDL_DECLAREd, it should be possible to apply a tag to a typedef, and have it always applied to the members. Paolo
On Mon, Oct 15, 2012 at 03:08:51PM +0200, Paolo Bonzini wrote: > Il 15/10/2012 10:12, Paolo Bonzini ha scritto: > > Il 12/10/2012 23:11, Michael Roth ha scritto: > >> + elif field['type'].startswith('enum '): > >> + typename = 'int' > > > > Note that there is support for enum properties in qdev. Please consider > > adding it, though it can be done as a follow-up. > > > > I'm going to play a bit with the series and convert 1 or 2 devices > > myself to see how it looks, then I'll give my acked-by. > > Ok, so now I played with it a bit. My main comments, which can all be > tackled as a follow-up, are: > > - immutable/derived/broken/elsewhere (and the default, let's call it > serialized) are really five cases of the same QIDL property. Perhaps > this could be enforced in the extended syntax like this: > > #define q_immutable QIDL(serialize(immutable)) > #define q_derived QIDL(serialize(derived)) > #define q_broken QIDL(serialize(broken)) > #define q_elsewhere QIDL(serialize(elsewhere)) > > I would also make it possible to explicitly specify the fifth state, if > only for symmetry. Agreed, that's a more proper grouping. Though, for consistency with QIDL(property, ...), I would do QIDL(serialize, ...) > > I'm not sure what your plans are for q_derived vs. VMState. If a field > X is set in pre_save hooks based on field Y, how should the fields be > set? X is usually not up-to-date, so it should be q_derived. But Y > cannot be serialized as is, so it should be q_elsewhere. One of the > two is wrong, which one? :) > > - q_properties are also always q_immutable. I think this should be > enforced in the code generator. Agreed. > > - it would be _much_ better if you could automatically derive properties > information for embedded structs. For example, Notifiers and qemu_irqs > are always q_immutable. NotifierLists probably are always q_elsewhere, > because the owner of the notifiers should add themselves back. q_inherit maybe? Otherwise we're overriding "q_default" in subtle ways that may not always be desired. > > In general, if struct X is QIDL_DECLAREd and only has q_immutable > fields, it can be taken as q_immutable. Hence for example the base > class should not need any decoration; ISADevice will be seen as > q_immutable, but PCIDevice will be seen as serialized. But even if a > struct is not QIDL_DECLAREd, it should be possible to apply a tag to a > typedef, and have it always applied to the members. I'd prefer to stick with a common declaration macro to handle tagging. How about QIDL_DECLARE(MyImmutableType, q_immutable) to apply a tag to all members? > > Paolo >
On Mon, Oct 15, 2012 at 11:35:46AM -0500, Michael Roth wrote: > On Mon, Oct 15, 2012 at 03:08:51PM +0200, Paolo Bonzini wrote: > > Il 15/10/2012 10:12, Paolo Bonzini ha scritto: > > > Il 12/10/2012 23:11, Michael Roth ha scritto: > > >> + elif field['type'].startswith('enum '): > > >> + typename = 'int' > > > > > > Note that there is support for enum properties in qdev. Please consider > > > adding it, though it can be done as a follow-up. > > > > > > I'm going to play a bit with the series and convert 1 or 2 devices > > > myself to see how it looks, then I'll give my acked-by. > > > > Ok, so now I played with it a bit. My main comments, which can all be > > tackled as a follow-up, are: > > > > - immutable/derived/broken/elsewhere (and the default, let's call it > > serialized) are really five cases of the same QIDL property. Perhaps > > this could be enforced in the extended syntax like this: > > > > #define q_immutable QIDL(serialize(immutable)) > > #define q_derived QIDL(serialize(derived)) > > #define q_broken QIDL(serialize(broken)) > > #define q_elsewhere QIDL(serialize(elsewhere)) > > > > I would also make it possible to explicitly specify the fifth state, if > > only for symmetry. > > Agreed, that's a more proper grouping. Though, for consistency with > QIDL(property, ...), I would do QIDL(serialize, ...) > Er, meant to respond to this in my previous reply: > > > > I'm not sure what your plans are for q_derived vs. VMState. If a field > > X is set in pre_save hooks based on field Y, how should the fields be > > set? X is usually not up-to-date, so it should be q_derived. But Y > > cannot be serialized as is, so it should be q_elsewhere. One of the > > two is wrong, which one? :) Why is it that Y can't be serialized as is in this example? X's derived state depends on it, so Y should be serialized in some form. Are you referring to a case where VMState sets/sends X currently, but not Y? If so, my goal is that serialization is done properly and independently of VMState. Then, for migration, we serialize it into a QObject (state_obj) from which a VMState-compatible QObject (wire_obj) can be built and fed to the wire (either via modified/re-interpreted VMSD, or a re-modeling of the VMSD into a wire schema (possibly automagically via QIDL at first) from which a savevm visitor could be generated). So, for example, a post_serialize() migration hook would have access to an up-to-date Y in the state_obj, since: a) Y is up-date (by virtue of not being q_derived). b) Y is q_elsewhere, but our post_serialize() hook has (or can be given) access to the Object* that serializes Y, since we're able to obtain a pointer to it or one of it's members. One exception might be that we send X unecessarilly, since it's derived from Y which is actually q_immutable, but there's no restriction that q_immutable fields cannot be marked as, say, q_default, so we can add those in. Hopefully in most cases the mapping from state_obj <-> wire_obj is trivial though and is little more than a qdict_copy(), but we can make the mapping from state_obj <-> wire_obj as flexible as we need to cover these cases, and potentially other compatibility requirements. > > > > Paolo > >
diff --git a/scripts/qidl.py b/scripts/qidl.py new file mode 100644 index 0000000..4e0880e --- /dev/null +++ b/scripts/qidl.py @@ -0,0 +1,290 @@ +# +# QIDL Code Generator +# +# Copyright IBM, Corp. 2012 +# +# Authors: +# Anthony Liguori <aliguori@us.ibm.com> +# Michael Roth <mdroth@linux.vnet.ibm.com> +# +# This work is licensed under the terms of the GNU GPLv2 or later. +# See the COPYING file in the top-level directory. + +from ordereddict import OrderedDict +from qidl_parser import parse_file +from qapi import parse_schema, mcgen, camel_case, de_camel_case +from qapi_visit import generate_visit_struct, push_indent, pop_indent +import sys +import getopt +import os +import errno + +def qapi_schema(node): + schema = OrderedDict() + data = OrderedDict() + fields = None + if node.has_key('typedef'): + schema['type'] = node['typedef'] + fields = node['type']['fields'] + elif node.has_key('struct'): + schema['type'] = node['struct'] + fields = node['fields'] + else: + raise Exception("top-level neither typedef nor struct") + + for field in fields: + if filter(lambda x: field.has_key(x), \ + ['is_derived', 'is_immutable', 'is_broken', 'is_function', \ + 'is_nested_decl', 'is_elsewhere']): + continue + + description = OrderedDict() + + if field['type'].endswith('_t'): + typename = field['type'][:-2] + elif field['type'].startswith('struct '): + typename = field['type'].split(" ")[1] + elif field['type'].startswith('enum '): + typename = 'int' + elif field['type'] == "_Bool": + typename = 'bool' + elif field['type'].endswith("char") and field.has_key('is_pointer'): + typename = 'str'; + elif field['type'] == 'int': + typename = 'int32' + elif field['type'] == 'unsigned int': + typename = 'uint32' + elif field['type'] == 'char': + typename = 'uint8' + else: + typename = field['type'] + + if field.has_key('is_array') and field['is_array']: + description['type'] = [typename] + description['<annotated>'] = 'true' + if field.has_key('array_size'): + description['array_size'] = field['array_size'] + if field.has_key('array_capacity'): + description['array_capacity'] = field['array_capacity'] + elif camel_case(de_camel_case(typename)) == typename and \ + (not field.has_key('is_pointer') or not field['is_pointer']): + description['type'] = typename + description['<annotated>'] = 'true' + description['embedded'] = 'true' + else: + description = typename + + if field.has_key('is_optional') and field['is_optional']: + data["*" + field['variable']] = description + else: + data[field['variable']] = description + + schema['data'] = data + return schema + +def parse_schema_file(filename): + return parse_schema(open(filename, 'r')) + +def write_file(output, filename): + if filename: + output_file = open(filename, 'w') + else: + output_file = sys.stdout + output_file.write(output) + if filename: + output_file.close() + +def property_list(node): + prop_list = [] + fields = None + if node.has_key('typedef'): + state = node['typedef'] + fields = node['type']['fields'] + elif node.has_key('struct'): + state = node['struct'] + fields = node['fields'] + else: + raise Exception("top-level neither typedef nor struct") + + for field in fields: + if not field.has_key('is_property'): + continue + + for arglist in field['property_fields']: + if field['variable'] == 'devfn': + typename = 'pci_devfn' + elif field['type'].endswith('_t'): + typename = field['type'][:-2] + elif field['type'] == "_Bool": + typename = 'bool' + elif field.has_key('is_pointer'): + if field['type'] in ("char", "const char"): + typename = "string" + elif field['type'] == "void": + typename = "ptr" + else: + typename = field['type'] + + prop = {} + prop['name'] = arglist[0] + prop['state'] = state + prop['type'] = typename + prop['field'] = field['variable'] + if len(arglist) == 2: + prop['default'] = arglist[1] + elif len(arglist) == 3: + prop['type'] = 'bit' + prop['bit'] = arglist[1] + prop['default'] = arglist[2] + + prop_list.append(prop) + + return prop_list + +def generate_include(include_path): + return mcgen(''' +#include "%(include)s" +''', + include=include_path) + +def generate_property_bit(type_name, prop): + if prop.has_key('default'): + return mcgen(''' + DEFINE_PROP_BIT(%(name)s, %(state)s, %(field)s, %(bit)s, %(default)s), +''', + name=prop['name'], state=prop['state'], + field=prop['field'], bit=prop['bit'], + default=prop['default']) + return mcgen(''' + DEFINE_PROP_BIT(%(name)s, %(state)s, %(field)s, %(bit)s), +''', + name=prop['name'], state=prop['state'], + field=prop['field'], bit=prop['bit']) + +def generate_property(type_name, prop): + if prop.has_key('default'): + return mcgen(''' + DEFINE_PROP_%(type)s(%(name)s, %(state)s, %(field)s, %(default)s), +''', + type=prop['type'].upper(), name=prop['name'], + state=prop['state'], field=prop['field'], + default=prop['default']) + return mcgen(''' + DEFINE_PROP_%(type)s(%(name)s, %(state)s, %(field)s), +''', + type=prop['type'].upper(), name=prop['name'], + state=prop['state'], field=prop['field']) + +def generate_properties(type_name, prop_list=[]): + output = "" + + for prop in prop_list: + if prop['type'] == 'bit': + output += generate_property_bit(type_name, prop) + else: + output += generate_property(type_name, prop) + + output += mcgen(''' + DEFINE_PROP_END_OF_LIST() +''') + + return output + +def generate_qidl_registration(type_name, schema, do_state, prop_list=[]): + schema_text = "" + for line in schema.to_json().split("\n"): + schema_text += "\n\"%s\\n\"" % line + visitor = "NULL" + if do_state: + visitor = "visit_type_%s" % type_name + + return mcgen(''' +static char *%(type_name)s_get_schema(Object *obj, Error **errp) +{ + return g_strdup(qidl_data_%(type_name)s.schema_json_text); +} + +static void %(type_name)s_register_qidl(void) +{ + static Property properties[] = { +%(properties)s + }; + ObjectProperty *schema_link; + + qidl_data_%(type_name)s.properties = properties; + qidl_data_%(type_name)s.visitor = %(visitor)s; + qidl_data_%(type_name)s.schema_json_text = %(schema_text)s; + + schema_link = object_property_find(container_get(object_get_root(), "/qidl/schemas"), + "%(type_name)s", NULL); + qidl_data_%(type_name)s.schema_obj = container_get(object_get_root(), "/qidl/schemas/%(type_name)s"); + if (!schema_link) { + object_property_add_str(qidl_data_%(type_name)s.schema_obj, "json_text", + %(type_name)s_get_schema, NULL, NULL); + } +} + +qidl_init(%(type_name)s_register_qidl) +''', + type_name=type_name, schema_text=schema_text, visitor=visitor, + properties=generate_properties(type_name, prop_list)) + +def main(argv=[]): + try: + opts, args = getopt.gnu_getopt(argv[1:], "o:cd:I:", + ["output-filepath=", "schema-filepath=", + "include="]) + except getopt.GetoptError, err: + print >> sys.stderr, err + return 1 + + output_filepath = None + schema_filepath = None + includes = [] + for o, a in opts: + if o in ("-f", "--output-filepath"): + output_filepath = a + if o in ("-s", "--schema-filepath"): + schema_filepath = a + elif o in ("-I", "--include"): + includes.append(a) + + nodes = parse_file(sys.stdin) + if not nodes: + return 2 + + if os.path.dirname(output_filepath) != "": + try: + os.makedirs(os.path.dirname(output_filepath)) + except os.error, e: + if e.errno != errno.EEXIST: + raise + output = "" + schema_text_all = "" + for include in includes: + output += generate_include(include) + for node in nodes: + do_state = False + schema = qapi_schema(node) + prop_list = [] + # qapi parser expects iteration to be line-by-line + schema_text = schema.to_json() + expr = parse_schema(schema_text.split("\n"))[0] + schema_text_all += schema_text + "\n\n" + + if node.has_key('do_state') and node['do_state']: + do_state = True + output += generate_visit_struct(expr['type'], expr['data'], True) + if node.has_key('do_properties') and node['do_properties']: + prop_list = property_list(node) + + output += generate_qidl_registration(expr['type'], schema, do_state, prop_list) + + if schema_filepath and schema_text_all != "": + write_file(schema_text_all, schema_filepath) + write_file(output, output_filepath) + + return 0 + +if __name__ == '__main__': + sys.exit(main(sys.argv))