* [Qemu-devel] [PATCH 01/22] qapi: qapi-visit.py -> qapi_visit.py so we can import
2012-09-21 14:07 [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization Michael Roth
@ 2012-09-21 14:07 ` Michael Roth
2012-09-21 14:07 ` [Qemu-devel] [PATCH 02/22] qapi: qapi-types.py -> qapi_types.py Michael Roth
` (21 subsequent siblings)
22 siblings, 0 replies; 39+ messages in thread
From: Michael Roth @ 2012-09-21 14:07 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, peter.maydell, aliguori, eblake
Python doesn't allow "-" in module names, so we need to rename the file
so we can re-use bits of the codegen
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
Makefile | 8 ++++----
scripts/{qapi-visit.py => qapi_visit.py} | 0
tests/Makefile | 4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
rename scripts/{qapi-visit.py => qapi_visit.py} (100%)
diff --git a/Makefile b/Makefile
index 971e92f..b82f4a8 100644
--- a/Makefile
+++ b/Makefile
@@ -188,8 +188,8 @@ qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h :\
$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
qga/qapi-generated/qga-qapi-visit.c qga/qapi-generated/qga-qapi-visit.h :\
-$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
- $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
+$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi_visit.py $(qapi-py)
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_visit.py $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
qga/qapi-generated/qga-qmp-commands.h qga/qapi-generated/qga-qmp-marshal.c :\
$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
@@ -198,8 +198,8 @@ qapi-types.c qapi-types.h :\
$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(gen-out-type) -o "." < $<, " GEN $@")
qapi-visit.c qapi-visit.h :\
-$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
- $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(gen-out-type) -o "." < $<, " GEN $@")
+$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi_visit.py $(qapi-py)
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_visit.py $(gen-out-type) -o "." < $<, " GEN $@")
qmp-commands.h qmp-marshal.c :\
$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -m -o "." < $<, " GEN $@")
diff --git a/scripts/qapi-visit.py b/scripts/qapi_visit.py
similarity index 100%
rename from scripts/qapi-visit.py
rename to scripts/qapi_visit.py
diff --git a/tests/Makefile b/tests/Makefile
index 26a67ce..58d5b3f 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -55,8 +55,8 @@ tests/test-qapi-types.c tests/test-qapi-types.h :\
$(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-types.py
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(gen-out-type) -o tests -p "test-" < $<, " GEN $@")
tests/test-qapi-visit.c tests/test-qapi-visit.h :\
-$(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-visit.py
- $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(gen-out-type) -o tests -p "test-" < $<, " GEN $@")
+$(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi_visit.py
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_visit.py $(gen-out-type) -o tests -p "test-" < $<, " GEN $@")
tests/test-qmp-commands.h tests/test-qmp-marshal.c :\
$(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-commands.py
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -o tests -p "test-" < $<, " GEN $@")
--
1.7.9.5
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PATCH 02/22] qapi: qapi-types.py -> qapi_types.py
2012-09-21 14:07 [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization Michael Roth
2012-09-21 14:07 ` [Qemu-devel] [PATCH 01/22] qapi: qapi-visit.py -> qapi_visit.py so we can import Michael Roth
@ 2012-09-21 14:07 ` Michael Roth
2012-09-21 14:07 ` [Qemu-devel] [PATCH 03/22] qapi: qapi-commands.py -> qapi_commands.py Michael Roth
` (20 subsequent siblings)
22 siblings, 0 replies; 39+ messages in thread
From: Michael Roth @ 2012-09-21 14:07 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, peter.maydell, aliguori, eblake
Python doesn't allow "-" in module names, so we need to rename the file
so we can re-use bits of the codegen
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
Makefile | 8 ++++----
scripts/{qapi-types.py => qapi_types.py} | 0
tests/Makefile | 4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
rename scripts/{qapi-types.py => qapi_types.py} (100%)
diff --git a/Makefile b/Makefile
index b82f4a8..c1ccb47 100644
--- a/Makefile
+++ b/Makefile
@@ -185,8 +185,8 @@ endif
qapi-py = $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddict.py
qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h :\
-$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
- $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
+$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi_types.py $(qapi-py)
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_types.py $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
qga/qapi-generated/qga-qapi-visit.c qga/qapi-generated/qga-qapi-visit.h :\
$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi_visit.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_visit.py $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
@@ -195,8 +195,8 @@ $(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-p
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
qapi-types.c qapi-types.h :\
-$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
- $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(gen-out-type) -o "." < $<, " GEN $@")
+$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi_types.py $(qapi-py)
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_types.py $(gen-out-type) -o "." < $<, " GEN $@")
qapi-visit.c qapi-visit.h :\
$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi_visit.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_visit.py $(gen-out-type) -o "." < $<, " GEN $@")
diff --git a/scripts/qapi-types.py b/scripts/qapi_types.py
similarity index 100%
rename from scripts/qapi-types.py
rename to scripts/qapi_types.py
diff --git a/tests/Makefile b/tests/Makefile
index 58d5b3f..23a71f5 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -52,8 +52,8 @@ tests/test-coroutine$(EXESUF): tests/test-coroutine.o $(coroutine-obj-y) $(tools
tests/test-iov$(EXESUF): tests/test-iov.o iov.o
tests/test-qapi-types.c tests/test-qapi-types.h :\
-$(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-types.py
- $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(gen-out-type) -o tests -p "test-" < $<, " GEN $@")
+$(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi_types.py
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_types.py $(gen-out-type) -o tests -p "test-" < $<, " GEN $@")
tests/test-qapi-visit.c tests/test-qapi-visit.h :\
$(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi_visit.py
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_visit.py $(gen-out-type) -o tests -p "test-" < $<, " GEN $@")
--
1.7.9.5
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PATCH 03/22] qapi: qapi-commands.py -> qapi_commands.py
2012-09-21 14:07 [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization Michael Roth
2012-09-21 14:07 ` [Qemu-devel] [PATCH 01/22] qapi: qapi-visit.py -> qapi_visit.py so we can import Michael Roth
2012-09-21 14:07 ` [Qemu-devel] [PATCH 02/22] qapi: qapi-types.py -> qapi_types.py Michael Roth
@ 2012-09-21 14:07 ` Michael Roth
2012-09-21 14:07 ` [Qemu-devel] [PATCH 04/22] qapi: qapi_visit.py, make code useable as module Michael Roth
` (19 subsequent siblings)
22 siblings, 0 replies; 39+ messages in thread
From: Michael Roth @ 2012-09-21 14:07 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, peter.maydell, aliguori, eblake
Python doesn't allow "-" in module names, so we need to rename the file
so we can re-use bits of the codegen.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
Makefile | 8 ++++----
scripts/{qapi-commands.py => qapi_commands.py} | 0
tests/Makefile | 4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
rename scripts/{qapi-commands.py => qapi_commands.py} (100%)
diff --git a/Makefile b/Makefile
index c1ccb47..b1e1304 100644
--- a/Makefile
+++ b/Makefile
@@ -191,8 +191,8 @@ qga/qapi-generated/qga-qapi-visit.c qga/qapi-generated/qga-qapi-visit.h :\
$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi_visit.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_visit.py $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
qga/qapi-generated/qga-qmp-commands.h qga/qapi-generated/qga-qmp-marshal.c :\
-$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
- $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
+$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi_commands.py $(qapi-py)
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_commands.py $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@")
qapi-types.c qapi-types.h :\
$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi_types.py $(qapi-py)
@@ -201,8 +201,8 @@ qapi-visit.c qapi-visit.h :\
$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi_visit.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_visit.py $(gen-out-type) -o "." < $<, " GEN $@")
qmp-commands.h qmp-marshal.c :\
-$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
- $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -m -o "." < $<, " GEN $@")
+$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi_commands.py $(qapi-py)
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_commands.py $(gen-out-type) -m -o "." < $<, " GEN $@")
QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h qga-qmp-commands.h)
$(qga-obj-y) qemu-ga.o: $(QGALIB_GEN)
diff --git a/scripts/qapi-commands.py b/scripts/qapi_commands.py
similarity index 100%
rename from scripts/qapi-commands.py
rename to scripts/qapi_commands.py
diff --git a/tests/Makefile b/tests/Makefile
index 23a71f5..e10aaed 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -58,8 +58,8 @@ tests/test-qapi-visit.c tests/test-qapi-visit.h :\
$(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi_visit.py
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_visit.py $(gen-out-type) -o tests -p "test-" < $<, " GEN $@")
tests/test-qmp-commands.h tests/test-qmp-marshal.c :\
-$(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-commands.py
- $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -o tests -p "test-" < $<, " GEN $@")
+$(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi_commands.py
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi_commands.py $(gen-out-type) -o tests -p "test-" < $<, " GEN $@")
tests/test-string-output-visitor$(EXESUF): tests/test-string-output-visitor.o $(test-qapi-obj-y)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PATCH 04/22] qapi: qapi_visit.py, make code useable as module
2012-09-21 14:07 [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization Michael Roth
` (2 preceding siblings ...)
2012-09-21 14:07 ` [Qemu-devel] [PATCH 03/22] qapi: qapi-commands.py -> qapi_commands.py Michael Roth
@ 2012-09-21 14:07 ` Michael Roth
2012-09-21 14:07 ` [Qemu-devel] [PATCH 05/22] qapi: qapi_visit.py, support arrays and complex qapi definitions Michael Roth
` (18 subsequent siblings)
22 siblings, 0 replies; 39+ messages in thread
From: Michael Roth @ 2012-09-21 14:07 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, peter.maydell, aliguori, eblake
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
scripts/qapi_visit.py | 157 +++++++++++++++++++++++++------------------------
1 file changed, 81 insertions(+), 76 deletions(-)
diff --git a/scripts/qapi_visit.py b/scripts/qapi_visit.py
index e2093e8..974e458 100644
--- a/scripts/qapi_visit.py
+++ b/scripts/qapi_visit.py
@@ -234,55 +234,57 @@ void visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name, Error **e
''',
name=name)
-try:
- opts, args = getopt.gnu_getopt(sys.argv[1:], "chp:o:",
- ["source", "header", "prefix=", "output-dir="])
-except getopt.GetoptError, err:
- print str(err)
- sys.exit(1)
-
-output_dir = ""
-prefix = ""
-c_file = 'qapi-visit.c'
-h_file = 'qapi-visit.h'
-
-do_c = False
-do_h = False
-
-for o, a in opts:
- if o in ("-p", "--prefix"):
- prefix = a
- elif o in ("-o", "--output-dir"):
- output_dir = a + "/"
- elif o in ("-c", "--source"):
+def main(argv=[]):
+ try:
+ opts, args = getopt.gnu_getopt(sys.argv[1:], "chp:o:",
+ ["source", "header", "prefix=",
+ "output-dir="])
+ except getopt.GetoptError, err:
+ print str(err)
+ sys.exit(1)
+
+ output_dir = ""
+ prefix = ""
+ c_file = 'qapi-visit.c'
+ h_file = 'qapi-visit.h'
+
+ do_c = False
+ do_h = False
+
+ for o, a in opts:
+ if o in ("-p", "--prefix"):
+ prefix = a
+ elif o in ("-o", "--output-dir"):
+ output_dir = a + "/"
+ elif o in ("-c", "--source"):
+ do_c = True
+ elif o in ("-h", "--header"):
+ do_h = True
+
+ if not do_c and not do_h:
do_c = True
- elif o in ("-h", "--header"):
do_h = True
-if not do_c and not do_h:
- do_c = True
- do_h = True
+ c_file = output_dir + prefix + c_file
+ h_file = output_dir + prefix + h_file
-c_file = output_dir + prefix + c_file
-h_file = output_dir + prefix + h_file
+ try:
+ os.makedirs(output_dir)
+ except os.error, e:
+ if e.errno != errno.EEXIST:
+ raise
-try:
- os.makedirs(output_dir)
-except os.error, e:
- if e.errno != errno.EEXIST:
- raise
-
-def maybe_open(really, name, opt):
- if really:
- return open(name, opt)
- else:
- import StringIO
- return StringIO.StringIO()
+ def maybe_open(really, name, opt):
+ if really:
+ return open(name, opt)
+ else:
+ import StringIO
+ return StringIO.StringIO()
-fdef = maybe_open(do_c, c_file, 'w')
-fdecl = maybe_open(do_h, h_file, 'w')
+ fdef = maybe_open(do_c, c_file, 'w')
+ fdecl = maybe_open(do_h, h_file, 'w')
-fdef.write(mcgen('''
+ fdef.write(mcgen('''
/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
/*
@@ -302,7 +304,7 @@ fdef.write(mcgen('''
''',
header=basename(h_file)))
-fdecl.write(mcgen('''
+ fdecl.write(mcgen('''
/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
/*
@@ -326,39 +328,42 @@ fdecl.write(mcgen('''
''',
prefix=prefix, guard=guardname(h_file)))
-exprs = parse_schema(sys.stdin)
-
-for expr in exprs:
- if expr.has_key('type'):
- ret = generate_visit_struct(expr['type'], expr['data'])
- ret += generate_visit_list(expr['type'], expr['data'])
- fdef.write(ret)
-
- ret = generate_declaration(expr['type'], expr['data'])
- fdecl.write(ret)
- elif expr.has_key('union'):
- ret = generate_visit_union(expr['union'], expr['data'])
- ret += generate_visit_list(expr['union'], expr['data'])
- fdef.write(ret)
-
- ret = generate_decl_enum('%sKind' % expr['union'], expr['data'].keys())
- ret += generate_declaration(expr['union'], expr['data'])
- fdecl.write(ret)
- elif expr.has_key('enum'):
- ret = generate_visit_list(expr['enum'], expr['data'])
- ret += generate_visit_enum(expr['enum'], expr['data'])
- fdef.write(ret)
-
- ret = generate_decl_enum(expr['enum'], expr['data'])
- ret += generate_enum_declaration(expr['enum'], expr['data'])
- fdecl.write(ret)
-
-fdecl.write('''
+ exprs = parse_schema(sys.stdin)
+
+ for expr in exprs:
+ if expr.has_key('type'):
+ ret = generate_visit_struct(expr['type'], expr['data'])
+ ret += generate_visit_list(expr['type'], expr['data'])
+ fdef.write(ret)
+
+ ret = generate_declaration(expr['type'], expr['data'])
+ fdecl.write(ret)
+ elif expr.has_key('union'):
+ ret = generate_visit_union(expr['union'], expr['data'])
+ ret += generate_visit_list(expr['union'], expr['data'])
+ fdef.write(ret)
+
+ ret = generate_decl_enum('%sKind' % expr['union'], expr['data'].keys())
+ ret += generate_declaration(expr['union'], expr['data'])
+ fdecl.write(ret)
+ elif expr.has_key('enum'):
+ ret = generate_visit_list(expr['enum'], expr['data'])
+ ret += generate_visit_enum(expr['enum'], expr['data'])
+ fdef.write(ret)
+
+ ret = generate_decl_enum(expr['enum'], expr['data'])
+ ret += generate_enum_declaration(expr['enum'], expr['data'])
+ fdecl.write(ret)
+
+ fdecl.write('''
#endif
-''')
+ ''')
+
+ fdecl.flush()
+ fdecl.close()
-fdecl.flush()
-fdecl.close()
+ fdef.flush()
+ fdef.close()
-fdef.flush()
-fdef.close()
+if __name__ == '__main__':
+ sys.exit(main(sys.argv))
--
1.7.9.5
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PATCH 05/22] qapi: qapi_visit.py, support arrays and complex qapi definitions
2012-09-21 14:07 [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization Michael Roth
` (3 preceding siblings ...)
2012-09-21 14:07 ` [Qemu-devel] [PATCH 04/22] qapi: qapi_visit.py, make code useable as module Michael Roth
@ 2012-09-21 14:07 ` Michael Roth
2012-09-21 14:07 ` [Qemu-devel] [PATCH 06/22] qapi: qapi_visit.py, support generating static functions Michael Roth
` (17 subsequent siblings)
22 siblings, 0 replies; 39+ messages in thread
From: Michael Roth @ 2012-09-21 14:07 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, peter.maydell, aliguori, eblake
Add support for arrays in the code generators.
Complex field descriptions can now be used to provide additional
information to the visitor generators, such as the max size of an array,
or the field within a struct to use to determine how many elements are
present in the array to avoid serializing uninitialized elements.
Add handling for these in the code generators as well.
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
scripts/qapi.py | 8 ++++--
scripts/qapi_commands.py | 8 +++---
scripts/qapi_types.py | 2 +-
scripts/qapi_visit.py | 64 +++++++++++++++++++++++++++++++++++++++++-----
4 files changed, 68 insertions(+), 14 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 122b4cb..a347203 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -110,12 +110,16 @@ def parse_args(typeinfo):
argentry = typeinfo[member]
optional = False
structured = False
+ annotated = False
if member.startswith('*'):
argname = member[1:]
optional = True
if isinstance(argentry, OrderedDict):
- structured = True
- yield (argname, argentry, optional, structured)
+ if argentry.has_key('<annotated>'):
+ annotated = True
+ else:
+ structured = True
+ yield (argname, argentry, optional, structured, annotated)
def de_camel_case(name):
new_name = ''
diff --git a/scripts/qapi_commands.py b/scripts/qapi_commands.py
index 3c4678d..a2e0c23 100644
--- a/scripts/qapi_commands.py
+++ b/scripts/qapi_commands.py
@@ -32,7 +32,7 @@ void %(visitor)s(Visitor *m, %(name)s * obj, const char *name, Error **errp);
def generate_command_decl(name, args, ret_type):
arglist=""
- for argname, argtype, optional, structured in parse_args(args):
+ for argname, argtype, optional, structured, annotated in parse_args(args):
argtype = c_type(argtype)
if argtype == "char *":
argtype = "const char *"
@@ -50,7 +50,7 @@ def gen_sync_call(name, args, ret_type, indent=0):
retval=""
if ret_type:
retval = "retval = "
- for argname, argtype, optional, structured in parse_args(args):
+ for argname, argtype, optional, structured, annotated in parse_args(args):
if optional:
arglist += "has_%s, " % c_var(argname)
arglist += "%s, " % (c_var(argname))
@@ -106,7 +106,7 @@ Visitor *v;
def gen_visitor_input_vars_decl(args):
ret = ""
push_indent()
- for argname, argtype, optional, structured in parse_args(args):
+ for argname, argtype, optional, structured, annotated in parse_args(args):
if optional:
ret += mcgen('''
bool has_%(argname)s = false;
@@ -145,7 +145,7 @@ v = qmp_input_get_visitor(mi);
''',
obj=obj)
- for argname, argtype, optional, structured in parse_args(args):
+ for argname, argtype, optional, structured, annotated in parse_args(args):
if optional:
ret += mcgen('''
visit_start_optional(v, &has_%(c_name)s, "%(name)s", errp);
diff --git a/scripts/qapi_types.py b/scripts/qapi_types.py
index 49ef569..6518768 100644
--- a/scripts/qapi_types.py
+++ b/scripts/qapi_types.py
@@ -45,7 +45,7 @@ struct %(name)s
''',
name=structname)
- for argname, argentry, optional, structured in parse_args(members):
+ for argname, argentry, optional, structured, annotated in parse_args(members):
if optional:
ret += mcgen('''
bool has_%(c_name)s;
diff --git a/scripts/qapi_visit.py b/scripts/qapi_visit.py
index 974e458..b3f34a2 100644
--- a/scripts/qapi_visit.py
+++ b/scripts/qapi_visit.py
@@ -16,6 +16,52 @@ import sys
import os
import getopt
import errno
+import types
+
+def generate_visit_carray_body(name, info):
+ if info['array_size'][0].isdigit():
+ array_size = info['array_size']
+ elif info['array_size'][0] == '(' and info['array_size'][-1] == ')':
+ array_size = info['array_size']
+ else:
+ array_size = "(*obj)->%s" % info['array_size']
+
+ if info.has_key('array_capacity'):
+ array_capacity = info['array_capacity']
+ else:
+ array_capacity = array_size
+
+ if camel_case(de_camel_case(info['type'][0])) == info['type'][0]:
+ ret = mcgen('''
+visit_start_carray(m, (void **)obj, "%(name)s", %(array_capacity)s, sizeof(%(type)s), errp);
+int %(name)s_i;
+for (%(name)s_i = 0; %(name)s_i < %(array_size)s; %(name)s_i++) {
+ %(type)s %(name)s_ptr = &(*obj)->%(name)s[%(name)s_i];
+ visit_next_carray(m, errp);
+ visit_type_%(type_short)s(m, &%(name)s_ptr, NULL, errp);
+}
+visit_end_carray(m, errp);
+''',
+ name=name, type=c_type(info['type'][0]),
+ type_short=info['type'][0],
+ array_size=array_size,
+ array_capacity=array_capacity)
+ else:
+ ret = mcgen('''
+visit_start_carray(m, (void **)obj, "%(name)s", %(array_capacity)s, sizeof(%(type)s), errp);
+int %(name)s_i;
+for (%(name)s_i = 0; %(name)s_i < %(array_size)s; %(name)s_i++) {
+ %(type)s *%(name)s_ptr = (%(type)s *)&(*obj)->%(name)s[%(name)s_i];
+ visit_next_carray(m, errp);
+ visit_type_%(type_short)s(m, %(name)s_ptr, NULL, errp);
+}
+visit_end_carray(m, errp);
+''',
+ name=name, type=c_type(info['type'][0]),
+ type_short=info['type'][0],
+ array_size=array_size,
+ array_capacity=array_capacity)
+ return ret
def generate_visit_struct_body(field_prefix, name, members):
ret = mcgen('''
@@ -45,10 +91,10 @@ if (!err) {
push_indent()
push_indent()
- for argname, argentry, optional, structured in parse_args(members):
+ for argname, argentry, optional, structured, annotated in parse_args(members):
if optional:
ret += mcgen('''
-visit_start_optional(m, obj ? &(*obj)->%(c_prefix)shas_%(c_name)s : NULL, "%(name)s", &err);
+visit_start_optional(m, obj ? &(*obj)->%(c_prefix)shas_%(c_name)s : NULL, "%(name)s", errp);
if (obj && (*obj)->%(prefix)shas_%(c_name)s) {
''',
c_prefix=c_var(field_prefix), prefix=field_prefix,
@@ -58,12 +104,16 @@ if (obj && (*obj)->%(prefix)shas_%(c_name)s) {
if structured:
ret += generate_visit_struct_body(field_prefix + argname, argname, argentry)
else:
- ret += mcgen('''
-visit_type_%(type)s(m, obj ? &(*obj)->%(c_prefix)s%(c_name)s : NULL, "%(name)s", &err);
+ if annotated:
+ if isinstance(argentry['type'], types.ListType):
+ ret += generate_visit_carray_body(argname, argentry)
+ else:
+ ret += mcgen('''
+visit_type_%(type)s(m, obj ? &(*obj)->%(c_prefix)s%(c_name)s : NULL, "%(name)s", errp);
''',
- c_prefix=c_var(field_prefix), prefix=field_prefix,
- type=type_name(argentry), c_name=c_var(argname),
- name=argname)
+ c_prefix=c_var(field_prefix), prefix=field_prefix,
+ type=type_name(argentry), c_name=c_var(argname),
+ name=argname)
if optional:
pop_indent()
--
1.7.9.5
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PATCH 06/22] qapi: qapi_visit.py, support generating static functions
2012-09-21 14:07 [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization Michael Roth
` (4 preceding siblings ...)
2012-09-21 14:07 ` [Qemu-devel] [PATCH 05/22] qapi: qapi_visit.py, support arrays and complex qapi definitions Michael Roth
@ 2012-09-21 14:07 ` Michael Roth
2012-09-21 14:07 ` [Qemu-devel] [PATCH 07/22] qapi: qapi_visit.py, support for visiting non-pointer/embedded structs Michael Roth
` (16 subsequent siblings)
22 siblings, 0 replies; 39+ messages in thread
From: Michael Roth @ 2012-09-21 14:07 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, peter.maydell, aliguori, eblake
qidl embeds visitor code into object files rather than linking against
seperate files, so allow for static declarations when we're using
qapi_visit.py as a library as we do with qidl.py
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
scripts/qapi_visit.py | 51 ++++++++++++++++++++++++++++++++-----------------
1 file changed, 33 insertions(+), 18 deletions(-)
diff --git a/scripts/qapi_visit.py b/scripts/qapi_visit.py
index b3f34a2..af23770 100644
--- a/scripts/qapi_visit.py
+++ b/scripts/qapi_visit.py
@@ -141,13 +141,16 @@ visit_end_optional(m, &err);
''')
return ret
-def generate_visit_struct(name, members):
+def generate_visit_struct(name, members, static=False):
+ ret_type = "void"
+ if static:
+ ret_type = "static " + ret_type
ret = mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **errp)
+%(ret_type)s visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **errp)
{
''',
- name=name)
+ name=name, ret_type=ret_type)
push_indent()
ret += generate_visit_struct_body("", name, members)
@@ -158,10 +161,13 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
''')
return ret
-def generate_visit_list(name, members):
+def generate_visit_list(name, members, static=False):
+ ret_type = "void"
+ if static:
+ ret_type = "static " + ret_type
return mcgen('''
-void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char *name, Error **errp)
+%(ret_type)s visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char *name, Error **errp)
{
GenericList *i, **prev = (GenericList **)obj;
Error *err = NULL;
@@ -183,19 +189,22 @@ void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char *name,
}
}
''',
- name=name)
+ name=name, ret_type=ret_type)
-def generate_visit_enum(name, members):
+def generate_visit_enum(name, members, static=False):
+ ret_type = "void"
+ if static:
+ ret_type = "static " + ret_type
return mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name, Error **errp)
+%(ret_type)s visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name, Error **errp)
{
visit_type_enum(m, (int *)obj, %(name)s_lookup, "%(name)s", name, errp);
}
''',
- name=name)
+ name=name, ret_type=ret_type)
-def generate_visit_union(name, members):
+def generate_visit_union(name, members, static=False):
ret = generate_visit_enum('%sKind' % name, members.keys())
ret += mcgen('''
@@ -252,18 +261,21 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
return ret
-def generate_declaration(name, members, genlist=True):
+def generate_declaration(name, members, genlist=True, static=False):
+ ret_type = "void"
+ if static:
+ ret_type = "static " + ret_type
ret = mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **errp);
+%(ret_type)s visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **errp);
''',
- name=name)
+ name=name, ret_type=ret_type)
if genlist:
ret += mcgen('''
-void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char *name, Error **errp);
+%(ret_type)s visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char *name, Error **errp);
''',
- name=name)
+ name=name, ret_type=ret_type)
return ret
@@ -277,12 +289,15 @@ void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char *name,
return ret
-def generate_decl_enum(name, members, genlist=True):
+def generate_decl_enum(name, members, genlist=True, static=False):
+ ret_type = "void"
+ if static:
+ ret_type = "static " + ret_type
return mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name, Error **errp);
+%(ret_type)s visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name, Error **errp);
''',
- name=name)
+ name=name, ret_type=ret_type)
def main(argv=[]):
try:
--
1.7.9.5
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PATCH 07/22] qapi: qapi_visit.py, support for visiting non-pointer/embedded structs
2012-09-21 14:07 [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization Michael Roth
` (5 preceding siblings ...)
2012-09-21 14:07 ` [Qemu-devel] [PATCH 06/22] qapi: qapi_visit.py, support generating static functions Michael Roth
@ 2012-09-21 14:07 ` Michael Roth
2012-09-21 14:07 ` [Qemu-devel] [PATCH 08/22] qapi: add visitor interfaces for C arrays Michael Roth
` (15 subsequent siblings)
22 siblings, 0 replies; 39+ messages in thread
From: Michael Roth @ 2012-09-21 14:07 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, peter.maydell, aliguori, eblake
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
scripts/qapi_visit.py | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/scripts/qapi_visit.py b/scripts/qapi_visit.py
index af23770..ebff0f0 100644
--- a/scripts/qapi_visit.py
+++ b/scripts/qapi_visit.py
@@ -107,6 +107,15 @@ if (obj && (*obj)->%(prefix)shas_%(c_name)s) {
if annotated:
if isinstance(argentry['type'], types.ListType):
ret += generate_visit_carray_body(argname, argentry)
+ elif argentry.has_key('<embedded struct>'):
+ tmp_ptr_name = "%s_%s_ptr" % (c_var(field_prefix).replace(".", ""), c_var(argname))
+ ret += mcgen('''
+%(type)s *%(tmp_ptr)s = &(*obj)->%(c_prefix)s%(c_name)s;
+visit_type_%(type)s(m, (obj && *obj) ? &%(tmp_ptr)s : NULL, "%(name)s", errp);
+''',
+ c_prefix=c_var(field_prefix), prefix=field_prefix,
+ type=type_name(argentry['type']), c_name=c_var(argname),
+ name=argname, tmp_ptr=tmp_ptr_name)
else:
ret += mcgen('''
visit_type_%(type)s(m, obj ? &(*obj)->%(c_prefix)s%(c_name)s : NULL, "%(name)s", errp);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PATCH 08/22] qapi: add visitor interfaces for C arrays
2012-09-21 14:07 [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization Michael Roth
` (6 preceding siblings ...)
2012-09-21 14:07 ` [Qemu-devel] [PATCH 07/22] qapi: qapi_visit.py, support for visiting non-pointer/embedded structs Michael Roth
@ 2012-09-21 14:07 ` Michael Roth
2012-09-21 14:07 ` [Qemu-devel] [PATCH 09/22] qapi: QmpOutputVisitor, implement array handling Michael Roth
` (14 subsequent siblings)
22 siblings, 0 replies; 39+ messages in thread
From: Michael Roth @ 2012-09-21 14:07 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, peter.maydell, aliguori, eblake
Generally these will be serialized into lists, but the
representation can be of any form so long as it can
be deserialized into a single-dimension C array.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qapi/qapi-visit-core.c | 25 +++++++++++++++++++++++++
qapi/qapi-visit-core.h | 8 ++++++++
2 files changed, 33 insertions(+)
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 7a82b63..9a74ed0 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -311,3 +311,28 @@ void input_type_enum(Visitor *v, int *obj, const char *strings[],
g_free(enum_str);
*obj = value;
}
+
+void visit_start_carray(Visitor *v, void **obj, const char *name,
+ size_t elem_count, size_t elem_size, Error **errp)
+{
+ g_assert(v->start_carray);
+ if (!error_is_set(errp)) {
+ v->start_carray(v, obj, name, elem_count, elem_size, errp);
+ }
+}
+
+void visit_next_carray(Visitor *v, Error **errp)
+{
+ g_assert(v->next_carray);
+ if (!error_is_set(errp)) {
+ v->next_carray(v, errp);
+ }
+}
+
+void visit_end_carray(Visitor *v, Error **errp)
+{
+ g_assert(v->end_carray);
+ if (!error_is_set(errp)) {
+ v->end_carray(v, errp);
+ }
+}
diff --git a/qapi/qapi-visit-core.h b/qapi/qapi-visit-core.h
index 60aceda..5eb1616 100644
--- a/qapi/qapi-visit-core.h
+++ b/qapi/qapi-visit-core.h
@@ -43,6 +43,10 @@ struct Visitor
void (*type_str)(Visitor *v, char **obj, const char *name, Error **errp);
void (*type_number)(Visitor *v, double *obj, const char *name,
Error **errp);
+ void (*start_carray)(Visitor *v, void **obj, const char *name,
+ size_t elem_count, size_t elem_size, Error **errp);
+ void (*next_carray)(Visitor *v, Error **errp);
+ void (*end_carray)(Visitor *v, Error **errp);
/* May be NULL */
void (*start_optional)(Visitor *v, bool *present, const char *name,
@@ -91,5 +95,9 @@ void visit_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp);
void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp);
+void visit_start_carray(Visitor *v, void **obj, const char *name,
+ size_t elem_count, size_t elem_size, Error **errp);
+void visit_next_carray(Visitor *v, Error **errp);
+void visit_end_carray(Visitor *v, Error **errp);
#endif
--
1.7.9.5
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PATCH 09/22] qapi: QmpOutputVisitor, implement array handling
2012-09-21 14:07 [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization Michael Roth
` (7 preceding siblings ...)
2012-09-21 14:07 ` [Qemu-devel] [PATCH 08/22] qapi: add visitor interfaces for C arrays Michael Roth
@ 2012-09-21 14:07 ` Michael Roth
2012-09-21 14:07 ` [Qemu-devel] [PATCH 10/22] qapi: QmpInputVisitor, " Michael Roth
` (13 subsequent siblings)
22 siblings, 0 replies; 39+ messages in thread
From: Michael Roth @ 2012-09-21 14:07 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, peter.maydell, aliguori, eblake
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qapi/qmp-output-visitor.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
index 2bce9d5..ea08795 100644
--- a/qapi/qmp-output-visitor.c
+++ b/qapi/qmp-output-visitor.c
@@ -181,6 +181,23 @@ static void qmp_output_type_number(Visitor *v, double *obj, const char *name,
qmp_output_add(qov, name, qfloat_from_double(*obj));
}
+static void qmp_output_start_carray(Visitor *v, void **obj, const char *name,
+ size_t elem_count, size_t elem_size,
+ Error **errp)
+{
+ qmp_output_start_list(v, name, errp);
+}
+
+static void qmp_output_next_carray(Visitor *v, Error **errp)
+{
+}
+
+static void qmp_output_end_carray(Visitor *v, Error **errp)
+{
+ QmpOutputVisitor *qov = to_qov(v);
+ qmp_output_pop(qov);
+}
+
QObject *qmp_output_get_qobject(QmpOutputVisitor *qov)
{
QObject *obj = qmp_output_first(qov);
@@ -228,6 +245,9 @@ QmpOutputVisitor *qmp_output_visitor_new(void)
v->visitor.type_bool = qmp_output_type_bool;
v->visitor.type_str = qmp_output_type_str;
v->visitor.type_number = qmp_output_type_number;
+ v->visitor.start_carray = qmp_output_start_carray;
+ v->visitor.next_carray = qmp_output_next_carray;
+ v->visitor.end_carray = qmp_output_end_carray;
QTAILQ_INIT(&v->stack);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PATCH 10/22] qapi: QmpInputVisitor, implement array handling
2012-09-21 14:07 [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization Michael Roth
` (8 preceding siblings ...)
2012-09-21 14:07 ` [Qemu-devel] [PATCH 09/22] qapi: QmpOutputVisitor, implement array handling Michael Roth
@ 2012-09-21 14:07 ` Michael Roth
2012-09-21 14:07 ` [Qemu-devel] [PATCH 11/22] qapi: qapi.py, make json parser more robust Michael Roth
` (12 subsequent siblings)
22 siblings, 0 replies; 39+ messages in thread
From: Michael Roth @ 2012-09-21 14:07 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, peter.maydell, aliguori, eblake
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qapi/qmp-input-visitor.c | 32 +++++++++++++++++++++++++++++++-
1 file changed, 31 insertions(+), 1 deletion(-)
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 107d8d3..c4388f3 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -132,7 +132,7 @@ static void qmp_input_start_struct(Visitor *v, void **obj, const char *kind,
return;
}
- if (obj) {
+ if (obj && *obj == NULL) {
*obj = g_malloc0(size);
}
}
@@ -274,6 +274,33 @@ static void qmp_input_start_optional(Visitor *v, bool *present,
*present = true;
}
+static void qmp_input_start_carray(Visitor *v, void **obj, const char *name,
+ size_t elem_count, size_t elem_size,
+ Error **errp)
+{
+ if (obj && *obj == NULL) {
+ *obj = g_malloc0(elem_count * elem_size);
+ }
+ qmp_input_start_list(v, name, errp);
+}
+
+static void qmp_input_next_carray(Visitor *v, Error **errp)
+{
+ QmpInputVisitor *qiv = to_qiv(v);
+ StackObject *so = &qiv->stack[qiv->nb_stack - 1];
+
+ if (so->entry == NULL) {
+ so->entry = qlist_first(qobject_to_qlist(so->obj));
+ } else {
+ so->entry = qlist_next(so->entry);
+ }
+}
+
+static void qmp_input_end_carray(Visitor *v, Error **errp)
+{
+ qmp_input_end_list(v, errp);
+}
+
Visitor *qmp_input_get_visitor(QmpInputVisitor *v)
{
return &v->visitor;
@@ -302,6 +329,9 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj)
v->visitor.type_str = qmp_input_type_str;
v->visitor.type_number = qmp_input_type_number;
v->visitor.start_optional = qmp_input_start_optional;
+ v->visitor.start_carray = qmp_input_start_carray;
+ v->visitor.next_carray = qmp_input_next_carray;
+ v->visitor.end_carray = qmp_input_end_carray;
qmp_input_push(v, obj, NULL);
qobject_incref(obj);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PATCH 11/22] qapi: qapi.py, make json parser more robust
2012-09-21 14:07 [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization Michael Roth
` (9 preceding siblings ...)
2012-09-21 14:07 ` [Qemu-devel] [PATCH 10/22] qapi: QmpInputVisitor, " Michael Roth
@ 2012-09-21 14:07 ` Michael Roth
2012-09-21 14:07 ` [Qemu-devel] [PATCH 12/22] qapi: add open-coded visitor for struct tm types Michael Roth
` (11 subsequent siblings)
22 siblings, 0 replies; 39+ messages in thread
From: Michael Roth @ 2012-09-21 14:07 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, peter.maydell, aliguori, eblake
Currently the QAPI JSON parser expects a very particular style of code
indentation, the major one being that terminating curly/square brackets are
not on placed on a seperate line. This is incompatible with most
pretty-print formats, so make it a little more robust by supporting
these cases.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
scripts/qapi.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index a347203..47cd672 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -81,7 +81,7 @@ def parse_schema(fp):
if line.startswith('#') or line == '\n':
continue
- if line.startswith(' '):
+ if line[0] in ['}', ']', ' ', '\t']:
expr += line
elif expr:
expr_eval = evaluate(expr)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PATCH 12/22] qapi: add open-coded visitor for struct tm types
2012-09-21 14:07 [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization Michael Roth
` (10 preceding siblings ...)
2012-09-21 14:07 ` [Qemu-devel] [PATCH 11/22] qapi: qapi.py, make json parser more robust Michael Roth
@ 2012-09-21 14:07 ` Michael Roth
2012-09-21 14:07 ` [Qemu-devel] [PATCH 13/22] qom-fuse: force single-threaded mode to avoid QMP races Michael Roth
` (10 subsequent siblings)
22 siblings, 0 replies; 39+ messages in thread
From: Michael Roth @ 2012-09-21 14:07 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, peter.maydell, aliguori, eblake
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qapi/Makefile.objs | 1 +
qapi/misc-qapi-visit.c | 14 ++++++++++++++
qapi/qapi-visit-core.h | 3 +++
3 files changed, 18 insertions(+)
create mode 100644 qapi/misc-qapi-visit.c
diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs
index 5f5846e..7604b52 100644
--- a/qapi/Makefile.objs
+++ b/qapi/Makefile.objs
@@ -1,3 +1,4 @@
qapi-obj-y = qapi-visit-core.o qapi-dealloc-visitor.o qmp-input-visitor.o
qapi-obj-y += qmp-output-visitor.o qmp-registry.o qmp-dispatch.o
qapi-obj-y += string-input-visitor.o string-output-visitor.o opts-visitor.o
+qapi-obj-y += misc-qapi-visit.o
diff --git a/qapi/misc-qapi-visit.c b/qapi/misc-qapi-visit.c
new file mode 100644
index 0000000..a44773d
--- /dev/null
+++ b/qapi/misc-qapi-visit.c
@@ -0,0 +1,14 @@
+#include <time.h>
+#include "qidl.h"
+
+void visit_type_tm(Visitor *v, struct tm *obj, const char *name, Error **errp)
+{
+ visit_start_struct(v, NULL, "struct tm", name, 0, errp);
+ visit_type_int32(v, &obj->tm_year, "tm_year", errp);
+ visit_type_int32(v, &obj->tm_mon, "tm_mon", errp);
+ visit_type_int32(v, &obj->tm_mday, "tm_mday", errp);
+ visit_type_int32(v, &obj->tm_hour, "tm_hour", errp);
+ visit_type_int32(v, &obj->tm_min, "tm_min", errp);
+ visit_type_int32(v, &obj->tm_sec, "tm_sec", errp);
+ visit_end_struct(v, errp);
+}
diff --git a/qapi/qapi-visit-core.h b/qapi/qapi-visit-core.h
index 5eb1616..10ec044 100644
--- a/qapi/qapi-visit-core.h
+++ b/qapi/qapi-visit-core.h
@@ -100,4 +100,7 @@ void visit_start_carray(Visitor *v, void **obj, const char *name,
void visit_next_carray(Visitor *v, Error **errp);
void visit_end_carray(Visitor *v, Error **errp);
+/* misc. visitors */
+void visit_type_tm(Visitor *m, struct tm *obj, const char *name, Error **errp);
+
#endif
--
1.7.9.5
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PATCH 13/22] qom-fuse: force single-threaded mode to avoid QMP races
2012-09-21 14:07 [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization Michael Roth
` (11 preceding siblings ...)
2012-09-21 14:07 ` [Qemu-devel] [PATCH 12/22] qapi: add open-coded visitor for struct tm types Michael Roth
@ 2012-09-21 14:07 ` Michael Roth
2012-09-21 14:07 ` [Qemu-devel] [PATCH 14/22] qom-fuse: workaround for truncated properties > 4096 Michael Roth
` (9 subsequent siblings)
22 siblings, 0 replies; 39+ messages in thread
From: Michael Roth @ 2012-09-21 14:07 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, peter.maydell, aliguori, eblake
python-fuse defaults to multi-threaded handling of filesystem calls.
In the case of QOM this can lead to threads reading QMP responses to
requests executed by other threads, causing all sorts of strangeness
when interacting with QOM mounts. For instance:
mdroth@loki:~/w/qom$ ls -l machine | grep type
ls: error while loading shared libraries: tls/libacl.so.1: \
cannot read file data: Error 21
We fix this by unconditionally passing in the -s option, which forces
single-threaded/serialized execution of filesystem calls. This flag is
only honored if we pass dash_s_do='setsingle' to the Fuse class
constructor and use the parse() method of that class to pass in
arguments, so we do that here as well.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
QMP/qom-fuse | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/QMP/qom-fuse b/QMP/qom-fuse
index 5c6754a..b4a4eb3 100755
--- a/QMP/qom-fuse
+++ b/QMP/qom-fuse
@@ -134,5 +134,16 @@ class QOMFS(Fuse):
if __name__ == '__main__':
import sys, os
- fs = QOMFS(QEMUMonitorProtocol(os.environ['QMP_SOCKET']))
- fs.main(sys.argv)
+ fuse_flags = list(sys.argv)
+ mount_point = None
+
+ if not fuse_flags[-1].startswith('-'):
+ mount_point = fuse_flags.pop()
+
+ # force single-threaded behavior to avoid races for QMP responses
+ if not '-s' in fuse_flags:
+ fuse_flags.append('-s')
+
+ fs = QOMFS(QEMUMonitorProtocol(os.environ['QMP_SOCKET']), dash_s_do='setsingle')
+ fs.parse(fuse_flags + (mount_point and [mount_point] or []))
+ fs.main()
--
1.7.9.5
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PATCH 14/22] qom-fuse: workaround for truncated properties > 4096
2012-09-21 14:07 [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization Michael Roth
` (12 preceding siblings ...)
2012-09-21 14:07 ` [Qemu-devel] [PATCH 13/22] qom-fuse: force single-threaded mode to avoid QMP races Michael Roth
@ 2012-09-21 14:07 ` Michael Roth
2012-09-21 14:07 ` [Qemu-devel] [PATCH 15/22] module additions for schema registration Michael Roth
` (8 subsequent siblings)
22 siblings, 0 replies; 39+ messages in thread
From: Michael Roth @ 2012-09-21 14:07 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, peter.maydell, aliguori, eblake
We currently hard-code property size at 4096 for the purposes of
getattr()/stat()/etc. For 'state' properties we can exceed this easily,
leading to truncated responses.
Instead, for a particular property, make it
max(4096, most_recent_property_size * 2). This allows some
head-room for properties that change size periodically (numbers,
strings, state properties containing arrays, etc)
Also, implement a simple property cache to avoid spinning on qom-get
if an application reads beyond the actual size. This also allows us
to use a snapshot of a single qom-get that persists across read()'s.
Old Cache entries are evicted as soon as we attempt to read() from
offset 0 again.
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
QMP/qom-fuse | 24 +++++++++++++++++-------
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/QMP/qom-fuse b/QMP/qom-fuse
index b4a4eb3..bbad0de 100755
--- a/QMP/qom-fuse
+++ b/QMP/qom-fuse
@@ -26,6 +26,7 @@ class QOMFS(Fuse):
self.qmp.connect()
self.ino_map = {}
self.ino_count = 1
+ self.prop_cache = {}
def get_ino(self, path):
if self.ino_map.has_key(path):
@@ -67,12 +68,16 @@ class QOMFS(Fuse):
if not self.is_property(path):
return -ENOENT
- path, prop = path.rsplit('/', 1)
- try:
- data = str(self.qmp.command('qom-get', path=path, property=prop))
- data += '\n' # make values shell friendly
- except:
- return -EPERM
+ # avoid extra calls to qom-get by using cached value when offset > 0
+ if offset == 0 or not self.prop_cache.has_key(path):
+ directory, prop = path.rsplit('/', 1)
+ try:
+ resp = str(self.qmp.command('qom-get', path=directory, property=prop))
+ self.prop_cache[path] = resp + '\n' # make values shell friendly
+ except:
+ return -EPERM
+
+ data = self.prop_cache[path]
if offset > len(data):
return ''
@@ -111,13 +116,18 @@ class QOMFS(Fuse):
0,
0))
elif self.is_property(path):
+ directory, prop = path.rsplit('/', 1)
+ try:
+ resp = str(self.qmp.command('qom-get', path=directory, property=prop))
+ except:
+ return -ENOENT
value = posix.stat_result((0644 | stat.S_IFREG,
self.get_ino(path),
0,
1,
1000,
1000,
- 4096,
+ max(len(resp) * 2, 4096),
0,
0,
0))
--
1.7.9.5
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PATCH 15/22] module additions for schema registration
2012-09-21 14:07 [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization Michael Roth
` (13 preceding siblings ...)
2012-09-21 14:07 ` [Qemu-devel] [PATCH 14/22] qom-fuse: workaround for truncated properties > 4096 Michael Roth
@ 2012-09-21 14:07 ` Michael Roth
2012-09-21 14:07 ` [Qemu-devel] [PATCH 16/22] qdev: move Property-related declarations to qdev-properties.h Michael Roth
` (7 subsequent siblings)
22 siblings, 0 replies; 39+ messages in thread
From: Michael Roth @ 2012-09-21 14:07 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, peter.maydell, aliguori, eblake
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
module.h | 2 ++
vl.c | 1 +
2 files changed, 3 insertions(+)
diff --git a/module.h b/module.h
index c4ccd57..cb81aa2 100644
--- a/module.h
+++ b/module.h
@@ -25,6 +25,7 @@ typedef enum {
MODULE_INIT_MACHINE,
MODULE_INIT_QAPI,
MODULE_INIT_QOM,
+ MODULE_INIT_QIDL,
MODULE_INIT_MAX
} module_init_type;
@@ -32,6 +33,7 @@ typedef enum {
#define machine_init(function) module_init(function, MODULE_INIT_MACHINE)
#define qapi_init(function) module_init(function, MODULE_INIT_QAPI)
#define type_init(function) module_init(function, MODULE_INIT_QOM)
+#define qidl_init(function) module_init(function, MODULE_INIT_QIDL)
void register_module_init(void (*fn)(void), module_init_type type);
diff --git a/vl.c b/vl.c
index 7c577fa..db50524 100644
--- a/vl.c
+++ b/vl.c
@@ -2392,6 +2392,7 @@ int main(int argc, char **argv, char **envp)
}
module_call_init(MODULE_INIT_QOM);
+ module_call_init(MODULE_INIT_QIDL);
runstate_init();
--
1.7.9.5
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PATCH 16/22] qdev: move Property-related declarations to qdev-properties.h
2012-09-21 14:07 [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization Michael Roth
` (14 preceding siblings ...)
2012-09-21 14:07 ` [Qemu-devel] [PATCH 15/22] module additions for schema registration Michael Roth
@ 2012-09-21 14:07 ` Michael Roth
2012-09-21 14:07 ` [Qemu-devel] [PATCH 17/22] qidl: add documentation Michael Roth
` (6 subsequent siblings)
22 siblings, 0 replies; 39+ messages in thread
From: Michael Roth @ 2012-09-21 14:07 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, peter.maydell, aliguori, eblake
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
hw/qdev-properties.h | 151 ++++++++++++++++++++++++++++++++++++++++++++++++++
hw/qdev.h | 126 +----------------------------------------
2 files changed, 152 insertions(+), 125 deletions(-)
create mode 100644 hw/qdev-properties.h
diff --git a/hw/qdev-properties.h b/hw/qdev-properties.h
new file mode 100644
index 0000000..ce79a79
--- /dev/null
+++ b/hw/qdev-properties.h
@@ -0,0 +1,151 @@
+/*
+ * Property definitions for qdev
+ *
+ * Copyright (c) 2009 CodeSourcery
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef QDEV_PROPERTIES_H
+#define QDEV_PROPERTIES_H
+
+#include "qemu/object.h"
+#include "qemu-queue.h"
+
+typedef struct Property Property;
+
+typedef struct PropertyInfo PropertyInfo;
+
+typedef struct CompatProperty CompatProperty;
+
+struct Property {
+ const char *name;
+ PropertyInfo *info;
+ int offset;
+ uint8_t bitnr;
+ uint8_t qtype;
+ int64_t defval;
+};
+
+struct PropertyInfo {
+ const char *name;
+ const char *legacy_name;
+ const char **enum_table;
+ int (*parse)(DeviceState *dev, Property *prop, const char *str);
+ int (*print)(DeviceState *dev, Property *prop, char *dest, size_t len);
+ ObjectPropertyAccessor *get;
+ ObjectPropertyAccessor *set;
+ ObjectPropertyRelease *release;
+};
+
+typedef struct GlobalProperty {
+ const char *driver;
+ const char *property;
+ const char *value;
+ QTAILQ_ENTRY(GlobalProperty) next;
+} GlobalProperty;
+
+extern PropertyInfo qdev_prop_bit;
+extern PropertyInfo qdev_prop_uint8;
+extern PropertyInfo qdev_prop_uint16;
+extern PropertyInfo qdev_prop_uint32;
+extern PropertyInfo qdev_prop_int32;
+extern PropertyInfo qdev_prop_uint64;
+extern PropertyInfo qdev_prop_hex8;
+extern PropertyInfo qdev_prop_hex32;
+extern PropertyInfo qdev_prop_hex64;
+extern PropertyInfo qdev_prop_string;
+extern PropertyInfo qdev_prop_chr;
+extern PropertyInfo qdev_prop_ptr;
+extern PropertyInfo qdev_prop_macaddr;
+extern PropertyInfo qdev_prop_losttickpolicy;
+extern PropertyInfo qdev_prop_bios_chs_trans;
+extern PropertyInfo qdev_prop_drive;
+extern PropertyInfo qdev_prop_netdev;
+extern PropertyInfo qdev_prop_vlan;
+extern PropertyInfo qdev_prop_pci_devfn;
+extern PropertyInfo qdev_prop_blocksize;
+extern PropertyInfo qdev_prop_pci_host_devaddr;
+
+#define DEFINE_PROP(_name, _state, _field, _prop, _type) { \
+ .name = (_name), \
+ .info = &(_prop), \
+ .offset = offsetof(_state, _field) \
+ + type_check(_type,typeof_field(_state, _field)), \
+ }
+#define DEFINE_PROP_DEFAULT(_name, _state, _field, _defval, _prop, _type) { \
+ .name = (_name), \
+ .info = &(_prop), \
+ .offset = offsetof(_state, _field) \
+ + type_check(_type,typeof_field(_state, _field)), \
+ .qtype = QTYPE_QINT, \
+ .defval = (_type)_defval, \
+ }
+#define DEFINE_PROP_BIT(_name, _state, _field, _bit, _defval) { \
+ .name = (_name), \
+ .info = &(qdev_prop_bit), \
+ .bitnr = (_bit), \
+ .offset = offsetof(_state, _field) \
+ + type_check(uint32_t,typeof_field(_state, _field)), \
+ .qtype = QTYPE_QBOOL, \
+ .defval = (bool)_defval, \
+ }
+
+#define DEFINE_PROP_UINT8(_n, _s, _f, _d) \
+ DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint8, uint8_t)
+#define DEFINE_PROP_UINT16(_n, _s, _f, _d) \
+ DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint16, uint16_t)
+#define DEFINE_PROP_UINT32(_n, _s, _f, _d) \
+ DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint32, uint32_t)
+#define DEFINE_PROP_INT32(_n, _s, _f, _d) \
+ DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_int32, int32_t)
+#define DEFINE_PROP_UINT64(_n, _s, _f, _d) \
+ DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint64, uint64_t)
+#define DEFINE_PROP_HEX8(_n, _s, _f, _d) \
+ DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_hex8, uint8_t)
+#define DEFINE_PROP_HEX32(_n, _s, _f, _d) \
+ DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_hex32, uint32_t)
+#define DEFINE_PROP_HEX64(_n, _s, _f, _d) \
+ DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_hex64, uint64_t)
+#define DEFINE_PROP_PCI_DEVFN(_n, _s, _f, _d) \
+ DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_pci_devfn, int32_t)
+
+#define DEFINE_PROP_PTR(_n, _s, _f) \
+ DEFINE_PROP(_n, _s, _f, qdev_prop_ptr, void*)
+#define DEFINE_PROP_CHR(_n, _s, _f) \
+ DEFINE_PROP(_n, _s, _f, qdev_prop_chr, CharDriverState*)
+#define DEFINE_PROP_STRING(_n, _s, _f) \
+ DEFINE_PROP(_n, _s, _f, qdev_prop_string, char*)
+#define DEFINE_PROP_NETDEV(_n, _s, _f) \
+ DEFINE_PROP(_n, _s, _f, qdev_prop_netdev, NetClientState*)
+#define DEFINE_PROP_VLAN(_n, _s, _f) \
+ DEFINE_PROP(_n, _s, _f, qdev_prop_vlan, NetClientState*)
+#define DEFINE_PROP_DRIVE(_n, _s, _f) \
+ DEFINE_PROP(_n, _s, _f, qdev_prop_drive, BlockDriverState *)
+#define DEFINE_PROP_MACADDR(_n, _s, _f) \
+ DEFINE_PROP(_n, _s, _f, qdev_prop_macaddr, MACAddr)
+#define DEFINE_PROP_LOSTTICKPOLICY(_n, _s, _f, _d) \
+ DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_losttickpolicy, \
+ LostTickPolicy)
+#define DEFINE_PROP_BIOS_CHS_TRANS(_n, _s, _f, _d) \
+ DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_bios_chs_trans, int)
+#define DEFINE_PROP_BLOCKSIZE(_n, _s, _f, _d) \
+ DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_blocksize, uint16_t)
+#define DEFINE_PROP_PCI_HOST_DEVADDR(_n, _s, _f) \
+ DEFINE_PROP(_n, _s, _f, qdev_prop_pci_host_devaddr, PCIHostDeviceAddress)
+
+#define DEFINE_PROP_END_OF_LIST() \
+ {}
+
+#endif
diff --git a/hw/qdev.h b/hw/qdev.h
index d699194..5e8fb21 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -8,10 +8,7 @@
#include "qapi/qapi-visit-core.h"
#include "qemu/object.h"
#include "error.h"
-
-typedef struct Property Property;
-
-typedef struct PropertyInfo PropertyInfo;
+#include "qdev-properties.h"
typedef struct CompatProperty CompatProperty;
@@ -122,33 +119,6 @@ struct BusState {
QLIST_ENTRY(BusState) sibling;
};
-struct Property {
- const char *name;
- PropertyInfo *info;
- int offset;
- uint8_t bitnr;
- uint8_t qtype;
- int64_t defval;
-};
-
-struct PropertyInfo {
- const char *name;
- const char *legacy_name;
- const char **enum_table;
- int (*parse)(DeviceState *dev, Property *prop, const char *str);
- int (*print)(DeviceState *dev, Property *prop, char *dest, size_t len);
- ObjectPropertyAccessor *get;
- ObjectPropertyAccessor *set;
- ObjectPropertyRelease *release;
-};
-
-typedef struct GlobalProperty {
- const char *driver;
- const char *property;
- const char *value;
- QTAILQ_ENTRY(GlobalProperty) next;
-} GlobalProperty;
-
/*** Board API. This should go away once we have a machine config file. ***/
DeviceState *qdev_create(BusState *bus, const char *name);
@@ -215,100 +185,6 @@ void do_info_qdm(Monitor *mon);
int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data);
int do_device_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
-/*** qdev-properties.c ***/
-
-extern PropertyInfo qdev_prop_bit;
-extern PropertyInfo qdev_prop_uint8;
-extern PropertyInfo qdev_prop_uint16;
-extern PropertyInfo qdev_prop_uint32;
-extern PropertyInfo qdev_prop_int32;
-extern PropertyInfo qdev_prop_uint64;
-extern PropertyInfo qdev_prop_hex8;
-extern PropertyInfo qdev_prop_hex32;
-extern PropertyInfo qdev_prop_hex64;
-extern PropertyInfo qdev_prop_string;
-extern PropertyInfo qdev_prop_chr;
-extern PropertyInfo qdev_prop_ptr;
-extern PropertyInfo qdev_prop_macaddr;
-extern PropertyInfo qdev_prop_losttickpolicy;
-extern PropertyInfo qdev_prop_bios_chs_trans;
-extern PropertyInfo qdev_prop_drive;
-extern PropertyInfo qdev_prop_netdev;
-extern PropertyInfo qdev_prop_vlan;
-extern PropertyInfo qdev_prop_pci_devfn;
-extern PropertyInfo qdev_prop_blocksize;
-extern PropertyInfo qdev_prop_pci_host_devaddr;
-
-#define DEFINE_PROP(_name, _state, _field, _prop, _type) { \
- .name = (_name), \
- .info = &(_prop), \
- .offset = offsetof(_state, _field) \
- + type_check(_type,typeof_field(_state, _field)), \
- }
-#define DEFINE_PROP_DEFAULT(_name, _state, _field, _defval, _prop, _type) { \
- .name = (_name), \
- .info = &(_prop), \
- .offset = offsetof(_state, _field) \
- + type_check(_type,typeof_field(_state, _field)), \
- .qtype = QTYPE_QINT, \
- .defval = (_type)_defval, \
- }
-#define DEFINE_PROP_BIT(_name, _state, _field, _bit, _defval) { \
- .name = (_name), \
- .info = &(qdev_prop_bit), \
- .bitnr = (_bit), \
- .offset = offsetof(_state, _field) \
- + type_check(uint32_t,typeof_field(_state, _field)), \
- .qtype = QTYPE_QBOOL, \
- .defval = (bool)_defval, \
- }
-
-#define DEFINE_PROP_UINT8(_n, _s, _f, _d) \
- DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint8, uint8_t)
-#define DEFINE_PROP_UINT16(_n, _s, _f, _d) \
- DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint16, uint16_t)
-#define DEFINE_PROP_UINT32(_n, _s, _f, _d) \
- DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint32, uint32_t)
-#define DEFINE_PROP_INT32(_n, _s, _f, _d) \
- DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_int32, int32_t)
-#define DEFINE_PROP_UINT64(_n, _s, _f, _d) \
- DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint64, uint64_t)
-#define DEFINE_PROP_HEX8(_n, _s, _f, _d) \
- DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_hex8, uint8_t)
-#define DEFINE_PROP_HEX32(_n, _s, _f, _d) \
- DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_hex32, uint32_t)
-#define DEFINE_PROP_HEX64(_n, _s, _f, _d) \
- DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_hex64, uint64_t)
-#define DEFINE_PROP_PCI_DEVFN(_n, _s, _f, _d) \
- DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_pci_devfn, int32_t)
-
-#define DEFINE_PROP_PTR(_n, _s, _f) \
- DEFINE_PROP(_n, _s, _f, qdev_prop_ptr, void*)
-#define DEFINE_PROP_CHR(_n, _s, _f) \
- DEFINE_PROP(_n, _s, _f, qdev_prop_chr, CharDriverState*)
-#define DEFINE_PROP_STRING(_n, _s, _f) \
- DEFINE_PROP(_n, _s, _f, qdev_prop_string, char*)
-#define DEFINE_PROP_NETDEV(_n, _s, _f) \
- DEFINE_PROP(_n, _s, _f, qdev_prop_netdev, NetClientState*)
-#define DEFINE_PROP_VLAN(_n, _s, _f) \
- DEFINE_PROP(_n, _s, _f, qdev_prop_vlan, NetClientState*)
-#define DEFINE_PROP_DRIVE(_n, _s, _f) \
- DEFINE_PROP(_n, _s, _f, qdev_prop_drive, BlockDriverState *)
-#define DEFINE_PROP_MACADDR(_n, _s, _f) \
- DEFINE_PROP(_n, _s, _f, qdev_prop_macaddr, MACAddr)
-#define DEFINE_PROP_LOSTTICKPOLICY(_n, _s, _f, _d) \
- DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_losttickpolicy, \
- LostTickPolicy)
-#define DEFINE_PROP_BIOS_CHS_TRANS(_n, _s, _f, _d) \
- DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_bios_chs_trans, int)
-#define DEFINE_PROP_BLOCKSIZE(_n, _s, _f, _d) \
- DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_blocksize, uint16_t)
-#define DEFINE_PROP_PCI_HOST_DEVADDR(_n, _s, _f) \
- DEFINE_PROP(_n, _s, _f, qdev_prop_pci_host_devaddr, PCIHostDeviceAddress)
-
-#define DEFINE_PROP_END_OF_LIST() \
- {}
-
/* Set properties between creation and init. */
void *qdev_get_prop_ptr(DeviceState *dev, Property *prop);
int qdev_prop_parse(DeviceState *dev, const char *name, const char *value);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PATCH 17/22] qidl: add documentation
2012-09-21 14:07 [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization Michael Roth
` (15 preceding siblings ...)
2012-09-21 14:07 ` [Qemu-devel] [PATCH 16/22] qdev: move Property-related declarations to qdev-properties.h Michael Roth
@ 2012-09-21 14:07 ` Michael Roth
2012-09-21 23:16 ` Eric Blake
2012-09-21 14:07 ` [Qemu-devel] [PATCH 18/22] qidl: add lexer library (based on QC parser) Michael Roth
` (5 subsequent siblings)
22 siblings, 1 reply; 39+ messages in thread
From: Michael Roth @ 2012-09-21 14:07 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, peter.maydell, aliguori, eblake
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
docs/qidl.txt | 347 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 347 insertions(+)
create mode 100644 docs/qidl.txt
diff --git a/docs/qidl.txt b/docs/qidl.txt
new file mode 100644
index 0000000..1cbf21f
--- /dev/null
+++ b/docs/qidl.txt
@@ -0,0 +1,347 @@
+How to Serialize Device State with QIDL
+======================================
+
+This document describes how to implement save/restore of a device in QEMU using
+the QIDL compiler. The QIDL compiler makes it easier to support live
+migration in devices by converging the serialization description with the
+device type declaration. It has the following features:
+
+ 1. Single description of device state and how to serialize
+
+ 2. Fully inclusive serialization description--fields that aren't serialized
+ are explicitly marked as such including the reason why.
+
+ 3. Optimized for the common case. Even without any special annotations,
+ many devices will Just Work out of the box.
+
+ 4. Build time schema definition. Since QIDL runs at build time, we have full
+ access to the schema during the build which means we can fail the build if
+ the schema breaks.
+
+For the rest of the document, the following simple device will be used as an
+example.
+
+ typedef struct SerialDevice {
+ SysBusDevice parent;
+
+ uint8_t thr; /* transmit holding register */
+ uint8_t lsr; /* line status register */
+ uint8_t ier; /* interrupt enable register */
+
+ int int_pending; /* whether we have a pending queued interrupt */
+ CharDriverState *chr; /* backend */
+ } SerialDevice;
+
+Getting Started
+---------------
+
+Coverting a device struct to being serializable by QIDL, in general, only
+requires the use of the QIDL_DECLARE() macro to handle the declaration.
+The above-mentioned SerialDevice struct, for instance, can be made
+serializable simply by using the following declaration format:
+
+ typedef struct SerialDevice SerialDevice;
+
+ QIDL_DECLARE(SerialDevice) {
+ SysBusDevice parent;
+
+ uint8_t thr; /* transmit holding register */
+ uint8_t lsr; /* line status register */
+ uint8_t ier; /* interrupt enable register */
+
+ int int_pending; /* whether we have a pending queued interrupt */
+ CharDriverState *chr; /* backend */
+ };
+
+Note that the typedef is required, and must be done in advance of the actual
+struct declaration.
+
+Specifying What/How State Gets Saved
+---------------------------------
+
+By default, QIDL saves every field in a structure it sees. This provides
+maximum correctness by default. However, device structures generally contain
+state that reflects state that is in someway duplicated or not guest visible.
+This more often that not reflects design implementation details.
+
+Since design implementation details change over time, saving this state makes
+compatibility hard to maintain. The proper solution is to use an intermediate
+protocol to handle cross-version compatibility (for instance, a QIDL-aware
+implementation of VMState). But we can reduce churn and streamline the
+serialization/deserialization process by explicitly marking fields with
+information that QIDL can use to determine whether or not a particular field
+will be serialized. However, a serializable device implementation that fails to
+serialize state that is required to fully guest state is a broken one, so to
+avoid that there are very strict rules about when this is allowed, and what
+needs to be done to ensure that this does not impact correctness.
+
+There are also occassions where we want to specify *how* a field is
+serialized. Array fields for instance might rely on a size value elsewhere in
+the struct to determine the size of a dynamically-allocated array or, in the
+case of a statically-allocated array, the number of elements that have
+actually be set/initialized. There also cases where a field should only be
+serialized if another value in the struct, or even a function call, indicates
+that the field has been initialized. Markers can be used to handle these
+types of cases as well.
+
+What follows is a description of these markers/annotations and how they are
+used.
+
+## qImmutable Fields
+
+If a field is only set during device construction, based on parameters passed to
+the device's constructor, then there is no need to send save and restore this
+value. We call these fields immutable and we tell QIDL about this fact by using
+a **qImmutable** marker.
+
+In our *SerialDevice* example, the *CharDriverState* pointer reflects the host
+backend that we use to send serial output to the user. This is only assigned
+during device construction and never changes. This means we can add an
+**immutable** marker to it:
+
+ QIDL_DECLARE(SerialDevice) {
+ SysBusDevice parent;
+
+ uint8_t thr;
+ uint8_t lsr;
+ uint8_t ier;
+
+ int int_pending;
+ CharDriverState *chr qImmutable;
+ };
+
+When reviewing patches that make use of the **qImmutable** marker, the following
+guidelines should be followed to determine if the marker is being used
+correctly.
+
+ 1. Check to see if the field is assigned anywhere other than the device
+ initialization function.
+
+ 2. Check to see if any function is being called that modifies the state of the
+ field outside of the initialization function.
+
+It can be subtle whether a field is truly immutable. A good example is a
+*QEMUTimer*. Timer's will usually have their timeout modified with a call to
+*qemu_mod_timer()* even though they are only assigned in the device
+initialization function.
+
+If the timer is always modified with a fixed value that is not dependent on
+guest state, then the timer is immutable since it's unaffected by the state of
+the guest.
+
+On the other hand, if the timer is modified based on guest state (such as a
+guest programmed time out), then the timer carries state. It may be necessary
+to save/restore the timer or mark it as **qDerived** and work with it
+accordingly.
+
+### qDerived Fields
+
+If a field is set based on some other field in the device's structure, then its
+value is derived. Since this is effectively duplicate state, we can avoid
+sending it and then recompute it when we need to. Derived state requires a bit
+more handling than immutable state.
+
+In our *SerialDevice* example, our *int_pending* flag is really derived from
+two pieces of state. It is set based on whether interrupts are enabled in the
+*ier* register and whether there is *THRE* flag is not set in the *lsr*
+register.
+
+To mark a field as derived, use the **derived** marker. To update our
+example, we would do:
+
+ QIDL_DECLARE(SerialDevice) {
+ SysBusDevice parent;
+
+ uint8_t thr;
+ uint8_t lsr;
+ uint8_t ier;
+
+ int int_pending qDerived;
+ CharDriverState *chr qImmutable;
+ };
+
+There is one other critical step needed when marking a field as derived. A
+*post_load* function must be added that updates this field after loading the
+rest of the device state. This function is implemented in the device's source
+file, not in the QIDL header. Below is an example of what this function may do:
+
+ static void serial_post_load(SerialDevice *s)
+ {
+ s->int_pending = !(s->lsr & THRE) && (s->ier & INTE);
+ }
+
+When reviewing a patch that marks a field as *derived*, the following criteria
+should be used:
+
+ 1. Does the device have a post load function?
+
+ 2. Does the post load function assign a value to all of the derived fields?
+
+ 3. Are there any obvious places where a derived field is holding unique state?
+
+### qBroken Fields
+
+QEMU does migration with a lot of devices today. When applying this methodology
+to these devices, one will quickly discover that there are a lot of fields that
+are not being saved today that are not derived or immutable state.
+
+These are all bugs. It just so happens that these bugs are usually not very
+serious. In many cases, they cause small functionality glitches that so far
+have not created any problems.
+
+Consider our *SerialDevice* example. In QEMU's real *SerialState* device, the
+*thr* register is not saved, yet we have not marked it immutable or derived.
+
+The *thr* register is a temporary holding register that the next character to
+transmit is placed in while we wait for the next baud cycle. In QEMU, we
+emulate a very fast baud rate regardless of what guest programs. This means
+that the contents of the *thr* register only matter for a very small period of
+time (measured in microseconds).
+
+The likelihood of a migration converging in that very small period of time when
+the *thr* register has a meaningful value is very small. Moreover, the worst
+thing that can happen by not saving this register is that we lose a byte in the
+data stream. Even if this has happened in practice, the chances of someone
+noticing this as a bug is pretty small.
+
+Nonetheless, this is a bug and needs to be eventually fixed. However, it would
+be very inconvenient to constantly break migration by fixing all of these bugs
+one-by-one. Instead, QIDL has a **broken** marker. This indicates that a field
+is not currently saved, but should be in the future.
+
+In general, qBroken markers should never be introduced in new code, and should
+be used instead as a development aid to avoid serialization issues while
+writing new device code.
+
+Below is an update of our example to reflect our real life serial device:
+
+ QIDL_DECLARE(SerialDevice) {
+ SysBusDevice parent;
+
+ uint8_t thr qBroken;
+ uint8_t lsr;
+ uint8_t ier;
+
+ int int_pending qDerived;
+ CharDriverState qImmutable *chr;
+ };
+
+When reviewing the use of the broken marker, the following things should be
+considered:
+
+ 1. What are the ramifications of not sending this data field?
+
+ 2. If the not sending this data field can cause data corruption or very poor
+ behavior within the guest, the broken marker is not appropriate to use.
+
+ 3. Assigning a default value to a field can also be used to fix a broken field
+ without significantly impacting live migration compatibility.
+
+### qElsewhere fields
+
+In some cases state is saved-off when serializing a seperate device
+structure. For example, IDEState stores a reference to an IDEBus structure:
+
+ QIDL_DECLARE(IDEState) {
+ IDEBus *bus qElsewhere;
+ uint8_t unit;
+ ...
+ };
+
+However, IDEState is actually a member of IDEBus, so would have already been
+serialized in the process of serializing IDEBus:
+
+ QIDL_DECLARE(IDEBus) {
+ BusState qbus;
+ IDEDevice *master;
+ IDEDevice *slave;
+ IDEState ifs[2];
+ ...
+ };
+
+To handle this case we've used the *qElsewhere* marker to note that the
+IDEBus* field in IDEState should not be saved since that is handled
+elsewhere.
+
+### qOptional fields
+
+Some state is only serialized in certain circumstances. To handle these cases
+you can specify a ***qOptional*** marker, which will, for a particular field
+"fieldname", tell QIDL to reference the field "has_fieldname" (of type bool)
+in the same struct to determine whether or not to serialize "fieldname". For
+example, if the data field was optionally serialized, you could do following:
+
+ QIDL_DECLARE(SerialFIFO) {
+ bool has_data;
+ uint8_t data[UART_FIFO_LENGTH] qOptional;
+ uint8_t count;
+ uint8_t itl;
+ uint8_t tail;
+ uint8_t head;
+ };
+
+Of course, your device code will need to be updated to set has_data when
+appropriate. If has_data is set based on guest state, then it must be
+serialized as well.
+
+Arrays
+------
+
+QIDL has support for multiple types of arrays. The following sections describe
+the different rules for arrays.
+
+Fixed Sized Arrays
+------------------
+
+A fixed sized array has a size that is known at build time. A typical example
+would be:
+
+ QIDL_DECLARE(SerialFIFO) {
+ uint8_t data[UART_FIFO_LENGTH];
+ uint8_t count;
+ uint8_t itl;
+ uint8_t tail;
+ uint8_t head;
+ };
+
+In this example, *data* is a fixed sized array. No special annotation is needed
+for QIDL to marshal this area correctly. The following guidelines apply to
+fixed sized arrays:
+
+ 1. The size of the array is part of the device ABI. It should not change
+ without renaming the field.
+
+Variable Sized, Fixed Capacity Arrays
+-------------------------------------
+
+Sometimes it's desirable to have a variable sized array. QIDL currently
+supports variable sized arrays provided that the maximum capacity is fixed and
+part of the device structure memory.
+
+A typical example would be a slightly modified version of our above example:
+
+ QIDL_DECLARE(SerialFIFO) {
+ uint8_t count;
+ uint8_t data[UART_FIFO_LENGTH] qSize(count);
+ uint8_t itl;
+ uint8_t tail;
+ uint8_t head;
+ };
+
+In this example, *data* is a variable sized array with a fixed capacity of
+*UART_FIFO_LENGTH*. When we serialize, we want only want to serialize *count*
+members.
+
+The ABI implications of capacity are a bit more relaxed with variable sized
+arrays. In general, you can increase or decrease the capacity without breaking
+the ABI although you may cause some instances of migration to fail between
+versions of QEMU with different capacities.
+
+When reviewing variable sized, fixed capacity arrays, keep the following things
+in mind:
+
+ 1. The variable size must occur before the array element in the state
+ structure.
+
+ 2. The capacity can change without breaking the ABI, but care should be used
+ when making these types of changes.
--
1.7.9.5
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH 17/22] qidl: add documentation
2012-09-21 14:07 ` [Qemu-devel] [PATCH 17/22] qidl: add documentation Michael Roth
@ 2012-09-21 23:16 ` Eric Blake
0 siblings, 0 replies; 39+ messages in thread
From: Eric Blake @ 2012-09-21 23:16 UTC (permalink / raw)
To: Michael Roth; +Cc: blauwirbel, peter.maydell, aliguori, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2927 bytes --]
On 09/21/2012 08:07 AM, Michael Roth wrote:
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
> docs/qidl.txt | 347 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 347 insertions(+)
> create mode 100644 docs/qidl.txt
> +Specifying What/How State Gets Saved
> +---------------------------------
Should the --- match the header length in the previous line?
> +
> +By default, QIDL saves every field in a structure it sees. This provides
> +maximum correctness by default. However, device structures generally contain
> +state that reflects state that is in someway duplicated or not guest visible.
s/someway/some way/
> +This more often that not reflects design implementation details.
s/that/than/
> +
> +Since design implementation details change over time, saving this state makes
> +compatibility hard to maintain. The proper solution is to use an intermediate
> +protocol to handle cross-version compatibility (for instance, a QIDL-aware
> +implementation of VMState). But we can reduce churn and streamline the
> +serialization/deserialization process by explicitly marking fields with
> +information that QIDL can use to determine whether or not a particular field
> +will be serialized. However, a serializable device implementation that fails to
> +serialize state that is required to fully guest state is a broken one, so to
s/fully guest/fully restore guest/
> +avoid that there are very strict rules about when this is allowed, and what
> +needs to be done to ensure that this does not impact correctness.
> +
> +There are also occassions where we want to specify *how* a field is
s/occassions/occasions/
> +
> +It can be subtle whether a field is truly immutable. A good example is a
> +*QEMUTimer*. Timer's will usually have their timeout modified with a call to
s/Timer's/Timers/
> +
> +In our *SerialDevice* example, our *int_pending* flag is really derived from
> +two pieces of state. It is set based on whether interrupts are enabled in the
> +*ier* register and whether there is *THRE* flag is not set in the *lsr*
s/there is/the/
> +When reviewing the use of the broken marker, the following things should be
> +considered:
> +
> + 1. What are the ramifications of not sending this data field?
> +
> + 2. If the not sending this data field can cause data corruption or very poor
s/the not/not/
> + behavior within the guest, the broken marker is not appropriate to use.
> +
> + 3. Assigning a default value to a field can also be used to fix a broken field
> + without significantly impacting live migration compatibility.
> +
> +### qElsewhere fields
> +
> +In some cases state is saved-off when serializing a seperate device
s/seperate/separate/
--
Eric Blake eblake@redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 617 bytes --]
^ permalink raw reply [flat|nested] 39+ messages in thread
* [Qemu-devel] [PATCH 18/22] qidl: add lexer library (based on QC parser)
2012-09-21 14:07 [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization Michael Roth
` (16 preceding siblings ...)
2012-09-21 14:07 ` [Qemu-devel] [PATCH 17/22] qidl: add documentation Michael Roth
@ 2012-09-21 14:07 ` Michael Roth
2012-09-21 23:18 ` Eric Blake
2012-09-21 14:07 ` [Qemu-devel] [PATCH 19/22] qidl: add C parser " Michael Roth
` (4 subsequent siblings)
22 siblings, 1 reply; 39+ messages in thread
From: Michael Roth @ 2012-09-21 14:07 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, peter.maydell, aliguori, eblake
Adds an abstract Lexer class to handle tokenizer via a
peek/pop/peekline/popline interface, along with an implementation for C
based on the lexer from qc.git
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
scripts/lexer.py | 306 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 306 insertions(+)
create mode 100644 scripts/lexer.py
diff --git a/scripts/lexer.py b/scripts/lexer.py
new file mode 100644
index 0000000..e740e5c
--- /dev/null
+++ b/scripts/lexer.py
@@ -0,0 +1,306 @@
+#
+# QEMU Lexer Library
+#
+# 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.
+# See the COPYING file in the top-level directory.
+#
+# The lexer code is based off of:
+# http://www.lysator.liu.se/c/ANSI-C-grammar-l.html
+
+class Input(object):
+ def __init__(self, fp):
+ self.fp = fp
+ self.line = None
+ self.offset = 0
+ self.is_eof = False
+ self.__fill_buf()
+
+ def __fill_buf(self):
+ if not self.line and not self.is_eof:
+ self.line = self.fp.readline()
+ if not self.line:
+ self.is_eof = True
+
+ def peek(self):
+ if self.is_eof:
+ return ""
+ return self.line[self.offset]
+
+ def pop(self):
+ if self.is_eof:
+ return ""
+ ch = self.line[self.offset]
+ self.offset += 1
+ if self.offset == len(self.line):
+ self.offset = 0
+ self.line = None
+ self.__fill_buf()
+ return ch
+
+ def peek_line(self):
+ return self.line
+
+ def pop_line(self):
+ line = self.line
+ self.line = None
+ self.offset = 0
+ self.__fill_buf()
+ return line
+
+ def eof(self):
+ return self.is_eof
+
+class Lexer(object):
+ def __init__(self, input, ignored_types=[]):
+ self.input = input
+ self.ignored_types = ignored_types
+ self.current_type = None
+ self.current_value = None
+
+ def get_token(self):
+ raise NotImplemented("derived classes must implement this method")
+
+ def __ensure_token(self):
+ while self.current_type == None and not self.input.eof():
+ t, v = self.get_token()
+ if t not in self.ignored_types:
+ self.current_type = t
+ self.current_value = v
+
+ def peek(self):
+ self.__ensure_token()
+ return self.current_value
+
+ def peek_line(self):
+ self.__ensure_token()
+ return self.input.peek_line()
+
+ def peek_type(self):
+ self.__ensure_token()
+ return self.current_type
+
+ def pop(self):
+ self.__ensure_token()
+ v = self.current_value
+ self.current_type = None
+ self.current_value = None
+ return v
+
+ def pop_line(self):
+ self.__ensure_token()
+ self.current_type = None
+ self.current_value = None
+ return self.input.pop_line()
+
+ def pop_expected(self, type_expected=None, value_expected=None):
+ self.__ensure_token()
+ if self.current_type != type_expected:
+ raise Exception("expected '%s', got %s %s" %
+ (type_expected, self.current_type, self.current_value))
+ if value_expected != None:
+ if self.current_value != value_expected:
+ raise Exception("expected '%s', got %s" %
+ (value_expected, self.current_value))
+ return self.pop()
+
+ def check_token(self, type_expected, value_expected=None):
+ self.__ensure_token()
+ if self.current_type != type_expected:
+ return False
+ if value_expected != None:
+ if self.current_value != value_expected:
+ return False
+ return True
+
+ def eof(self):
+ self.__ensure_token()
+ return self.current_type == None
+
+def in_range(ch, start, end):
+ if ch >= start and ch <= end:
+ return True
+ return False
+
+# D [0-9]
+# L [a-zA-Z_]
+# H [a-fA-F0-9]
+# E [Ee][+-]?{D}+
+# FS (f|F|l|L)
+# IS (u|U|l|L)*
+
+def is_D(ch):
+ return in_range(ch, '0', '9')
+
+def is_L(ch):
+ return in_range(ch, 'a', 'z') or in_range(ch, 'A', 'Z') or ch == '_'
+
+def is_H(ch):
+ return in_range(ch, 'a', 'f') or in_range(ch, 'A', 'F') or is_D(ch)
+
+def is_FS(ch):
+ return ch in 'fFlL'
+
+def is_IS(ch):
+ return ch in 'uUlL'
+
+class CLexer(Lexer):
+ def __init__(self, input, ignored_types=[]):
+ super(CLexer, self).__init__(input, ignored_types)
+
+ # used internally, external users should use
+ # CLexer.peek()/peek_type()/pop() instead
+ def get_token(self):
+ token = ''
+ while not self.input.eof():
+ ch = self.input.peek()
+
+ if is_L(ch):
+ token += ch
+ self.input.pop()
+ ch = self.input.peek()
+ while is_L(ch) or is_D(ch):
+ token += ch
+ self.input.pop()
+ ch = self.input.peek()
+ if token in [ 'auto', 'break', 'case', 'const', 'continue',
+ 'default', 'do', 'else', 'enum', 'extern',
+ 'for', 'goto', 'if', 'register', 'return',
+ 'signed', 'sizeof',
+ 'static', 'struct', 'typedef', 'union',
+ 'unsigned', 'volatile', 'while' ]:
+ return (token, token)
+ else:
+ return ('symbol', token)
+ elif ch == "'":
+ token += ch
+ self.input.pop()
+
+ ch = self.input.peek()
+ if ch == '\\':
+ token += ch
+ self.input.pop()
+ token += self.input.pop()
+ else:
+ token += ch
+ token += self.input.pop()
+ return ('literal', token)
+ elif ch == '"':
+ token += ch
+ self.input.pop()
+
+ ch = self.input.peek()
+ while ch not in ['', '"']:
+ token += ch
+ self.input.pop()
+ if ch == '\\':
+ token += self.input.pop()
+ ch = self.input.peek()
+ token += ch
+ self.input.pop()
+ return ('literal', token)
+ elif ch in '.><+-*/%&^|!;{},:=()[]~?':
+ token += ch
+ self.input.pop()
+ ch = self.input.peek()
+ tmp_token = token + ch
+ if tmp_token in ['<:']:
+ return ('operator', '[')
+ elif tmp_token in [':>']:
+ return ('operator', ']')
+ elif tmp_token in ['<%']:
+ return ('operator', '{')
+ elif tmp_token in ['%>']:
+ return ('operator', '}')
+ elif tmp_token == '//':
+ token = tmp_token
+ ch = self.input.peek()
+ while ch != '\n' and ch != '':
+ token += ch
+ self.input.pop()
+ ch = self.input.peek()
+ return ('comment', token)
+ elif tmp_token == '/*':
+ token = tmp_token
+ self.input.pop()
+
+ ch = self.input.peek()
+ while True:
+ while ch != '*':
+ token += ch
+ self.input.pop()
+ ch = self.input.peek()
+ token += ch
+ self.input.pop()
+ ch = self.input.peek()
+ if ch == '/':
+ token += ch
+ self.input.pop()
+ break
+ return ('comment', token)
+ elif tmp_token in [ '+=', '-=', '*=', '/=', '%=', '&=', '^=',
+ '|=', '>>', '<<', '++', '--', '->', '&&',
+ '||', '<=', '>=', '==', '!=' ]:
+ return ('operator', tmp_token)
+ else:
+ return ('operator', token)
+ elif ch == '0':
+ token += ch
+ self.input.pop()
+ ch = self.input.peek()
+ if ch in 'xX':
+ token += ch
+ self.input.pop()
+ ch = self.input.peek()
+ while is_H(ch):
+ token += ch
+ self.input.pop()
+ ch = self.input.peek()
+ while is_IS(ch):
+ token += ch
+ self.input.pop()
+ ch = self.input.peek()
+ elif is_D(ch):
+ token += ch
+ self.input.pop()
+ ch = self.input.peek()
+ while is_D(ch):
+ token += ch
+ self.input.pop()
+ ch = self.input.peek()
+ return ('literal', token)
+ elif is_D(ch):
+ token += ch
+ self.input.pop()
+ ch = self.input.peek()
+ while is_D(ch):
+ token += ch
+ self.input.pop()
+ ch = self.input.peek()
+ return ('literal', token)
+ elif ch in ' \t\v\n\f':
+ token += ch
+ self.input.pop()
+ ch = self.input.peek()
+ while len(ch) and ch in ' \t\v\n\f':
+ token += ch
+ self.input.pop()
+ ch = self.input.peek()
+ return ('whitespace', token)
+ elif ch in '#':
+ token += ch
+ self.input.pop()
+ ch = self.input.peek()
+ while len(ch) and ch != '\n':
+ token += ch
+ self.input.pop()
+ ch = self.input.peek()
+ return ('directive', token)
+ else:
+ return ('unknown', ch)
+ return (None, None)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH 18/22] qidl: add lexer library (based on QC parser)
2012-09-21 14:07 ` [Qemu-devel] [PATCH 18/22] qidl: add lexer library (based on QC parser) Michael Roth
@ 2012-09-21 23:18 ` Eric Blake
2012-09-21 23:52 ` Michael Roth
0 siblings, 1 reply; 39+ messages in thread
From: Eric Blake @ 2012-09-21 23:18 UTC (permalink / raw)
To: Michael Roth; +Cc: blauwirbel, peter.maydell, aliguori, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1021 bytes --]
On 09/21/2012 08:07 AM, Michael Roth wrote:
> Adds an abstract Lexer class to handle tokenizer via a
> peek/pop/peekline/popline interface, along with an implementation for C
> based on the lexer from qc.git
>
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
> scripts/lexer.py | 306 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 306 insertions(+)
> create mode 100644 scripts/lexer.py
>
> diff --git a/scripts/lexer.py b/scripts/lexer.py
> new file mode 100644
> index 0000000..e740e5c
> --- /dev/null
> +++ b/scripts/lexer.py
> @@ -0,0 +1,306 @@
> +#
> +# QEMU Lexer Library
> +#
> +# 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.
Any specific reason this is not GPLv2+?
--
Eric Blake eblake@redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 617 bytes --]
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Qemu-devel] [PATCH 18/22] qidl: add lexer library (based on QC parser)
2012-09-21 23:18 ` Eric Blake
@ 2012-09-21 23:52 ` Michael Roth
2012-09-25 21:09 ` Anthony Liguori
0 siblings, 1 reply; 39+ messages in thread
From: Michael Roth @ 2012-09-21 23:52 UTC (permalink / raw)
To: Eric Blake; +Cc: blauwirbel, peter.maydell, aliguori, qemu-devel
On Fri, Sep 21, 2012 at 05:18:09PM -0600, Eric Blake wrote:
> On 09/21/2012 08:07 AM, Michael Roth wrote:
> > Adds an abstract Lexer class to handle tokenizer via a
> > peek/pop/peekline/popline interface, along with an implementation for C
> > based on the lexer from qc.git
> >
> > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> > ---
> > scripts/lexer.py | 306 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 306 insertions(+)
> > create mode 100644 scripts/lexer.py
> >
> > diff --git a/scripts/lexer.py b/scripts/lexer.py
> > new file mode 100644
> > index 0000000..e740e5c
> > --- /dev/null
> > +++ b/scripts/lexer.py
> > @@ -0,0 +1,306 @@
> > +#
> > +# QEMU Lexer Library
> > +#
> > +# 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.
>
> Any specific reason this is not GPLv2+?
I thought I'd assigned the same license Anthony used for
https://github.com/aliguori/qidl/blob/master/qc.py, but I
guess the license wasn't actually specified. If it's fine
by Anthony I'll switch them to GPLv2+.
>
> --
> Eric Blake eblake@redhat.com +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Qemu-devel] [PATCH 18/22] qidl: add lexer library (based on QC parser)
2012-09-21 23:52 ` Michael Roth
@ 2012-09-25 21:09 ` Anthony Liguori
0 siblings, 0 replies; 39+ messages in thread
From: Anthony Liguori @ 2012-09-25 21:09 UTC (permalink / raw)
To: Michael Roth, Eric Blake; +Cc: blauwirbel, peter.maydell, qemu-devel
Michael Roth <mdroth@linux.vnet.ibm.com> writes:
> On Fri, Sep 21, 2012 at 05:18:09PM -0600, Eric Blake wrote:
>> On 09/21/2012 08:07 AM, Michael Roth wrote:
>> > Adds an abstract Lexer class to handle tokenizer via a
>> > peek/pop/peekline/popline interface, along with an implementation for C
>> > based on the lexer from qc.git
>> >
>> > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
>> > ---
>> > scripts/lexer.py | 306 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> > 1 file changed, 306 insertions(+)
>> > create mode 100644 scripts/lexer.py
>> >
>> > diff --git a/scripts/lexer.py b/scripts/lexer.py
>> > new file mode 100644
>> > index 0000000..e740e5c
>> > --- /dev/null
>> > +++ b/scripts/lexer.py
>> > @@ -0,0 +1,306 @@
>> > +#
>> > +# QEMU Lexer Library
>> > +#
>> > +# 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.
>>
>> Any specific reason this is not GPLv2+?
>
> I thought I'd assigned the same license Anthony used for
> https://github.com/aliguori/qidl/blob/master/qc.py, but I
> guess the license wasn't actually specified. If it's fine
> by Anthony I'll switch them to GPLv2+.
Works for me.
Regards,
Anthony Liguori
>
>>
>> --
>> Eric Blake eblake@redhat.com +1-919-301-3266
>> Libvirt virtualization library http://libvirt.org
>>
^ permalink raw reply [flat|nested] 39+ messages in thread
* [Qemu-devel] [PATCH 19/22] qidl: add C parser (based on QC parser)
2012-09-21 14:07 [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization Michael Roth
` (17 preceding siblings ...)
2012-09-21 14:07 ` [Qemu-devel] [PATCH 18/22] qidl: add lexer library (based on QC parser) Michael Roth
@ 2012-09-21 14:07 ` Michael Roth
2012-09-21 23:19 ` Eric Blake
2012-09-21 14:07 ` [Qemu-devel] [PATCH 20/22] qidl: add QAPI-based code generator Michael Roth
` (3 subsequent siblings)
22 siblings, 1 reply; 39+ messages in thread
From: Michael Roth @ 2012-09-21 14:07 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, peter.maydell, aliguori, eblake
This introduces the QIDL parser to process QIDL annotations in C files.
This code is mostly a straight import from qc.git, with some reworking
to handle the declaration/annotation format and lexer we're using for
QEMU.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
scripts/qidl_parser.py | 262 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 262 insertions(+)
create mode 100644 scripts/qidl_parser.py
diff --git a/scripts/qidl_parser.py b/scripts/qidl_parser.py
new file mode 100644
index 0000000..fe155f7
--- /dev/null
+++ b/scripts/qidl_parser.py
@@ -0,0 +1,262 @@
+#
+# QEMU IDL Parser
+#
+# 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.
+# See the COPYING file in the top-level directory.
+
+import sys, json
+from lexer import Input, CLexer
+
+def process_annotation(node, params):
+ annotation_type = params[0]
+ if annotation_type == "derived":
+ node['is_derived'] = True
+ elif annotation_type == 'immutable':
+ node['is_immutable'] = True
+ elif annotation_type == 'elsewhere':
+ node['is_elsewhere'] = True
+ elif annotation_type == 'broken':
+ node['is_broken'] = True
+ elif annotation_type == 'size_is':
+ node['is_array'] = True
+ node['array_size'] = params[1]
+ elif annotation_type == 'optional':
+ node['is_optional'] = True
+ elif annotation_type == 'property':
+ node['is_property'] = True
+ if node.has_key('property_fields'):
+ node['property_fields'].append(params[1:])
+ else:
+ node['property_fields'] = [params[1:]]
+
+ return node
+
+def parse_annotations(l, node):
+ while l.check_token('symbol', 'QIDL'):
+ params = []
+ l.pop()
+
+ l.pop_expected('operator', '(')
+ open_parens = 1
+ param = ""
+ while open_parens:
+ if l.check_token('operator', ','):
+ params.append(param)
+ param = ""
+ l.pop()
+ continue
+
+ if l.check_token('operator', '('):
+ open_parens += 1
+ elif l.check_token('operator', ')'):
+ open_parens -= 1
+
+ if open_parens > 0:
+ param += l.peek()
+
+ l.pop()
+
+ if param != "":
+ params.append(param)
+
+ node = process_annotation(node, params)
+
+ return node
+
+def parse_type(l):
+ node = {}
+
+ typename = ''
+ if l.check_token('const', 'const'):
+ typename += l.pop() + ' '
+
+ if l.check_token('struct', 'struct'):
+ typename += l.pop() + ' '
+
+ if l.check_token('unsigned', 'unsigned'):
+ typename += l.pop() + ' '
+
+ if l.check_token('union', 'union'):
+ typename += l.pop() + ' '
+
+ if l.check_token('enum', 'enum'):
+ typename += l.pop() + ' '
+
+ # we don't currently handle embedded struct declarations, skip them for now
+ if l.check_token('operator', '{'):
+ open_braces = 1
+ while open_braces:
+ l.pop()
+ if l.check_token('operator', '{'):
+ open_braces += 1
+ elif l.check_token('operator', '}'):
+ open_braces -= 1
+ l.pop()
+ typename += "<anon>"
+ node['is_nested_decl'] = True
+ else:
+ if l.check_token('operator', '*'):
+ l.pop()
+ node['is_pointer'] = True
+ else:
+ typename += l.pop_expected('symbol')
+
+ node['type'] = typename
+
+ node = parse_annotations(l, node)
+
+ if l.check_token('operator', '*'):
+ l.pop()
+ node['is_pointer'] = True
+
+ return node
+
+def parse_var_decl(l, repeating_type=None):
+ if repeating_type == None:
+ node = parse_type(l)
+ else:
+ node = { 'type': repeating_type }
+
+ if l.check_token('operator', '('):
+ node['is_function'] = True
+ l.pop()
+ l.pop_expected('operator', '*')
+ variable = l.pop_expected('symbol')
+ l.pop_expected('operator', ')')
+
+ # skip the param list since we don't use it currently
+ l.pop_expected('operator', '(')
+ open_parens = 1
+ while open_parens:
+ if l.check_token('operator', '('):
+ open_parens += 1
+ elif l.check_token('operator', ')'):
+ open_parens -= 1
+ l.pop()
+ else:
+ variable = l.pop_expected('symbol')
+ node['variable'] = variable
+
+ if l.check_token('operator', '['):
+ l.pop()
+ expression = ""
+ while not l.check_token('operator', ']'):
+ expression += l.pop()
+ l.pop_expected('operator', ']')
+
+ if not node.has_key('is_array'):
+ node['is_array'] = True
+ node['array_size'] = expression
+ else:
+ node['array_capacity'] = expression
+
+ node = parse_annotations(l, node)
+
+ return node
+
+def parse_struct(l):
+ l.pop_expected('struct', 'struct')
+
+ name = None
+ if l.check_token('symbol'):
+ name = l.pop()
+
+ l.pop_expected('operator', '{')
+
+ nodes = []
+
+ while not l.check_token('operator', '}'):
+ node = parse_var_decl(l)
+ nodes.append(node)
+ while l.check_token('operator', ','):
+ l.pop()
+ node = parse_var_decl(l, node['type'])
+ nodes.append(node)
+
+ l.pop_expected('operator', ';')
+
+ l.pop()
+
+ return { 'struct': name, 'fields': nodes }
+
+def parse_typedef(l):
+ l.pop_expected('typedef', 'typedef')
+
+ node = parse_struct(l)
+ typename = l.pop_expected('symbol')
+
+ return { 'typedef': typename, 'type': node }
+
+def parse_declaration_params(l):
+ declaration_info = {}
+ params = []
+ arg_string = ""
+ parens = 0
+ l.pop_expected('symbol', 'QIDL_START')
+ while not l.eof():
+ if l.check_token('operator', '('):
+ parens += 1
+ elif l.check_token('operator', ')'):
+ parens -= 1
+ if parens == 0:
+ break
+ elif parens > 0:
+ if not l.check_token('operator', ','):
+ params.append(l.peek())
+ l.pop()
+ l.pop_expected('operator', ')')
+ if parens != 0:
+ raise Exception("unmatched parenthesis in QIDL macro")
+
+ declaration_info['id'] = params[0]
+ declaration_info['do_state'] = True
+ declaration_info['do_properties'] = True
+ if "skip_state" in params:
+ declaration_info['do_state'] = False
+ if "skip_properties" in params:
+ declaration_info['do_properties'] = False
+
+ return declaration_info
+
+def parse_declaration(l):
+ declaration_info = parse_declaration_params(l)
+
+ if l.check_token('typedef'):
+ node = parse_typedef(l)
+ elif l.check_token('struct'):
+ node = parse_struct(l)
+ else:
+ raise Exception("unsupported QIDL declaration")
+
+ l.pop_expected('operator', ';')
+ node['id'] = declaration_info['id']
+ node['do_state'] = declaration_info['do_state']
+ node['do_properties'] = declaration_info['do_properties']
+
+ return node
+
+def parse_file(f):
+ nodes = []
+ filtered_tokens = ['whitespace', 'comment', 'directive']
+ l = CLexer(Input(f), filtered_tokens)
+ while not l.eof():
+ line = l.peek_line()
+ if line.startswith("QIDL_START("):
+ node = parse_declaration(l)
+ nodes.append(node)
+ else:
+ l.pop_line()
+ return nodes
+
+def main():
+ nodes = parse_file(sys.stdin)
+ print json.dumps(nodes, sort_keys=True, indent=2)
+
+if __name__ == '__main__':
+ main()
--
1.7.9.5
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH 19/22] qidl: add C parser (based on QC parser)
2012-09-21 14:07 ` [Qemu-devel] [PATCH 19/22] qidl: add C parser " Michael Roth
@ 2012-09-21 23:19 ` Eric Blake
0 siblings, 0 replies; 39+ messages in thread
From: Eric Blake @ 2012-09-21 23:19 UTC (permalink / raw)
To: Michael Roth; +Cc: blauwirbel, peter.maydell, aliguori, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1097 bytes --]
On 09/21/2012 08:07 AM, Michael Roth wrote:
> This introduces the QIDL parser to process QIDL annotations in C files.
> This code is mostly a straight import from qc.git, with some reworking
> to handle the declaration/annotation format and lexer we're using for
> QEMU.
>
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
> scripts/qidl_parser.py | 262 ++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 262 insertions(+)
> create mode 100644 scripts/qidl_parser.py
>
> diff --git a/scripts/qidl_parser.py b/scripts/qidl_parser.py
> new file mode 100644
> index 0000000..fe155f7
> --- /dev/null
> +++ b/scripts/qidl_parser.py
> @@ -0,0 +1,262 @@
> +#
> +# QEMU IDL Parser
> +#
> +# 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.
Any reason this is not GPLv2+?
--
Eric Blake eblake@redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 617 bytes --]
^ permalink raw reply [flat|nested] 39+ messages in thread
* [Qemu-devel] [PATCH 20/22] qidl: add QAPI-based code generator
2012-09-21 14:07 [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization Michael Roth
` (18 preceding siblings ...)
2012-09-21 14:07 ` [Qemu-devel] [PATCH 19/22] qidl: add C parser " Michael Roth
@ 2012-09-21 14:07 ` Michael Roth
2012-09-21 14:07 ` [Qemu-devel] [PATCH 21/22] qidl: qidl.h, definitions for qidl annotations Michael Roth
` (2 subsequent siblings)
22 siblings, 0 replies; 39+ messages in thread
From: Michael Roth @ 2012-09-21 14:07 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, peter.maydell, aliguori, eblake
This takes the declarations generated by the QIDL parser and converts
them to QAPI schemas to generate the visitor routines and other
supporting code for QIDL.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
scripts/qidl.py | 281 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 281 insertions(+)
create mode 100644 scripts/qidl.py
diff --git a/scripts/qidl.py b/scripts/qidl.py
new file mode 100644
index 0000000..5726d98
--- /dev/null
+++ b/scripts/qidl.py
@@ -0,0 +1,281 @@
+#
+# 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.
+# 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 json
+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 = {}
+
+ 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['<annotated>'] = 'true'
+ description['<embedded struct>'] = 'true'
+ description['type'] = typename
+ 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 = json.dumps("%s" % json.dumps(schema))
+ 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=", "include="])
+ except getopt.GetoptError, err:
+ print >> sys.stderr, err
+ return 1
+
+ output_filepath = None
+ includes = []
+ for o, a in opts:
+ if o in ("-f", "--output-filepath"):
+ output_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 = ""
+ 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 = json.dumps(schema, indent=4).replace("\"", "'").split("\n")
+ expr = parse_schema(schema_text)[0]
+
+ 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)
+
+ write_file(output, output_filepath)
+
+ return 0
+
+if __name__ == '__main__':
+ sys.exit(main(sys.argv))
--
1.7.9.5
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PATCH 21/22] qidl: qidl.h, definitions for qidl annotations
2012-09-21 14:07 [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization Michael Roth
` (19 preceding siblings ...)
2012-09-21 14:07 ` [Qemu-devel] [PATCH 20/22] qidl: add QAPI-based code generator Michael Roth
@ 2012-09-21 14:07 ` Michael Roth
2012-09-21 14:07 ` [Qemu-devel] [PATCH 22/22] qidl: unit tests and build infrastructure Michael Roth
2012-09-21 15:57 ` [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization Paolo Bonzini
22 siblings, 0 replies; 39+ messages in thread
From: Michael Roth @ 2012-09-21 14:07 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, peter.maydell, aliguori, eblake
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qidl.h | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 113 insertions(+)
create mode 100644 qidl.h
diff --git a/qidl.h b/qidl.h
new file mode 100644
index 0000000..5fa2180
--- /dev/null
+++ b/qidl.h
@@ -0,0 +1,113 @@
+/*
+ * QEMU IDL Macros/stubs
+ *
+ * See docs/qidl.txt for usage information.
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ * 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.
+ *
+ */
+
+#ifndef QIDL_H
+#define QIDL_H
+
+#include <glib.h>
+#include "qapi/qapi-visit-core.h"
+#include "qemu/object.h"
+#include "hw/qdev-properties.h"
+
+#ifdef QIDL_GEN
+
+/* we pass the code through the preprocessor with QIDL_GEN defined to parse
+ * structures as they'd appear after preprocessing, and use the following
+ * definitions mostly to re-insert the initial macros/annotations so they
+ * stick around for the parser to process
+ */
+#define QIDL(...) QIDL(__VA_ARGS__)
+#define QIDL_START(name, ...) QIDL_START(name, ##__VA_ARGS__)
+
+#define QIDL_VISIT_TYPE(name, v, s, f, e)
+#define QIDL_SCHEMA_ADD_LINK(name, obj, path, errp)
+#define QIDL_PROPERTIES(name)
+#define QIDL_DECLARE(name, ...) \
+ QIDL_START(name, ##__VA_ARGS__) \
+ struct name
+
+#else /* !QIDL_GEN */
+
+#define QIDL(...)
+#ifdef QIDL_ENABLED
+#define QIDL_START(name, ...) \
+ static struct { \
+ void (*visitor)(Visitor *, struct name **, const char *, Error **); \
+ const char *schema_json_text; \
+ Object *schema_obj; \
+ Property *properties; \
+ } qidl_data_##name;
+#else
+#define QIDL_START(name, ...)
+#endif
+
+#define QIDL_DECLARE(name, ...) \
+ QIDL_START(name, ##__VA_ARGS__) \
+ struct name
+
+#define QIDL_VISIT_TYPE(name, v, s, f, e) \
+ g_assert(qidl_data_##name.visitor); \
+ qidl_data_##name.visitor(v, s, NULL, e)
+
+#define QIDL_SCHEMA_ADD_LINK(name, obj, path, errp) \
+ g_assert(qidl_data_##name.schema_obj); \
+ object_property_add_link(obj, path, "container", \
+ &qidl_data_##name.schema_obj, errp)
+
+#define QIDL_PROPERTIES(name) \
+ qidl_data_##name.properties
+
+#endif /* QIDL_GEN */
+
+/* must be "called" in any C files that make use of QIDL-generated code */
+#define QIDL_ENABLE()
+
+/* QIDL annotations/markers
+ *
+ * qImmutable: state is fully restorable via device
+ * [re-]initialization/realization
+ *
+ * qDerived: state can be fully reconstructed from other fields (and will be,
+ * via [re-]initialization of the device or a separate hook)
+ *
+ * qBroken: state should (or possibly should) be saved, but isn't. mostly an aid
+ * for device developers having issues with serialization of a particular
+ * field, committed code should contain these except in special circumstances
+ *
+ * qOptional: <field> should only be serialized if the field by the name of
+ * has_<field> is true
+ *
+ * qElsewhere: state should be serialized, but is done so elsewhere (for
+ * instance, by another device with a pointer to the same data)
+ *
+ * qSize(field): for static/dynamically-allocated arrays. specifies the field
+ * in the structure containing the number of elements that should be
+ * serialized. if argument is wrapped in parenthesis it is instead interpreted
+ * as an expression that should be invaluated to determine the size.
+ *
+ * qProperty(<property name> [, <default value>]): specifies that field is a
+ * qdev-style property. all properties of the struct are then accessible via
+ * QIDL_PROPERTIES(<device name>) macro.
+ */
+
+#define qImmutable QIDL(immutable)
+#define qDerived QIDL(derived)
+#define qBroken QIDL(broken)
+#define qOptional QIDL(optional)
+#define qElsewhere QIDL(elsewhere)
+#define qSize(...) QIDL(size_is, ##__VA_ARGS__)
+#define qProperty(name, ...) QIDL(property, name, ##__VA_ARGS__)
+
+#endif
--
1.7.9.5
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PATCH 22/22] qidl: unit tests and build infrastructure
2012-09-21 14:07 [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization Michael Roth
` (20 preceding siblings ...)
2012-09-21 14:07 ` [Qemu-devel] [PATCH 21/22] qidl: qidl.h, definitions for qidl annotations Michael Roth
@ 2012-09-21 14:07 ` Michael Roth
2012-09-21 15:57 ` [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization Paolo Bonzini
22 siblings, 0 replies; 39+ messages in thread
From: Michael Roth @ 2012-09-21 14:07 UTC (permalink / raw)
To: qemu-devel; +Cc: blauwirbel, peter.maydell, aliguori, eblake
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
Makefile | 2 +
rules.mak | 20 ++++-
tests/Makefile | 8 +-
tests/test-qidl-included.h | 31 ++++++++
tests/test-qidl-linked.c | 93 ++++++++++++++++++++++
tests/test-qidl-linked.h | 18 +++++
tests/test-qidl.c | 187 ++++++++++++++++++++++++++++++++++++++++++++
7 files changed, 356 insertions(+), 3 deletions(-)
create mode 100644 tests/test-qidl-included.h
create mode 100644 tests/test-qidl-linked.c
create mode 100644 tests/test-qidl-linked.h
create mode 100644 tests/test-qidl.c
diff --git a/Makefile b/Makefile
index b1e1304..6b30ce9 100644
--- a/Makefile
+++ b/Makefile
@@ -231,6 +231,7 @@ clean:
if test -d $$d; then $(MAKE) -C $$d $@ || exit 1; fi; \
rm -f $$d/qemu-options.def; \
done
+ find -depth -name qidl-generated -type d -exec rm -rf {} \;
VERSION ?= $(shell cat VERSION)
@@ -401,6 +402,7 @@ qemu-doc.dvi qemu-doc.html qemu-doc.info qemu-doc.pdf: \
# rebuilt before other object files
Makefile: $(GENERATED_HEADERS)
+
# Include automatically generated dependency files
# Dependencies in Makefile.objs files come from our recursive subdir rules
-include $(wildcard *.d tests/*.d)
diff --git a/rules.mak b/rules.mak
index 1b173aa..f6a0201 100644
--- a/rules.mak
+++ b/rules.mak
@@ -15,7 +15,25 @@ MAKEFLAGS += -rR
QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(*D)/$(*F).d
%.o: %.c
- $(call quiet-command,$(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) -c -o $@ $<," CC $(TARGET_DIR)$@")
+
+%.qidl.c: %.c $(SRC_PATH)/qidl.h $(addprefix $(SRC_PATH)/scripts/,lexer.py qidl.py qidl_parser.py qapi.py qapi_visit.py)
+ $(call rm -f $(*D)/qidl-generated/$(*F).qidl.c)
+ $(if $(strip $(shell grep "QIDL_ENABLE()" $< 1>/dev/null && echo "true")), \
+ $(call quiet-command, \
+ $(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(CFLAGS) -E -c -DQIDL_GEN $< | \
+ $(PYTHON) $(SRC_PATH)/scripts/qidl.py \
+ --output-filepath=$(*D)/qidl-generated/$(*F).qidl.c || [ "$$?" -eq 2 ], \
+ "qidl PP $(*D)/$(*F).c"),)
+
+%.o: %.c %.qidl.c
+ $(if $(strip $(shell test -f $(*D)/qidl-generated/$(*F).qidl.c && echo "true")), \
+ $(call quiet-command, \
+ $(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) -c \
+ -DQIDL_ENABLED -include $< -o $@ $(*D)/qidl-generated/$(*F).qidl.c, \
+ "qidl CC $@"), \
+ $(call quiet-command, \
+ $(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(QEMU_DGFLAGS) $(CFLAGS) -c \
+ -o $@ $<," CC $@"))
ifeq ($(LIBTOOL),)
%.lo: %.c
diff --git a/tests/Makefile b/tests/Makefile
index e10aaed..fe2d025 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -15,6 +15,7 @@ check-unit-y += tests/test-string-output-visitor$(EXESUF)
check-unit-y += tests/test-coroutine$(EXESUF)
check-unit-y += tests/test-visitor-serialization$(EXESUF)
check-unit-y += tests/test-iov$(EXESUF)
+check-unit-y += tests/test-qidl$(EXESUF)
check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
@@ -34,11 +35,12 @@ test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \
tests/test-coroutine.o tests/test-string-output-visitor.o \
tests/test-string-input-visitor.o tests/test-qmp-output-visitor.o \
tests/test-qmp-input-visitor.o tests/test-qmp-input-strict.o \
- tests/test-qmp-commands.o tests/test-visitor-serialization.o
+ tests/test-qmp-commands.o tests/test-visitor-serialization.o \
+ tests/test-qidl.o
test-qapi-obj-y = $(qobject-obj-y) $(qapi-obj-y) $(tools-obj-y)
test-qapi-obj-y += tests/test-qapi-visit.o tests/test-qapi-types.o
-test-qapi-obj-y += module.o
+test-qapi-obj-y += module.o $(qom-obj-y)
$(test-obj-y): QEMU_INCLUDES += -Itests
@@ -84,6 +86,8 @@ check-qtest-$(CONFIG_POSIX)=$(foreach TARGET,$(TARGETS), $(check-qtest-$(TARGET)
qtest-obj-y = tests/libqtest.o $(oslib-obj-y) $(tools-obj-y)
$(check-qtest-y): $(qtest-obj-y)
+tests/test-qidl$(EXESUF): tests/test-qidl.o tests/test-qidl-linked.o $(test-qapi-obj-y) qapi/misc-qapi-visit.o
+
.PHONY: check-help
check-help:
@echo "Regression testing targets:"
diff --git a/tests/test-qidl-included.h b/tests/test-qidl-included.h
new file mode 100644
index 0000000..cf78c23
--- /dev/null
+++ b/tests/test-qidl-included.h
@@ -0,0 +1,31 @@
+/*
+ * Unit-tests for QIDL-generated visitors/code
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ * Michael Roth <mdroth@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef TEST_QIDL_INCLUDED_H
+#define TEST_QIDL_INCLUDED_H
+
+#include "qidl.h"
+
+typedef struct TestStructIncluded TestStructIncluded;
+
+QIDL_DECLARE(TestStructIncluded) {
+ int32_t a qImmutable;
+ int32_t b;
+ uint32_t c qImmutable;
+ uint32_t d;
+ uint64_t e qImmutable;
+ uint64_t f qProperty("f", 42);
+ char *g qProperty("g");
+ char *h qImmutable qProperty("h");
+};
+
+#endif
diff --git a/tests/test-qidl-linked.c b/tests/test-qidl-linked.c
new file mode 100644
index 0000000..5564118
--- /dev/null
+++ b/tests/test-qidl-linked.c
@@ -0,0 +1,93 @@
+/*
+ * Unit-tests for QIDL-generated visitors/code
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ * Michael Roth <mdroth@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qidl.h"
+#include "test-qidl-linked.h"
+#include "hw/qdev-properties.h"
+#include "qapi/qmp-input-visitor.h"
+#include "qapi/qmp-output-visitor.h"
+#include "qapi/qapi-dealloc-visitor.h"
+
+QIDL_ENABLE()
+
+typedef struct TestStructLinked TestStructLinked;
+
+QIDL_DECLARE(TestStructLinked) {
+ int32_t a qImmutable;
+ int32_t b;
+ uint32_t c qImmutable;
+ uint32_t d;
+ uint64_t e qImmutable;
+ uint64_t f qProperty("f", 42);
+ char *g qProperty("g");
+ char *h qImmutable qProperty("h");
+};
+
+/* exercise generated code from annotations in objects we link against */
+void test_linked_object_annotations(gconstpointer opaque)
+{
+ TestStructLinked *s1, *s2 = NULL;
+ Property *props;
+ QmpInputVisitor *qiv;
+ QmpOutputVisitor *qov;
+ QObject *s1_obj;
+ Error *err = NULL;
+
+ s1 = g_malloc0(sizeof(TestStructLinked));
+ s1->a = 42;
+ s1->b = INT32_MAX;
+ s1->c = 43;
+ s1->d = UINT32_MAX;
+ s1->e = 44;
+ s1->f = UINT64_MAX;
+ s1->g = g_strdup("test string g");
+ s1->h = g_strdup("test string h");
+
+ qov = qmp_output_visitor_new();
+ QIDL_VISIT_TYPE(TestStructLinked, qmp_output_get_visitor(qov), &s1, NULL, &err);
+ g_assert(err == NULL);
+
+ s1_obj = qmp_output_get_qobject(qov);
+ qiv = qmp_input_visitor_new(s1_obj);
+
+ qobject_decref(s1_obj);
+ qmp_output_visitor_cleanup(qov);
+ g_free(s1->g);
+ g_free(s1->h);
+ g_free(s1);
+
+ s2 = g_malloc0(sizeof(TestStructLinked));
+ QIDL_VISIT_TYPE(TestStructLinked, qmp_input_get_visitor(qiv), &s2, NULL, &err);
+ g_assert(err == NULL);
+
+ g_assert_cmpint(s2->a, ==, 0);
+ g_assert_cmpint(s2->b, ==, INT32_MAX);
+ g_assert_cmpint(s2->c, ==, 0);
+ g_assert_cmpint(s2->d, ==, UINT32_MAX);
+ g_assert_cmpint(s2->e, ==, 0);
+ g_assert_cmpint(s2->f, ==, UINT64_MAX);
+ g_assert_cmpstr(s2->g, ==, "test string g");
+ g_assert(s2->h == NULL);
+
+ qmp_input_visitor_cleanup(qiv);
+ g_free(s2->g);
+ g_free(s2);
+
+ props = QIDL_PROPERTIES(TestStructLinked);
+ g_assert_cmpstr(props[0].name, ==, "f");
+ g_assert_cmpint(props[0].defval, ==, 42);
+ g_assert_cmpstr(props[1].name, ==, "g");
+ g_assert_cmpint(props[1].defval, ==, 0);
+ g_assert_cmpstr(props[2].name, ==, "h");
+ g_assert_cmpint(props[2].defval, ==, 0);
+ g_assert(props[3].name == NULL);
+}
diff --git a/tests/test-qidl-linked.h b/tests/test-qidl-linked.h
new file mode 100644
index 0000000..1b100a2
--- /dev/null
+++ b/tests/test-qidl-linked.h
@@ -0,0 +1,18 @@
+/*
+ * Unit-tests for QIDL-generated visitors/code
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ * Michael Roth <mdroth@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef TEST_QIDL_LINKED_H
+#define TEST_QIDL_LINKED_H
+
+void test_linked_object_annotations(gconstpointer opaque);
+
+#endif
diff --git a/tests/test-qidl.c b/tests/test-qidl.c
new file mode 100644
index 0000000..505b764
--- /dev/null
+++ b/tests/test-qidl.c
@@ -0,0 +1,187 @@
+/*
+ * Unit-tests for QIDL-generated visitors/code
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ * Michael Roth <mdroth@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include <glib.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include "qidl.h"
+#include "test-qidl-included.h"
+#include "test-qidl-linked.h"
+#include "hw/qdev-properties.h"
+#include "qapi/qmp-input-visitor.h"
+#include "qapi/qmp-output-visitor.h"
+#include "qapi/qapi-dealloc-visitor.h"
+
+QIDL_ENABLE()
+
+PropertyInfo qdev_prop_uint64;
+PropertyInfo qdev_prop_string;
+
+typedef struct TestStructMain TestStructMain;
+
+QIDL_DECLARE(TestStructMain, state, properties) {
+ int32_t a qImmutable;
+ int32_t b;
+ uint32_t c qImmutable;
+ uint32_t d;
+ uint64_t e qImmutable;
+ uint64_t f qProperty("f", 42);
+ char *g qProperty("g");
+ char *h qImmutable qProperty("h");
+};
+
+/* exercise generated code from annotations in main() object file */
+static void test_main_object_annotations(gconstpointer opaque)
+{
+ TestStructMain *s1, *s2 = NULL;
+ Property *props;
+ QmpInputVisitor *qiv;
+ QmpOutputVisitor *qov;
+ QObject *s1_obj;
+ Error *err = NULL;
+
+ s1 = g_malloc0(sizeof(TestStructMain));
+ s1->a = 42;
+ s1->b = INT32_MAX;
+ s1->c = 43;
+ s1->d = UINT32_MAX;
+ s1->e = 44;
+ s1->f = UINT64_MAX;
+ s1->g = g_strdup("test string g");
+ s1->h = g_strdup("test string h");
+
+ qov = qmp_output_visitor_new();
+ QIDL_VISIT_TYPE(TestStructMain, qmp_output_get_visitor(qov), &s1, NULL, &err);
+ g_assert(err == NULL);
+
+ s1_obj = qmp_output_get_qobject(qov);
+ qiv = qmp_input_visitor_new(s1_obj);
+
+ qobject_decref(s1_obj);
+ qmp_output_visitor_cleanup(qov);
+ g_free(s1->g);
+ g_free(s1->h);
+ g_free(s1);
+
+ s2 = g_malloc0(sizeof(TestStructMain));
+ QIDL_VISIT_TYPE(TestStructMain, qmp_input_get_visitor(qiv), &s2, NULL, &err);
+ g_assert(err == NULL);
+
+ g_assert_cmpint(s2->a, ==, 0);
+ g_assert_cmpint(s2->b, ==, INT32_MAX);
+ g_assert_cmpint(s2->c, ==, 0);
+ g_assert_cmpint(s2->d, ==, UINT32_MAX);
+ g_assert_cmpint(s2->e, ==, 0);
+ g_assert_cmpint(s2->f, ==, UINT64_MAX);
+ g_assert_cmpstr(s2->g, ==, "test string g");
+ g_assert(s2->h == NULL);
+
+ qmp_input_visitor_cleanup(qiv);
+ g_free(s2->g);
+ g_free(s2);
+
+ props = QIDL_PROPERTIES(TestStructMain);
+ g_assert_cmpstr(props[0].name, ==, "f");
+ g_assert_cmpint(props[0].defval, ==, 42);
+ g_assert_cmpstr(props[1].name, ==, "g");
+ g_assert_cmpint(props[1].defval, ==, 0);
+ g_assert_cmpstr(props[2].name, ==, "h");
+ g_assert_cmpint(props[2].defval, ==, 0);
+ g_assert(props[3].name == NULL);
+}
+
+/* exercise generated code from simplified annotations in main() object file */
+static void test_main_object_annotations_simple(gconstpointer opaque)
+{
+ test_main_object_annotations(opaque);
+}
+
+/* exercise generated code from annotations in included header files */
+static void test_header_file_annotations(gconstpointer opaque)
+{
+ TestStructIncluded *s1, *s2 = NULL;
+ Property *props;
+ QmpInputVisitor *qiv;
+ QmpOutputVisitor *qov;
+ QObject *s1_obj;
+ Error *err = NULL;
+
+ s1 = g_malloc0(sizeof(TestStructIncluded));
+ s1->a = 42;
+ s1->b = INT32_MAX;
+ s1->c = 43;
+ s1->d = UINT32_MAX;
+ s1->e = 44;
+ s1->f = UINT64_MAX;
+ s1->g = g_strdup("test string g");
+ s1->h = g_strdup("test string h");
+
+ qov = qmp_output_visitor_new();
+ QIDL_VISIT_TYPE(TestStructIncluded, qmp_output_get_visitor(qov), &s1, NULL, &err);
+ g_assert(err == NULL);
+
+ s1_obj = qmp_output_get_qobject(qov);
+ qiv = qmp_input_visitor_new(s1_obj);
+
+ qobject_decref(s1_obj);
+ qmp_output_visitor_cleanup(qov);
+ g_free(s1->g);
+ g_free(s1->h);
+ g_free(s1);
+
+ s2 = g_malloc0(sizeof(TestStructIncluded));
+ QIDL_VISIT_TYPE(TestStructIncluded, qmp_input_get_visitor(qiv), &s2, NULL, &err);
+ g_assert(err == NULL);
+
+ g_assert_cmpint(s2->a, ==, 0);
+ g_assert_cmpint(s2->b, ==, INT32_MAX);
+ g_assert_cmpint(s2->c, ==, 0);
+ g_assert_cmpint(s2->d, ==, UINT32_MAX);
+ g_assert_cmpint(s2->e, ==, 0);
+ g_assert_cmpint(s2->f, ==, UINT64_MAX);
+ g_assert_cmpstr(s2->g, ==, "test string g");
+ g_assert(s2->h == NULL);
+
+ qmp_input_visitor_cleanup(qiv);
+ g_free(s2->g);
+ g_free(s2);
+
+ props = QIDL_PROPERTIES(TestStructIncluded);
+ g_assert_cmpstr(props[0].name, ==, "f");
+ g_assert_cmpint(props[0].defval, ==, 42);
+ g_assert_cmpstr(props[1].name, ==, "g");
+ g_assert_cmpint(props[1].defval, ==, 0);
+ g_assert_cmpstr(props[2].name, ==, "h");
+ g_assert_cmpint(props[2].defval, ==, 0);
+ g_assert(props[3].name == NULL);
+}
+
+int main(int argc, char **argv)
+{
+ module_call_init(MODULE_INIT_QOM);
+ module_call_init(MODULE_INIT_QIDL);
+
+ g_test_init(&argc, &argv, NULL);
+
+ g_test_add_data_func("/qidl/build_test/main_object_annotations", NULL,
+ test_main_object_annotations);
+ g_test_add_data_func("/qidl/build_test/main_object_annotations_simple", NULL,
+ test_main_object_annotations_simple);
+ g_test_add_data_func("/qidl/build_test/linked_object_annotations", NULL,
+ test_linked_object_annotations);
+ g_test_add_data_func("/qidl/build_test/header_file_annotations", NULL,
+ test_header_file_annotations);
+
+ g_test_run();
+
+ return 0;
+}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization
2012-09-21 14:07 [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization Michael Roth
` (21 preceding siblings ...)
2012-09-21 14:07 ` [Qemu-devel] [PATCH 22/22] qidl: unit tests and build infrastructure Michael Roth
@ 2012-09-21 15:57 ` Paolo Bonzini
2012-09-21 16:24 ` Michael Roth
22 siblings, 1 reply; 39+ messages in thread
From: Paolo Bonzini @ 2012-09-21 15:57 UTC (permalink / raw)
To: Michael Roth; +Cc: blauwirbel, peter.maydell, aliguori, eblake, qemu-devel
Il 21/09/2012 16:07, Michael Roth ha scritto:
>
> QIDL_DECLARE(SerialDevice) {
> SysBusDevice parent;
>
> uint8_t thr; /* transmit holding register */
> uint8_t lsr; /* line status register */
> uint8_t ier; /* interrupt enable register */
>
> int int_pending qDerived; /* whether we have a pending queued interrupt */
> CharDriverState *chr qImmutable; /* backend */
> };
I thought we agreed on QIDL(derived), QIDL(immutable) etc. These
prefixes just do not scale...
Paolo
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization
2012-09-21 15:57 ` [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization Paolo Bonzini
@ 2012-09-21 16:24 ` Michael Roth
2012-09-22 14:33 ` Blue Swirl
0 siblings, 1 reply; 39+ messages in thread
From: Michael Roth @ 2012-09-21 16:24 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: blauwirbel, peter.maydell, aliguori, eblake, qemu-devel
On Fri, Sep 21, 2012 at 05:57:42PM +0200, Paolo Bonzini wrote:
> Il 21/09/2012 16:07, Michael Roth ha scritto:
> >
> > QIDL_DECLARE(SerialDevice) {
> > SysBusDevice parent;
> >
> > uint8_t thr; /* transmit holding register */
> > uint8_t lsr; /* line status register */
> > uint8_t ier; /* interrupt enable register */
> >
> > int int_pending qDerived; /* whether we have a pending queued interrupt */
> > CharDriverState *chr qImmutable; /* backend */
> > };
>
> I thought we agreed on QIDL(derived), QIDL(immutable) etc. These
> prefixes just do not scale...
qImmutable gets defined as QIDL(immutable) via qidl.h, and underneath
the covers it's all QIDL(). So we can change them easily if need be, and
still have the optional of using QIDL() for any current or new
annotations that get introduced. But QIDL() just ends up being
really noisey in practice, especially when a field has multiple
annotations, so I'd like to make that kind of usage the exceptional
case rather than the common one.
I went with qUppercase because it avoids all the previous issues with
using leading underscores, and it's reserved in terms of QEMU coding
guidelines as far as I can tell (we generally require leading capital
for typedefs and lowercase for variable names, and can work around
exceptions on a case by case basis by using QIDL() or some other name).
I also had it as q_* for a bit but that didn't seem much better on the
eyes we looking at converted structures.
>
> Paolo
>
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization
2012-09-21 16:24 ` Michael Roth
@ 2012-09-22 14:33 ` Blue Swirl
2012-09-24 18:14 ` Michael Roth
0 siblings, 1 reply; 39+ messages in thread
From: Blue Swirl @ 2012-09-22 14:33 UTC (permalink / raw)
To: Michael Roth; +Cc: Paolo Bonzini, aliguori, eblake, qemu-devel, peter.maydell
On Fri, Sep 21, 2012 at 4:24 PM, Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> On Fri, Sep 21, 2012 at 05:57:42PM +0200, Paolo Bonzini wrote:
>> Il 21/09/2012 16:07, Michael Roth ha scritto:
>> >
>> > QIDL_DECLARE(SerialDevice) {
>> > SysBusDevice parent;
>> >
>> > uint8_t thr; /* transmit holding register */
>> > uint8_t lsr; /* line status register */
>> > uint8_t ier; /* interrupt enable register */
>> >
>> > int int_pending qDerived; /* whether we have a pending queued interrupt */
>> > CharDriverState *chr qImmutable; /* backend */
>> > };
>>
>> I thought we agreed on QIDL(derived), QIDL(immutable) etc. These
>> prefixes just do not scale...
>
> qImmutable gets defined as QIDL(immutable) via qidl.h, and underneath
> the covers it's all QIDL(). So we can change them easily if need be, and
> still have the optional of using QIDL() for any current or new
> annotations that get introduced. But QIDL() just ends up being
> really noisey in practice, especially when a field has multiple
> annotations, so I'd like to make that kind of usage the exceptional
> case rather than the common one.
>
> I went with qUppercase because it avoids all the previous issues with
> using leading underscores, and it's reserved in terms of QEMU coding
> guidelines as far as I can tell (we generally require leading capital
> for typedefs and lowercase for variable names, and can work around
> exceptions on a case by case basis by using QIDL() or some other name).
> I also had it as q_* for a bit but that didn't seem much better on the
> eyes we looking at converted structures.
It looks like Hungarian notation and very much unlike other QEMU code.
I'd use q_ or qidl_ prefix instead, or rather QIDL().
>
>>
>> Paolo
>>
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization
2012-09-22 14:33 ` Blue Swirl
@ 2012-09-24 18:14 ` Michael Roth
2012-09-25 6:37 ` Paolo Bonzini
0 siblings, 1 reply; 39+ messages in thread
From: Michael Roth @ 2012-09-24 18:14 UTC (permalink / raw)
To: Blue Swirl; +Cc: Paolo Bonzini, aliguori, eblake, qemu-devel, peter.maydell
On Sat, Sep 22, 2012 at 02:33:52PM +0000, Blue Swirl wrote:
> On Fri, Sep 21, 2012 at 4:24 PM, Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> > On Fri, Sep 21, 2012 at 05:57:42PM +0200, Paolo Bonzini wrote:
> >> Il 21/09/2012 16:07, Michael Roth ha scritto:
> >> >
> >> > QIDL_DECLARE(SerialDevice) {
> >> > SysBusDevice parent;
> >> >
> >> > uint8_t thr; /* transmit holding register */
> >> > uint8_t lsr; /* line status register */
> >> > uint8_t ier; /* interrupt enable register */
> >> >
> >> > int int_pending qDerived; /* whether we have a pending queued interrupt */
> >> > CharDriverState *chr qImmutable; /* backend */
> >> > };
> >>
> >> I thought we agreed on QIDL(derived), QIDL(immutable) etc. These
> >> prefixes just do not scale...
> >
> > qImmutable gets defined as QIDL(immutable) via qidl.h, and underneath
> > the covers it's all QIDL(). So we can change them easily if need be, and
> > still have the optional of using QIDL() for any current or new
> > annotations that get introduced. But QIDL() just ends up being
> > really noisey in practice, especially when a field has multiple
> > annotations, so I'd like to make that kind of usage the exceptional
> > case rather than the common one.
> >
> > I went with qUppercase because it avoids all the previous issues with
> > using leading underscores, and it's reserved in terms of QEMU coding
> > guidelines as far as I can tell (we generally require leading capital
> > for typedefs and lowercase for variable names, and can work around
> > exceptions on a case by case basis by using QIDL() or some other name).
> > I also had it as q_* for a bit but that didn't seem much better on the
> > eyes we looking at converted structures.
>
> It looks like Hungarian notation and very much unlike other QEMU code.
> I'd use q_ or qidl_ prefix instead, or rather QIDL().
>
I wanted some way to distinguish from other qemu code to avoid conflicts,
but i think q_* seems reasonable if we reserve the prefix via CODING_STYLE.
Then for conflicts outside our control we can either use a different name
for the annotations or use the long-form QIDL() style depending on the
circumstances.
> >
> >>
> >> Paolo
> >>
>
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization
2012-09-24 18:14 ` Michael Roth
@ 2012-09-25 6:37 ` Paolo Bonzini
2012-09-25 15:45 ` Michael Roth
0 siblings, 1 reply; 39+ messages in thread
From: Paolo Bonzini @ 2012-09-25 6:37 UTC (permalink / raw)
To: Michael Roth; +Cc: Blue Swirl, peter.maydell, aliguori, eblake, qemu-devel
Il 24/09/2012 20:14, Michael Roth ha scritto:
>>> > > I went with qUppercase because it avoids all the previous issues with
>>> > > using leading underscores, and it's reserved in terms of QEMU coding
>>> > > guidelines as far as I can tell (we generally require leading capital
>>> > > for typedefs and lowercase for variable names, and can work around
>>> > > exceptions on a case by case basis by using QIDL() or some other name).
>>> > > I also had it as q_* for a bit but that didn't seem much better on the
>>> > > eyes we looking at converted structures.
>> >
>> > It looks like Hungarian notation and very much unlike other QEMU code.
>> > I'd use q_ or qidl_ prefix instead, or rather QIDL().
>> >
> I wanted some way to distinguish from other qemu code to avoid conflicts,
> but i think q_* seems reasonable if we reserve the prefix via CODING_STYLE.
> Then for conflicts outside our control we can either use a different name
> for the annotations or use the long-form QIDL() style depending on the
> circumstances.
I'm not sure why we need two ways to say the same thing... I know it's
just bikeshedding to some extent, but I'd really like to standardize on
a single form.
Paolo
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization
2012-09-25 6:37 ` Paolo Bonzini
@ 2012-09-25 15:45 ` Michael Roth
2012-09-25 21:12 ` Anthony Liguori
0 siblings, 1 reply; 39+ messages in thread
From: Michael Roth @ 2012-09-25 15:45 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Blue Swirl, peter.maydell, aliguori, eblake, qemu-devel
On Tue, Sep 25, 2012 at 08:37:16AM +0200, Paolo Bonzini wrote:
> Il 24/09/2012 20:14, Michael Roth ha scritto:
> >>> > > I went with qUppercase because it avoids all the previous issues with
> >>> > > using leading underscores, and it's reserved in terms of QEMU coding
> >>> > > guidelines as far as I can tell (we generally require leading capital
> >>> > > for typedefs and lowercase for variable names, and can work around
> >>> > > exceptions on a case by case basis by using QIDL() or some other name).
> >>> > > I also had it as q_* for a bit but that didn't seem much better on the
> >>> > > eyes we looking at converted structures.
> >> >
> >> > It looks like Hungarian notation and very much unlike other QEMU code.
> >> > I'd use q_ or qidl_ prefix instead, or rather QIDL().
> >> >
> > I wanted some way to distinguish from other qemu code to avoid conflicts,
> > but i think q_* seems reasonable if we reserve the prefix via CODING_STYLE.
> > Then for conflicts outside our control we can either use a different name
> > for the annotations or use the long-form QIDL() style depending on the
> > circumstances.
>
> I'm not sure why we need two ways to say the same thing... I know it's
> just bikeshedding to some extent, but I'd really like to standardize on
> a single form.
QIDL() (or maybe qidl()) should be the One True Form. It's the
only one that provides both proper namespacing and can be used both for
simple annotations and for ones that take parameters.
I guess the real question is whether or not it makes sense to provide
"shortcuts" for the more common annotations to avoid clutter. I've heard
it both ways, so it's hard to decide.
So let's bikeshed a bit. Maybe to put things into perspective, we're looking
at (and I'm just gonna go ahead and switch the OTF to qidl() now so we're
looking at the best case scenarios for both, and include q_* as well):
a) One True Form:
QIDL_DECLARE(RTCState) {
ISADevice dev qidl(immutable);
MemoryRegion io qidl(immutable);
uint8_t cmos_data[128];
uint8_t cmos_index;
int32_t base_year qidl(property, "base_year", 1980);
uint64_t base_rtc;
uint64_t last_update;
int64_t offset;
qemu_irq irq qidl(immutable);
qemu_irq sqw_irq qidl(immutable);
int it_shift qidl(immutable);
/* periodic timer */
QEMUTimer *periodic_timer;
int64_t next_periodic_time;
/* update-ended timer */
QEMUTimer *update_timer;
uint64_t next_alarm_time;
uint16_t irq_reinject_on_ack_count qidl(broken);
uint32_t irq_coalesced;
uint32_t period;
bool has_coalesced_timer;
QEMUTimer *coalesced_timer qidl(optional);
Notifier clock_reset_notifier qidl(broken);
LostTickPolicy lost_tick_policy qidl(immutable) \
qidl(property, "lost_tick_policy", LOST_TICK_DISCARD);
Notifier suspend_notifier qidl(broken);
};
b) current simplified form:
QIDL_DECLARE(RTCState) {
ISADevice dev qImmutable;
MemoryRegion io qImmutable;
uint8_t cmos_data[128];
uint8_t cmos_index;
int32_t base_year qProperty("base_year", 1980);
uint64_t base_rtc;
uint64_t last_update;
int64_t offset;
qemu_irq irq qImmutable;
qemu_irq sqw_irq qImmutable;
int it_shift qImmutable;
/* periodic timer */
QEMUTimer *periodic_timer;
int64_t next_periodic_time;
/* update-ended timer */
QEMUTimer *update_timer;
uint64_t next_alarm_time;
uint16_t irq_reinject_on_ack_count qBroken;
uint32_t irq_coalesced;
uint32_t period;
bool has_coalesced_timer;
QEMUTimer *coalesced_timer qOptional;
Notifier clock_reset_notifier qBroken;
LostTickPolicy lost_tick_policy qImmutable \
qProperty("lost_tick_policy", LOST_TICK_DISCARD);
Notifier suspend_notifier qBroken;
};
c) proposed simplified form:
QIDL_DECLARE(RTCState) {
ISADevice dev q_immutable;
MemoryRegion io q_immutable;
uint8_t cmos_data[128];
uint8_t cmos_index;
int32_t base_year q_property("base_year", 1980);
uint64_t base_rtc;
uint64_t last_update;
int64_t offset;
qemu_irq irq q_immutable;
qemu_irq sqw_irq q_immutable;
int it_shift q_immutable;
/* periodic timer */
QEMUTimer *periodic_timer;
int64_t next_periodic_time;
/* update-ended timer */
QEMUTimer *update_timer;
uint64_t next_alarm_time;
uint16_t irq_reinject_on_ack_count q_broken;
uint32_t irq_coalesced;
uint32_t period;
bool has_coalesced_timer;
QEMUTimer *coalesced_timer q_optional;
Notifier clock_reset_notifier q_broken;
LostTickPolicy lost_tick_policy q_immutable \
q_property("lost_tick_policy", LOST_TICK_DISCARD);
Notifier suspend_notifier q_broken;
Personally, I think b) is the only simplified form that reduces overall visual
noise. So if b) isn't an option, I think a) is the way to go. Using the
lowercasing for qidl(), and lack of an underscore that introduces an extra
break, makes it a lot easier on the eyes, IMO. It's still more keystrokes, but
that's not really my main concern: I just don't want to make all our devices
a headache to look at.
>
> Paolo
>
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization
2012-09-25 15:45 ` Michael Roth
@ 2012-09-25 21:12 ` Anthony Liguori
2012-09-26 9:57 ` Paolo Bonzini
2012-09-26 10:20 ` Kevin Wolf
0 siblings, 2 replies; 39+ messages in thread
From: Anthony Liguori @ 2012-09-25 21:12 UTC (permalink / raw)
To: Michael Roth, Paolo Bonzini; +Cc: Blue Swirl, peter.maydell, eblake, qemu-devel
Michael Roth <mdroth@linux.vnet.ibm.com> writes:
> On Tue, Sep 25, 2012 at 08:37:16AM +0200, Paolo Bonzini wrote:
>> Il 24/09/2012 20:14, Michael Roth ha scritto:
>> >>> > > I went with qUppercase because it avoids all the previous issues with
>> >>> > > using leading underscores, and it's reserved in terms of QEMU coding
>> >>> > > guidelines as far as I can tell (we generally require leading capital
>> >>> > > for typedefs and lowercase for variable names, and can work around
>> >>> > > exceptions on a case by case basis by using QIDL() or some other name).
>> >>> > > I also had it as q_* for a bit but that didn't seem much better on the
>> >>> > > eyes we looking at converted structures.
>> >> >
>> >> > It looks like Hungarian notation and very much unlike other QEMU code.
>> >> > I'd use q_ or qidl_ prefix instead, or rather QIDL().
>> >> >
>> > I wanted some way to distinguish from other qemu code to avoid conflicts,
>> > but i think q_* seems reasonable if we reserve the prefix via CODING_STYLE.
>> > Then for conflicts outside our control we can either use a different name
>> > for the annotations or use the long-form QIDL() style depending on the
>> > circumstances.
>>
>> I'm not sure why we need two ways to say the same thing... I know it's
>> just bikeshedding to some extent, but I'd really like to standardize on
>> a single form.
>
> QIDL() (or maybe qidl()) should be the One True Form. It's the
> only one that provides both proper namespacing and can be used both for
> simple annotations and for ones that take parameters.
>
> I guess the real question is whether or not it makes sense to provide
> "shortcuts" for the more common annotations to avoid clutter. I've heard
> it both ways, so it's hard to decide.
>
> So let's bikeshed a bit. Maybe to put things into perspective, we're looking
> at (and I'm just gonna go ahead and switch the OTF to qidl() now so we're
> looking at the best case scenarios for both, and include q_* as well):
>
> a) One True Form:
> QIDL_DECLARE(RTCState) {
> ISADevice dev qidl(immutable);
> MemoryRegion io qidl(immutable);
Just like sparse is a "compiler", so is qidl. We are free to use the
'_' + lowercase prefix.
ISADevice _immutable dev;
It's an established practice in wide-use.
Regards,
Anthony Liguori
> uint8_t cmos_data[128];
> uint8_t cmos_index;
> int32_t base_year qidl(property, "base_year", 1980);
> uint64_t base_rtc;
> uint64_t last_update;
> int64_t offset;
> qemu_irq irq qidl(immutable);
> qemu_irq sqw_irq qidl(immutable);
> int it_shift qidl(immutable);
> /* periodic timer */
> QEMUTimer *periodic_timer;
> int64_t next_periodic_time;
> /* update-ended timer */
> QEMUTimer *update_timer;
> uint64_t next_alarm_time;
> uint16_t irq_reinject_on_ack_count qidl(broken);
> uint32_t irq_coalesced;
> uint32_t period;
> bool has_coalesced_timer;
> QEMUTimer *coalesced_timer qidl(optional);
> Notifier clock_reset_notifier qidl(broken);
> LostTickPolicy lost_tick_policy qidl(immutable) \
> qidl(property, "lost_tick_policy", LOST_TICK_DISCARD);
> Notifier suspend_notifier qidl(broken);
> };
>
> b) current simplified form:
> QIDL_DECLARE(RTCState) {
> ISADevice dev qImmutable;
> MemoryRegion io qImmutable;
> uint8_t cmos_data[128];
> uint8_t cmos_index;
> int32_t base_year qProperty("base_year", 1980);
> uint64_t base_rtc;
> uint64_t last_update;
> int64_t offset;
> qemu_irq irq qImmutable;
> qemu_irq sqw_irq qImmutable;
> int it_shift qImmutable;
> /* periodic timer */
> QEMUTimer *periodic_timer;
> int64_t next_periodic_time;
> /* update-ended timer */
> QEMUTimer *update_timer;
> uint64_t next_alarm_time;
> uint16_t irq_reinject_on_ack_count qBroken;
> uint32_t irq_coalesced;
> uint32_t period;
> bool has_coalesced_timer;
> QEMUTimer *coalesced_timer qOptional;
> Notifier clock_reset_notifier qBroken;
> LostTickPolicy lost_tick_policy qImmutable \
> qProperty("lost_tick_policy", LOST_TICK_DISCARD);
> Notifier suspend_notifier qBroken;
> };
>
> c) proposed simplified form:
> QIDL_DECLARE(RTCState) {
> ISADevice dev q_immutable;
> MemoryRegion io q_immutable;
> uint8_t cmos_data[128];
> uint8_t cmos_index;
> int32_t base_year q_property("base_year", 1980);
> uint64_t base_rtc;
> uint64_t last_update;
> int64_t offset;
> qemu_irq irq q_immutable;
> qemu_irq sqw_irq q_immutable;
> int it_shift q_immutable;
> /* periodic timer */
> QEMUTimer *periodic_timer;
> int64_t next_periodic_time;
> /* update-ended timer */
> QEMUTimer *update_timer;
> uint64_t next_alarm_time;
> uint16_t irq_reinject_on_ack_count q_broken;
> uint32_t irq_coalesced;
> uint32_t period;
> bool has_coalesced_timer;
> QEMUTimer *coalesced_timer q_optional;
> Notifier clock_reset_notifier q_broken;
> LostTickPolicy lost_tick_policy q_immutable \
> q_property("lost_tick_policy", LOST_TICK_DISCARD);
> Notifier suspend_notifier q_broken;
>
> Personally, I think b) is the only simplified form that reduces overall visual
> noise. So if b) isn't an option, I think a) is the way to go. Using the
> lowercasing for qidl(), and lack of an underscore that introduces an extra
> break, makes it a lot easier on the eyes, IMO. It's still more keystrokes, but
> that's not really my main concern: I just don't want to make all our devices
> a headache to look at.
>
>>
>> Paolo
>>
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization
2012-09-25 21:12 ` Anthony Liguori
@ 2012-09-26 9:57 ` Paolo Bonzini
2012-09-26 10:20 ` Kevin Wolf
1 sibling, 0 replies; 39+ messages in thread
From: Paolo Bonzini @ 2012-09-26 9:57 UTC (permalink / raw)
To: Anthony Liguori
Cc: Blue Swirl, peter.maydell, eblake, Michael Roth, qemu-devel
Il 25/09/2012 23:12, Anthony Liguori ha scritto:
> Just like sparse is a "compiler", so is qidl. We are free to use the
> '_' + lowercase prefix.
>
> ISADevice _immutable dev;
>
> It's an established practice in wide-use.
But QEMU is also compiled with GCC, so we're not. The Linux kernel
coding standards are not something to imitate for what concerns use of
reserved identifiers, if only because it targets a freestanding environment.
Paolo
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization
2012-09-25 21:12 ` Anthony Liguori
2012-09-26 9:57 ` Paolo Bonzini
@ 2012-09-26 10:20 ` Kevin Wolf
2012-09-26 10:33 ` Paolo Bonzini
1 sibling, 1 reply; 39+ messages in thread
From: Kevin Wolf @ 2012-09-26 10:20 UTC (permalink / raw)
To: Anthony Liguori
Cc: peter.maydell, qemu-devel, Michael Roth, Blue Swirl,
Paolo Bonzini, eblake
Am 25.09.2012 23:12, schrieb Anthony Liguori:
> Michael Roth <mdroth@linux.vnet.ibm.com> writes:
>
>> On Tue, Sep 25, 2012 at 08:37:16AM +0200, Paolo Bonzini wrote:
>>> Il 24/09/2012 20:14, Michael Roth ha scritto:
>>>>>>>> I went with qUppercase because it avoids all the previous issues with
>>>>>>>> using leading underscores, and it's reserved in terms of QEMU coding
>>>>>>>> guidelines as far as I can tell (we generally require leading capital
>>>>>>>> for typedefs and lowercase for variable names, and can work around
>>>>>>>> exceptions on a case by case basis by using QIDL() or some other name).
>>>>>>>> I also had it as q_* for a bit but that didn't seem much better on the
>>>>>>>> eyes we looking at converted structures.
>>>>>>
>>>>>> It looks like Hungarian notation and very much unlike other QEMU code.
>>>>>> I'd use q_ or qidl_ prefix instead, or rather QIDL().
>>>>>>
>>>> I wanted some way to distinguish from other qemu code to avoid conflicts,
>>>> but i think q_* seems reasonable if we reserve the prefix via CODING_STYLE.
>>>> Then for conflicts outside our control we can either use a different name
>>>> for the annotations or use the long-form QIDL() style depending on the
>>>> circumstances.
>>>
>>> I'm not sure why we need two ways to say the same thing... I know it's
>>> just bikeshedding to some extent, but I'd really like to standardize on
>>> a single form.
>>
>> QIDL() (or maybe qidl()) should be the One True Form. It's the
>> only one that provides both proper namespacing and can be used both for
>> simple annotations and for ones that take parameters.
>>
>> I guess the real question is whether or not it makes sense to provide
>> "shortcuts" for the more common annotations to avoid clutter. I've heard
>> it both ways, so it's hard to decide.
>>
>> So let's bikeshed a bit. Maybe to put things into perspective, we're looking
>> at (and I'm just gonna go ahead and switch the OTF to qidl() now so we're
>> looking at the best case scenarios for both, and include q_* as well):
>>
>> a) One True Form:
>> QIDL_DECLARE(RTCState) {
>> ISADevice dev qidl(immutable);
>> MemoryRegion io qidl(immutable);
>
> Just like sparse is a "compiler", so is qidl. We are free to use the
> '_' + lowercase prefix.
>
> ISADevice _immutable dev;
>
> It's an established practice in wide-use.
Not commenting on the underscore, but you did one thing that I want to
support: Put the (q)_immutable in a place where it looks like a
qualifier. Not so important for the qidl(...) syntax, but with the
simplified forms I definitely like it better.
I think I would even have made it '(q)_immutable ISADevice dev;', but
having the field name last is what really matters for readability.
Kevin
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization
2012-09-26 10:20 ` Kevin Wolf
@ 2012-09-26 10:33 ` Paolo Bonzini
2012-09-26 15:12 ` Michael Roth
0 siblings, 1 reply; 39+ messages in thread
From: Paolo Bonzini @ 2012-09-26 10:33 UTC (permalink / raw)
To: Kevin Wolf
Cc: peter.maydell, Anthony Liguori, qemu-devel, Michael Roth,
Blue Swirl, eblake
Il 26/09/2012 12:20, Kevin Wolf ha scritto:
>>> >> QIDL_DECLARE(RTCState) {
>>> >> ISADevice dev qidl(immutable);
>>> >> MemoryRegion io qidl(immutable);
>> >
>> > Just like sparse is a "compiler", so is qidl. We are free to use the
>> > '_' + lowercase prefix.
>> >
>> > ISADevice _immutable dev;
>> >
>> > It's an established practice in wide-use.
> Not commenting on the underscore, but you did one thing that I want to
> support: Put the (q)_immutable in a place where it looks like a
> qualifier. Not so important for the qidl(...) syntax, but with the
> simplified forms I definitely like it better.
>
> I think I would even have made it '(q)_immutable ISADevice dev;', but
> having the field name last is what really matters for readability.
Agreed. I don't want to be a nuisance, so: Michael, please pick one between
ISADevice QIDL(immutable) dev
ISADevice q_immutable dev
ISADevice qidl(immutable) dev
and if you choose the second, let's make QIDL an implementation detail,
i.e. document that every new attribute we introduce should define a new
q_* macro.
Paolo
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH v2] Add infrastructure for QIDL-based device serialization
2012-09-26 10:33 ` Paolo Bonzini
@ 2012-09-26 15:12 ` Michael Roth
0 siblings, 0 replies; 39+ messages in thread
From: Michael Roth @ 2012-09-26 15:12 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Kevin Wolf, peter.maydell, Anthony Liguori, qemu-devel,
Blue Swirl, eblake
On Wed, Sep 26, 2012 at 12:33:17PM +0200, Paolo Bonzini wrote:
> Il 26/09/2012 12:20, Kevin Wolf ha scritto:
> >>> >> QIDL_DECLARE(RTCState) {
> >>> >> ISADevice dev qidl(immutable);
> >>> >> MemoryRegion io qidl(immutable);
> >> >
> >> > Just like sparse is a "compiler", so is qidl. We are free to use the
> >> > '_' + lowercase prefix.
> >> >
> >> > ISADevice _immutable dev;
> >> >
> >> > It's an established practice in wide-use.
> > Not commenting on the underscore, but you did one thing that I want to
> > support: Put the (q)_immutable in a place where it looks like a
> > qualifier. Not so important for the qidl(...) syntax, but with the
> > simplified forms I definitely like it better.
> >
> > I think I would even have made it '(q)_immutable ISADevice dev;', but
> > having the field name last is what really matters for readability.
>
> Agreed. I don't want to be a nuisance, so: Michael, please pick one between
Not a problem, the parser supports both before/after. I prefer before as well,
except in the case of q_property("name", <options>) where we often need to put
the variable name on a second line, but those aren't too common so let's just
standardize on before for now since that'll benefit the common use case better.
>
> ISADevice QIDL(immutable) dev
> ISADevice q_immutable dev
> ISADevice qidl(immutable) dev
>
> and if you choose the second, let's make QIDL an implementation detail,
> i.e. document that every new attribute we introduce should define a new
> q_* macro.
Ok, sounds like a plan. let's do q_*.
>
> Paolo
>
^ permalink raw reply [flat|nested] 39+ messages in thread