[Qemu-devel] [PATCH v4 00/26] Add infrastructure for QIDL-based device serialization

[Qemu-devel] [PATCH v4 00/26] Add infrastructure for QIDL-based device serialization

[Michael Roth]

These patches are based are origin/master, and can also be obtained from:

git://github.com/mdroth/qemu.git qidl-base-v4

Changes since v3:

 - Added documentation for array_capacity/array_size/embedded QAPI
   directives and improved existing docs. (Paolo)
     > qapi: Improve existing docs and document annotated QAPI types
 - Fix potential for unbalanced structures with visit_type_carray() in
   similar manner to type_list()/type_struct(). (Paolo)
 - Some comments in qapi_visit.py to clarify carray handling. (Paolo)
 - Avoid redundant re-definitions in qidl.h for -DQIDL_GEN (Paolo)
 - Added additional unit tests to exercise QAPI handling of
   array_capacity/array_size/embedded values for 'annotated' QAPI-defined
   types.
 - Added a to_json() method to OrderedDict() so that ordering is retained
   in the schemas that QIDL generates.
     > qapi: ordereddict, add to_json() method
 - *.qidl.schema QAPI schema files are now generated along-side *.qidl.c
   files when --enable-debug configure option is specified.
 - 2 additional patches to fix pre-existing QAPI issues triggered in testing:
     > qapi: QmpInputVisitor, don't re-allocate memory in start_struct
     > qapi: fix potential segfault for visit_type_size()

Changes since v2:

 - Documentations fix-ups and clarifications (Eric)
 - Moved annotations in front of variable names and documented this as
   the default (Paolo, Kevin)
 - Switched to q_* prefix instead of q<Capital> (Blue, Paolo, Anthony)
 - Switched to GPLv2+ licensing in QIDL lexer/parser
 - Minor clean-ups in unit tests

Changes since v1:

 - Simplified declaration format for QIDL-fied structures (Anthony, Blue)
 - Documentations fix-ups and clarifications (Eric, Peter)
 - Reduced build-time impact of QIDL by scanning for a QIDL_ENABLED()
   directive in .c files before running them through the
   the preprocessor and QIDL parser, and using a Makefile-set cflag
   to avoid declaring QIDL-related code/structures for files that don't
   include the directive.
 - Moved lexer functionality into a standalone lexer class, simplified
   interface to avoid the need to track offsets into the token stream.
 - Fixed an issue when deserializing a static array of structs using the
   new visit_type_carray() interfaces
 - Included a fix for a qom-fuse bug caused by multiple threads contending
   for QMP responses
 - Added brief descriptions of annotations to qidl.h
 - Minor clean-ups to the QIDL parser code

Changes since rfc v2:

 - Parser/Codegen fix-ups for cases encountered converting piix ide and usb.
 - Fixed license headers.
 - Stricter arg-checking for QIDL macros when passing to codegen.
 - Renamed QAPI visit_*_array interfaces to visit_*_carray to clarify that
   these are serialization routines for single-dimension C arrays.

These patches add infrastructure and unit tests for QIDL, which provides
a serialization framework for QEMU device structures by generating visitor
routines for device structs based on simple field annotations. Examples of
how this is done are included in patch 17, but, for brevity, a sample struct
such as this:

    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;

can now be made serializable with the following changes:

    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 q_derived int_pending; /* whether we have a pending queued interrupt */
        CharDriverState q_immutable *chr; /* backend */
    };

To make use of generated visitor code, and .c file need only call the
QIDL_ENABLE() somewhere in the code body, which will then give it access to
visitor routines for any QIDL-ified device structures in that file, or included
from other files. These routines can then be used for
serialization/deserialization of the device state in a manner suitable for tasks
such as introspection/testing (generally via a r/w 'state' QOM property) or
migration.

The overall goal is to expose all migrateable device state in this manner so
that we can decouple serialization duties from savevm/VMState and convert them
into a stable, code-agnostic wire protocol, relying instead on intermediate
translation routines to handle the work of massaging serialized state into a
format suitable for the wire and vice-versa.

The following WIP branch contains the first set of QIDL conversions:

https://github.com/mdroth/qemu/commits/qidl

So far i440fx, pcibus, cirrus_vga, uhci, rtc, and isa/piix ide have been
converted, and I'm hoping to have most common PC devices converted over
within the next few weeks.

Please review. Comments/suggestions are very welcome.

 Makefile                                       |   26 +-
 QMP/qom-fuse                                   |   39 +-
 configure                                      |    1 +
 docs/qapi-code-gen.txt                         |  251 ++++++++++++-
 docs/qidl.txt                                  |  347 +++++++++++++++++
 hw/qdev-properties.h                           |  151 ++++++++
 hw/qdev.h                                      |  126 +------
 module.h                                       |    2 +
 qapi/Makefile.objs                             |    1 +
 qapi/misc-qapi-visit.c                         |   14 +
 qapi/qapi-visit-core.c                         |   31 +-
 qapi/qapi-visit-core.h                         |   11 +
 qapi/qmp-input-visitor.c                       |   32 +-
 qapi/qmp-output-visitor.c                      |   19 +
 qidl.h                                         |  105 ++++++
 rules.mak                                      |   25 +-
 scripts/lexer.py                               |  306 +++++++++++++++
 scripts/ordereddict.py                         |   51 +++
 scripts/qapi-visit.py                          |  364 ------------------
 scripts/qapi.py                                |   16 +-
 scripts/{qapi-commands.py => qapi_commands.py} |    8 +-
 scripts/{qapi-types.py => qapi_types.py}       |    2 +-
 scripts/qapi_visit.py                          |  481 ++++++++++++++++++++++++
 scripts/qidl.py                                |  290 ++++++++++++++
 scripts/qidl_parser.py                         |  269 +++++++++++++
 tests/Makefile                                 |   20 +-
 tests/test-qidl-included.h                     |   31 ++
 tests/test-qidl-linked.c                       |   93 +++++
 tests/test-qidl-linked.h                       |   18 +
 tests/test-qidl.c                              |  376 ++++++++++++++++++
 tests/test-qmp-input-visitor.c                 |    2 +-
 vl.c                                           |    1 +
 32 files changed, 2965 insertions(+), 544 deletions(-)

[Qemu-devel] [PATCH v4 01/26] qapi: qapi-visit.py -> qapi_visit.py so we can import

[Michael Roth]

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>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.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 a9c22bf..e380569 100644
--- a/Makefile
+++ b/Makefile
@@ -193,8 +193,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   $@")
@@ -203,8 +203,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

[Qemu-devel] [PATCH v4 02/26] qapi: qapi-types.py -> qapi_types.py

[Michael Roth]

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: Paolo Bonzini <pbonzini@redhat.com>
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 e380569..475b868 100644
--- a/Makefile
+++ b/Makefile
@@ -190,8 +190,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   $@")
@@ -200,8 +200,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

[Qemu-devel] [PATCH v4 03/26] qapi: qapi-commands.py -> qapi_commands.py

[Michael Roth]

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: Paolo Bonzini <pbonzini@redhat.com>
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 475b868..da4360f 100644
--- a/Makefile
+++ b/Makefile
@@ -196,8 +196,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)
@@ -206,8 +206,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

[Qemu-devel] [PATCH v4 04/26] qapi: qapi_visit.py, make code useable as module

[Michael Roth]

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.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 a360de7..f98abf2 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

[Qemu-devel] [PATCH v4 05/26] qapi: qapi_visit.py, support arrays and complex qapi definitions

[Michael Roth]

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    |  102 ++++++++++++++++++++++++++++++++++++++++++----
 4 files changed, 106 insertions(+), 14 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index afc5f32..555d823 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 1b84834..87d0218 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 f98abf2..d80d3d8 100644
--- a/scripts/qapi_visit.py
+++ b/scripts/qapi_visit.py
@@ -16,6 +16,90 @@ 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
+
+    # Visitors for "objects" (qidl-generated visitors for structs or
+    # qapi-defined types) expect a <type>** argument so that, in some
+    # cases, memory can be allocated for the object and a pointer to that
+    # memory can be assigned to the location/pointer passed to the visitor
+    # call.
+    #
+    # For other cases, "primitive" types, we don't do allocations and thus
+    # expect a <type>*.
+    #
+    # So we use special handling for the former case, below, to get the
+    # arguments right. We identify these cases by checking if the type
+    # uses camel-case, which is reserved for typedef'd structs/objects as
+    # per QEMU coding standards and QAPI convention.
+    if camel_case(de_camel_case(info['type'][0])) == info['type'][0]:
+        ret = mcgen('''
+
+if (!error_is_set(errp)) {
+    Error *%(local_err)s = NULL;
+    visit_start_carray(m, (void **)obj, "%(name)s", %(array_capacity)s, sizeof(%(type)s), &%(local_err)s);
+    if (!%(local_err)s) {
+        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, &%(local_err)s);
+            visit_type_%(type_short)s(m, &%(name)s_ptr, NULL, &%(local_err)s);
+        }
+
+        error_propagate(errp, %(local_err)s);
+        %(local_err)s = NULL;
+
+        /* Always call end_carry if start_carray succeeded.  */
+        visit_end_carray(m, &%(local_err)s);
+    }
+    error_propagate(errp, %(local_err)s);
+}
+''',
+                    name=name, type=c_type(info['type'][0]),
+                    type_short=info['type'][0],
+                    array_size=str(array_size),
+                    array_capacity=str(array_capacity),
+                    local_err=name + "_err")
+    else:
+        ret = mcgen('''
+if (!error_is_set(errp)) {
+    Error *%(local_err)s = NULL;
+    visit_start_carray(m, (void **)obj, "%(name)s", %(array_capacity)s, sizeof(%(type)s), &%(local_err)s);
+    if (!%(local_err)s) {
+        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, &%(local_err)s);
+            visit_type_%(type_short)s(m, %(name)s_ptr, NULL, &%(local_err)s);
+        }
+
+        error_propagate(errp, %(local_err)s);
+        %(local_err)s = NULL;
+
+        /* Always call end_carry if start_carray succeeded.  */
+        visit_end_carray(m, &%(local_err)s);
+    }
+    error_propagate(errp, %(local_err)s);
+}
+''',
+                    name=name, type=c_type(info['type'][0]),
+                    type_short=info['type'][0],
+                    array_size=str(array_size),
+                    array_capacity=str(array_capacity),
+                    local_err=name + "_err")
+    return ret
 
 def generate_visit_struct_body(field_prefix, name, members):
     ret = mcgen('''
@@ -45,10 +129,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 +142,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

[Qemu-devel] [PATCH v4 06/26] qapi: qapi_visit.py, support generating static functions

[Michael Roth]

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>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.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 d80d3d8..856df5e 100644
--- a/scripts/qapi_visit.py
+++ b/scripts/qapi_visit.py
@@ -179,13 +179,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)
@@ -196,10 +199,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;
@@ -221,19 +227,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('''
@@ -290,18 +299,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
 
@@ -315,12 +327,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

[Qemu-devel] [PATCH v4 07/26] qapi: qapi_visit.py, support for visiting non-pointer/embedded structs

[Michael Roth]

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 856df5e..c531a5a 100644
--- a/scripts/qapi_visit.py
+++ b/scripts/qapi_visit.py
@@ -145,6 +145,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') and argentry['embedded'] == 'true':
+                    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

[Qemu-devel] [PATCH v4 08/26] qapi: add visitor interfaces for C arrays

[Michael Roth]

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.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
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

[Qemu-devel] [PATCH v4 09/26] qapi: QmpOutputVisitor, implement array handling

[Michael Roth]

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qapi/qmp-output-visitor.c |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
index 2bce9d5..83cf0dc 100644
--- a/qapi/qmp-output-visitor.c
+++ b/qapi/qmp-output-visitor.c
@@ -181,6 +181,22 @@ 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)
+{
+    qmp_output_end_list(v, errp);
+}
+
 QObject *qmp_output_get_qobject(QmpOutputVisitor *qov)
 {
     QObject *obj = qmp_output_first(qov);
@@ -228,6 +244,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

[Qemu-devel] [PATCH v4 10/26] qapi: QmpInputVisitor, implement array handling

[Michael Roth]

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qapi/qmp-input-visitor.c |   30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 107d8d3..635106e 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -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

[Qemu-devel] [PATCH v4 11/26] qapi: QmpInputVisitor, don't re-allocate memory in start_struct

[Michael Roth]

If we're given a pointer that has already be initialized to a non-NULL
value, don't attempt to allocate memory for the object as we'll likely
clobber something we weren't supposed to.

Also, fix up a check in the unit test that may fail as a result of this
change do to it not initializing the object to NULL before-hand and thus
depending on this behavior to clobber a potentially garbage ptr value.

This is needed to handle embedded/non-pointer struct fields.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qapi/qmp-input-visitor.c       |    2 +-
 tests/test-qmp-input-visitor.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 635106e..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);
     }
 }
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index 8f5a509..58e04f1 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -247,7 +247,7 @@ static void test_visitor_in_union(TestInputVisitorData *data,
 {
     Visitor *v;
     Error *err = NULL;
-    UserDefUnion *tmp;
+    UserDefUnion *tmp = NULL;
 
     v = visitor_input_test_init(data, "{ 'type': 'b', 'data' : { 'integer': 42 } }");
 
-- 
1.7.9.5

[Qemu-devel] [PATCH v4 12/26] qapi: fix potential segfault for visit_type_size()

[Michael Roth]

visit_type_size() was added for use-cases currently only encountered by
OptsVisitor users, which implements a specific handler for
visit_type_size(). For Visitor implementations that don't implement the
handler, we fallback to using v->type_uint64().

However, some visitor implementations, such as Qmp*Visitor, also rely on
fallback code to handle visit_type_uint64() calls, and leave v->type_uint64
unset. This leads to a segfault when we try to use visit_type_size().

Fix this by calling the visit_type_uint64() function in visit_type_size()'s
fallback instead of calling v->type_uint64() directly.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qapi/qapi-visit-core.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 9a74ed0..dd28cb9 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -237,7 +237,11 @@ void visit_type_int64(Visitor *v, int64_t *obj, const char *name, Error **errp)
 void visit_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp)
 {
     if (!error_is_set(errp)) {
-        (v->type_size ? v->type_size : v->type_uint64)(v, obj, name, errp);
+        if (v->type_size) {
+            v->type_size(v, obj, name, errp);
+        } else {
+            visit_type_uint64(v, obj, name, errp);
+        }
     }
 }
 
-- 
1.7.9.5

[Qemu-devel] [PATCH v4 13/26] qapi: ordereddict, add to_json() method

[Michael Roth]

This allows OrderedDict() instances to be [pretty-]printed to
QAPI-compatible JSON structures with field ordering preserved. This is
useful for QIDL, which generates schemas programatically and exposes
them to users in various ways.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 scripts/ordereddict.py |   51 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/scripts/ordereddict.py b/scripts/ordereddict.py
index 7242b50..f1f8c71 100644
--- a/scripts/ordereddict.py
+++ b/scripts/ordereddict.py
@@ -23,6 +23,21 @@
 from UserDict import DictMixin
 
 class OrderedDict(dict, DictMixin):
+    class Indent(object):
+        def __init__(self, enable=True, initial_spacing=4, initial_level=0):
+            self.enable = enable
+            self.level = initial_level
+            self.spacing = initial_spacing
+        def inc(self, n=1):
+            self.level += n
+            return self.level
+        def dec(self, n=1):
+            self.level -= n
+            return self.level
+        def prefix(self):
+            if self.enable:
+                return " " * (self.spacing * self.level)
+            return ""
 
     def __init__(self, *args, **kwds):
         if len(args) > 1:
@@ -125,3 +140,39 @@ class OrderedDict(dict, DictMixin):
 
     def __ne__(self, other):
         return not self == other
+
+    def __obj_to_json(self, node, pretty=True, indent_spacing=4, indent_level=0):
+        i = self.Indent(pretty, indent_spacing, indent_level)
+        text = ""
+
+        if isinstance(node, OrderedDict):
+            if indent_level == 0:
+                text += "%s{\n" % i.prefix()
+            else:
+                text += "{\n"
+            i.inc()
+            for (k, v) in node.items():
+                text += "%s'%s': %s" % (i.prefix(), k,
+                    self.__obj_to_json(v, pretty, i.spacing, i.level))
+            i.dec()
+            text += "%s}" % i.prefix()
+        elif isinstance(node, list):
+            text += "[\n"
+            i.inc()
+            for e in node:
+                text += "%s%s" % (i.prefix(),
+                    self.__obj_to_json(e, pretty, i.spacing, i.level))
+            i.dec()
+            text += "%s]" % i.prefix()
+        else:
+            text += repr(node)
+
+        if indent_level > 0:
+            text += ",\n"
+
+        if not pretty:
+            return text.replace("\n", " ")
+        return text
+
+    def to_json(self, pretty=True, indent_spacing=4, indent_level=0):
+        return self.__obj_to_json(self)
-- 
1.7.9.5

[Qemu-devel] [PATCH v4 14/26] qapi: qapi.py, make json parser more robust

[Michael Roth]

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.

Also add support for parsing numerical fields. Currently they are
ignored.

QIDL will make use of both of these changes with the schemas it
generates.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 scripts/qapi.py |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 555d823..333f375 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -37,6 +37,12 @@ def tokenize(data):
                 else:
                     string += ch
             yield string
+        elif ch.isdigit():
+            number = ch
+            while data[0].isdigit():
+                number += data[0]
+                data = data[1:]
+            yield number
 
 def parse(tokens):
     if tokens[0] == '{':
@@ -81,7 +87,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

[Qemu-devel] [PATCH v4 15/26] qapi: add open-coded visitor for struct tm types

[Michael Roth]

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.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..5546865
--- /dev/null
+++ b/qapi/misc-qapi-visit.c
@@ -0,0 +1,14 @@
+#include <time.h>
+#include "qapi/qapi-visit-core.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

[Qemu-devel] [PATCH v4 16/26] qapi: Improve existing docs and document annotated QAPI types

[Michael Roth]

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 docs/qapi-code-gen.txt |  251 +++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 237 insertions(+), 14 deletions(-)

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index cccb11e..d3ca74a 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -22,38 +22,160 @@ This document will describe how the schemas, scripts, and resulting
 code is used.
 
 
-== QMP/Guest agent schema ==
+== QAPI/QMP/Guest agent schema ==
 
-This file defines the types, commands, and events used by QMP.  It should
-fully describe the interface used by QMP.
+QAPI schema files define the types, commands, and events used by QMP and
+the QEMU guest agent, and should fully describe their interfaces.  QAPI
+schemas may be used for other purposes as well, but QMP and the guest agent
+are the primary use-cases.
 
-This file is designed to be loosely based on JSON although it's technically
-executable Python.  While dictionaries are used, they are parsed as
+Schema files are designed to be loosely based on JSON, although technically
+they are executable Python.  While dictionaries are used, they are parsed as
 OrderedDicts so that ordering is preserved.
 
 There are two basic syntaxes used, type definitions and command definitions.
 
-The first syntax defines a type and is represented by a dictionary.  There are
-two kinds of types that are supported: complex user-defined types, and enums.
+The first syntax defines a user-defined/named type and is represented by a
+dictionary.  There are currently 3 kinds of user-defined/named types that are
+supported: complex types, unions, and enums.  Complex types and unions are
+composed of other complex types/unions/enums, lists thereof, or
+internally-defined types as described below.
 
-A complex type is a dictionary containing a single key who's value is a
-dictionary.  This corresponds to a struct in C or an Object in JSON.  An
-example of a complex type is:
+NOTE: The value of a argument's/member's field in the schema defines the
+member's type in any native C interfaces/types generated from the schema.
+Non-native/external/wire interfaces will sometimes use alternative encodings
+to represent the field values; QMP for instance uses JSON 'Number' types, as
+defined by IETF RFC4627, to represent the internally-defined 'int'/'double'
+types described below to external users, which QEMU internally converts to
+64-bit signed integers, and then to the specific internally-defined/native C
+type described by the schema, and vice-versa).
+
+This document does not attempt to describe the structure of data as represented
+by these "external" interfaces (since, how QAPI is exposed externally is
+dependent on Visitor "plug-ins", such as QMP, which are free to use whatever
+encodings/representations are appropriate for the use-cases they serve).
+
+Instead, we describe here only the abstract, internal representations that
+QAPI is built around, and, in some cases, the native C representations that
+those representations ultimately map to.
+
+=== Complex types ===
+
+A complex type is a dictionary with one of more keys.  This corresponds to a
+struct in C or an Object in JSON.  An example of a complex type is:
 
  { 'type': 'MyType',
    'data': { 'member1': 'str', 'member2': 'int', '*member3': 'str' } }
 
+In this example we've defined 'MyType' to be a dictionary with 3 members:
+'member1', 'member2', and 'member3', or type 'str', 'int', and 'str',
+respectively.
+
 The use of '*' as a prefix to the name means the member is optional.  Optional
 members should always be added to the end of the dictionary to preserve
 backwards compatibility.
 
-An enumeration type is a dictionary containing a single key who's value is a
-list of strings.  An example enumeration is:
+In cases where a type will only be used once, you also have the option of
+declaring the type "anonymously" by embedding it into a type or command
+definition rather than naming the type and defining it seperately.  For
+example, the following definition:
+
+ { 'type': 'MyOuterType',
+   'data': { 'member1': { 'inner_member1': 'str', 'inner_member2': 'int' } } }
+
+will, functionally at least, result in the same internal/external
+representation as:
+
+ { 'type': 'MyInnerType',
+   'data': { 'inner_member1': 'str', 'inner_member2': 'int' } }
+
+ { 'type': 'MyOuterType',
+   'data': { 'member1': 'MyInnerType' } }
+
+=== Union types ===
+
+Union types are also supported:
+
+ { 'union': 'MyUnion',
+   'data': {
+        'member-for-this': 'MyType1',
+        'member-for-that': 'MyType2',
+    } }
+
+This roughly corresponds to a complex type of the following form:
+
+ { 'enum': 'MyUnionKind',
+   'data': [ 'member-for-this', 'member-for-that' ] }
+
+ { 'type': 'MyUnion',
+   'data': { 'type': 'MyUnionKind', 'data': 'MyType1' } }
+
+or
+
+ { 'type': 'MyUnion',
+   'data': { 'type': 'MyUnionKind', 'data': 'MyType2' } }
+
+Whether the (nested) 'data' field is of type 'MyType1' or 'MyType2' is
+dependent on the (nested) 'type' field's value.
+
+=== Enum types ===
+
+An enumeration type is essentially a 'str' type, but restricted to an allowed
+range that is specified by the list of strings in the enum definition. An
+example enumeration is:
 
  { 'enum': 'MyEnum', 'data': [ 'value1', 'value2', 'value3' ] }
 
-Generally speaking, complex types and enums should always use CamelCase for
-the type names.
+In terms of external/wire users, this is basically a way to specify that a
+field has a 'str' type, but that the value must be one of 'value1', 'value2',
+or 'value3'.
+
+Generally speaking, complex types, unions, and enums should always use
+CamelCase for the type names.  Lower-case types are reserved for the
+internally-defined types described below.
+
+=== List types ===
+
+List types can be defined by enclosing a named type (Complex/Union/Enum
+types as described above) in brackets.  For example:
+
+ { 'type': 'MyInnerType',
+   'data': { 'inner_member1': 'str', 'inner_member2': 'int' } }
+
+ { 'type': 'MyOuterType',
+   'data': { 'member1': ['MyInnerType'] } }
+
+defines MyOuterType as being a complex type whose 'member1' field is a list
+of 0 or more instances of 'MyInnerType'
+
+Lists of internally-defined types (described below) are not currently supported.
+You can, however, encapsulate internally-defined type with a simple named type
+so that you can create a list.  For instance:
+
+ { 'type': 'String',
+   'data': { 'str': 'str' } }
+
+would allow for ['String'] list types in commands or named type definitions.
+
+=== Internally-defined types ===
+
+In addition to the types specified above, member fields can also be of the
+following internally-defined/'native' types:
+
+ 'int': a signed 64-bit integer
+ 'int8': a signed 8-bit integer
+ 'int16': a signed 16-bit integer
+ 'int32': a signed 32-bit integer
+ 'int64': a signed 64-bit integer
+ 'uint8': an unsigned 8-bit integer
+ 'uint16': an unsigned 16-bit integer
+ 'uint32': an unsigned 32-bit integer
+ 'uint64': an unsigned 64-bit integer
+ 'double': a double-precision floating point value
+ 'str': a pointer to a null-terminated char*
+ 'bool': a bool
+
+=== Commands ===
 
 Commands are defined by using a list containing three members.  The first
 member is the command name, the second member is a dictionary containing
@@ -67,6 +189,107 @@ An example command is:
 
 Command names should be all lower case with words separated by a hyphen.
 
+See the output of the code generators (usage information below)
+given a schema declaration of this sort for the specifics regarding the
+internal/native C representations.
+
+
+== Internally-used schema extensions ==
+
+The following describes some extensions that have been added to the QAPI
+schema format to handle additional use cases such as QIDL-based device
+serialization.  These extensions should be considered
+"unstable"/"unsupported" for external users, and should not be used
+in externally-advertised QAPI schemas, or to define externally-accessible
+interfaces, until they've been deemed stable.  These are also currently
+non-applicable for QAPI type-generators, and are meant to better describe
+an existing C type for QAPI visitor-generators.
+
+We document these extensions here now as a reference for developers.
+
+Currently these extensions are done on a per-field basis by using a more
+verbose syntax and designating that field as being "annotated". For instance,
+starting with a "complex" type such as:
+
+ { 'type': 'MyType',
+   'data': { 'member1': 'str', 'member2': 'int', 'member3': ['SomeType']
+    } }
+
+we can add type-specific annotations for a particular field, say,
+'member3', by using the following form:
+
+ { 'type': 'MyType',
+   'data': { 'member1': 'str', 'member2': 'int',
+             'member3': {
+              '<annotated>': 'true',
+              'type': ['SomeType'],
+              'annotation1': ...,
+              ...
+              'annotationN': ...,
+ } } }
+
+As noted, the annotations you can use depend on the type of the field.
+Currently, the types that support annotations are:
+
+ <ComplexType>
+   The following annotations are available for a named/user-defined
+   complex type (as described previously in this document):
+
+     'embedded':
+       This annotation field can be used to provide a hint to the QAPI code
+       generator that the type is represented natively as an embedded struct,
+       as opposed to a struct ptr, as is normally the case. The allowed
+       values for the field are strings: either 'true' or 'false'.
+
+ ['<ComplexType>']
+ ['str' | 'int' | 'uint64' | 'str' | <etc>]
+   An annotated list type has an implied effect of hinting to the QAPI code
+   generator that the list is represented internally an array rather than
+   a linked list structure, as is normally the case. This implied behavior
+   is due to that fact that this is currently the common case for annotated
+   lists. Support for annotated lists that should still be handled internally
+   has linked list can be accomplished in the future by adding an addtional
+   annotation.
+
+   This syntax also allows you to specify a list of
+   internally-defined/'native' types, as described above, which is not
+   supported when using the non-annotated syntax.
+
+   The following annotations are available:
+
+     'array_capacity':
+       Specifies the number of elements in the array. This accepts
+       a numerical value of 0 or more, or a C expression
+       (see: 'array_size'). If this annotation is absent,
+       the 'array_size' annotation must be provided, and will be
+       treated both as the capacity of the array and the number of
+       initialized/relevant elements therein.
+
+     'array_size':
+       For cases where the number of elements that should be sent/read
+       differs from the capacity of the array field where those elements
+       are/will be stored, you can use this annotation to hint the QAPI
+       code generator accordingly. Possible values are either a numerical
+       value of 0 or more (but <= array_capacity), a string value that
+       specifies an integer field within the same instance of the parent
+       type that stores the array size, or a C expression.
+
+       To specify a C expression, use a string of the form:
+       '(<expression>)', such as '(sizeof_some_array() + 4)', for example.
+       The expression will be evaluated in the context of the generated
+       visitor code. In the case of QIDL, this will be equivalent to
+       having an expression within a function at the end of the
+       compilation unit for which the code was generated. This may not
+       hold for non-QIDL users, however, in which case the expression
+       must only rely on constants or global-accessible variables/functions.
+
+       In either case, however, you can reference the field 'obj', which
+       will be a pointer to the instance of the parent class being
+       processed by the generated code. See the generated visitor code for
+       specifics, and use this sparingly (read: only if you absolutely
+       have to), as support for referencing the parent obj in this fashion
+       is likely to be dropped unless a pressing use-case arises.
+
 
 == Code generation ==
 
-- 
1.7.9.5

[Qemu-devel] [PATCH v4 17/26] qom-fuse: force single-threaded mode to avoid QMP races

[Michael Roth]

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.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
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

[Qemu-devel] [PATCH v4 18/26] qom-fuse: workaround for truncated properties > 4096

[Michael Roth]

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>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.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

[Qemu-devel] [PATCH v4 19/26] module additions for schema registration

[Michael Roth]

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.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 5b357a3..0747614 100644
--- a/vl.c
+++ b/vl.c
@@ -2412,6 +2412,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

[Qemu-devel] [PATCH v4 20/26] qdev: move Property-related declarations to qdev-properties.h

[Michael Roth]

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
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 c6ac636..6f2c7f2 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);
@@ -214,100 +184,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

[Qemu-devel] [PATCH v4 21/26] qidl: add documentation

[Michael Roth]

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
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..7d17cdc
--- /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 some way duplicated or not guest visible.
+This more often than 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 restore 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 occasions 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.
+
+## q_immutable 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 **q_immutable** 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 q_immutable *chr;
+    };
+
+When reviewing patches that make use of the **q_immutable** 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*.  Timers 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 **q_derived** and work with it
+accordingly.
+
+### q_derived 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 the *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 q_derived int_pending;
+        CharDriverState q_immutable *chr;
+    };
+
+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?
+
+### q_broken 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, q_broken 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 q_broken thr;
+        uint8_t lsr;
+        uint8_t ier;
+
+        int q_derived int_pending;
+        CharDriverState q_immutable *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 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.
+
+### q_elsewhere fields
+
+In some cases state is saved-off when serializing a separate device
+structure. For example, IDEState stores a reference to an IDEBus structure:
+
+    QIDL_DECLARE(IDEState) {
+        IDEBus q_elsewhere *bus;
+        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 *q_elsewhere* marker to note that the
+IDEBus* field in IDEState should not be saved since that is handled
+elsewhere.
+
+### q_optional fields
+
+Some state is only serialized in certain circumstances. To handle these cases
+you can specify a ***q_optional*** 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 q_optional data[UART_FIFO_LENGTH];
+        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 q_size(count) data[UART_FIFO_LENGTH];
+        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

[Qemu-devel] [PATCH v4 22/26] qidl: add lexer library (based on QC parser)

[Michael Roth]

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

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
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..96c6c1a
--- /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 or later.
+# 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

[Qemu-devel] [PATCH v4 23/26] qidl: add C parser (based on QC parser)

[Michael Roth]

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.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 scripts/qidl_parser.py |  269 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 269 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..7e8573f
--- /dev/null
+++ b/scripts/qidl_parser.py
@@ -0,0 +1,269 @@
+#
+# 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 or later.
+# 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
+        expression = params[1]
+        if expression.isdigit():
+            expression = int(expression)
+        node['array_size'] = expression
+    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
+            if expression.isdigit():
+                expression = int(expression)
+            node['array_size'] = expression
+        else:
+            if expression.isdigit():
+                expression = int(expression)
+            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

[Qemu-devel] [PATCH v4 24/26] qidl: add QAPI-based code generator

[Michael Roth]

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.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 scripts/qidl.py |  290 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 290 insertions(+)
 create mode 100644 scripts/qidl.py

diff --git a/scripts/qidl.py b/scripts/qidl.py
new file mode 100644
index 0000000..4e0880e
--- /dev/null
+++ b/scripts/qidl.py
@@ -0,0 +1,290 @@
+#
+# QIDL Code Generator
+#
+# Copyright IBM, Corp. 2012
+#
+# Authors:
+#  Anthony Liguori <aliguori@us.ibm.com>
+#  Michael Roth    <mdroth@linux.vnet.ibm.com>
+#
+# This work is licensed under the terms of the GNU GPLv2 or later.
+# See the COPYING file in the top-level directory.
+
+from ordereddict import OrderedDict
+from qidl_parser import parse_file
+from qapi import parse_schema, mcgen, camel_case, de_camel_case
+from qapi_visit import generate_visit_struct, push_indent, pop_indent
+import sys
+import getopt
+import os
+import errno
+
+def qapi_schema(node):
+    schema = OrderedDict()
+    data = OrderedDict()
+    fields = None
+    if node.has_key('typedef'):
+        schema['type'] = node['typedef']
+        fields = node['type']['fields']
+    elif node.has_key('struct'):
+        schema['type'] = node['struct']
+        fields = node['fields']
+    else:
+        raise Exception("top-level neither typedef nor struct")
+
+    for field in fields:
+        if filter(lambda x: field.has_key(x), \
+                ['is_derived', 'is_immutable', 'is_broken', 'is_function', \
+                 'is_nested_decl', 'is_elsewhere']):
+            continue
+
+        description = OrderedDict()
+
+        if field['type'].endswith('_t'):
+            typename = field['type'][:-2]
+        elif field['type'].startswith('struct '):
+            typename = field['type'].split(" ")[1]
+        elif field['type'].startswith('enum '):
+            typename = 'int'
+        elif field['type'] == "_Bool":
+            typename = 'bool'
+        elif field['type'].endswith("char") and field.has_key('is_pointer'):
+            typename = 'str';
+        elif field['type'] == 'int':
+            typename = 'int32'
+        elif field['type'] == 'unsigned int':
+            typename = 'uint32'
+        elif field['type'] == 'char':
+            typename = 'uint8'
+        else:
+            typename = field['type']
+
+        if field.has_key('is_array') and field['is_array']:
+            description['type'] = [typename]
+            description['<annotated>'] = 'true'
+            if field.has_key('array_size'):
+                description['array_size'] = field['array_size']
+            if field.has_key('array_capacity'):
+                description['array_capacity'] = field['array_capacity']
+        elif camel_case(de_camel_case(typename)) == typename and \
+            (not field.has_key('is_pointer') or not field['is_pointer']):
+            description['type'] = typename
+            description['<annotated>'] = 'true'
+            description['embedded'] = 'true'
+        else:
+            description = typename
+
+        if field.has_key('is_optional') and field['is_optional']:
+            data["*" + field['variable']] = description
+        else:
+            data[field['variable']] = description
+
+    schema['data'] = data
+    return schema
+
+def parse_schema_file(filename):
+    return parse_schema(open(filename, 'r'))
+
+def write_file(output, filename):
+    if filename:
+        output_file = open(filename, 'w')
+    else:
+        output_file = sys.stdout
+    output_file.write(output)
+    if filename:
+        output_file.close()
+
+def property_list(node):
+    prop_list = []
+    fields = None
+    if node.has_key('typedef'):
+        state = node['typedef']
+        fields = node['type']['fields']
+    elif node.has_key('struct'):
+        state = node['struct']
+        fields = node['fields']
+    else:
+        raise Exception("top-level neither typedef nor struct")
+
+    for field in fields:
+        if not field.has_key('is_property'):
+            continue
+
+        for arglist in field['property_fields']:
+            if field['variable'] == 'devfn':
+                typename = 'pci_devfn'
+            elif field['type'].endswith('_t'):
+                typename = field['type'][:-2]
+            elif field['type'] == "_Bool":
+                typename = 'bool'
+            elif field.has_key('is_pointer'):
+                if field['type'] in ("char", "const char"):
+                    typename = "string"
+                elif field['type'] == "void":
+                    typename = "ptr"
+            else:
+                typename = field['type']
+
+            prop = {}
+            prop['name'] = arglist[0]
+            prop['state'] = state
+            prop['type'] = typename
+            prop['field'] = field['variable']
+            if len(arglist) == 2:
+                prop['default'] = arglist[1]
+            elif len(arglist) == 3:
+                prop['type'] = 'bit'
+                prop['bit'] = arglist[1]
+                prop['default'] = arglist[2]
+
+            prop_list.append(prop)
+
+    return prop_list
+
+def generate_include(include_path):
+    return mcgen('''
+#include "%(include)s"
+''',
+                       include=include_path)
+
+def generate_property_bit(type_name, prop):
+    if prop.has_key('default'):
+        return mcgen('''
+        DEFINE_PROP_BIT(%(name)s, %(state)s, %(field)s, %(bit)s, %(default)s),
+''',
+                        name=prop['name'], state=prop['state'],
+                        field=prop['field'], bit=prop['bit'],
+                        default=prop['default'])
+    return mcgen('''
+        DEFINE_PROP_BIT(%(name)s, %(state)s, %(field)s, %(bit)s),
+''',
+                 name=prop['name'], state=prop['state'],
+                 field=prop['field'], bit=prop['bit'])
+
+def generate_property(type_name, prop):
+    if prop.has_key('default'):
+        return mcgen('''
+        DEFINE_PROP_%(type)s(%(name)s, %(state)s, %(field)s, %(default)s),
+''',
+                        type=prop['type'].upper(),  name=prop['name'],
+                        state=prop['state'], field=prop['field'],
+                        default=prop['default'])
+    return mcgen('''
+        DEFINE_PROP_%(type)s(%(name)s, %(state)s, %(field)s),
+''',
+                 type=prop['type'].upper(),  name=prop['name'],
+                 state=prop['state'], field=prop['field'])
+
+def generate_properties(type_name, prop_list=[]):
+    output = ""
+
+    for prop in prop_list:
+        if prop['type'] == 'bit':
+            output += generate_property_bit(type_name, prop)
+        else:
+            output += generate_property(type_name, prop)
+
+    output += mcgen('''
+        DEFINE_PROP_END_OF_LIST()
+''')
+
+    return output
+
+def generate_qidl_registration(type_name, schema, do_state, prop_list=[]):
+    schema_text = ""
+    for line in schema.to_json().split("\n"):
+        schema_text += "\n\"%s\\n\"" % line
+    visitor = "NULL"
+    if do_state:
+        visitor = "visit_type_%s" % type_name
+
+    return mcgen('''
+static char *%(type_name)s_get_schema(Object *obj, Error **errp)
+{
+    return g_strdup(qidl_data_%(type_name)s.schema_json_text);
+}
+
+static void %(type_name)s_register_qidl(void)
+{
+    static Property properties[] = {
+%(properties)s
+    };
+    ObjectProperty *schema_link;
+
+    qidl_data_%(type_name)s.properties = properties;
+    qidl_data_%(type_name)s.visitor = %(visitor)s;
+    qidl_data_%(type_name)s.schema_json_text = %(schema_text)s;
+
+    schema_link = object_property_find(container_get(object_get_root(), "/qidl/schemas"),
+                                       "%(type_name)s", NULL);
+    qidl_data_%(type_name)s.schema_obj = container_get(object_get_root(), "/qidl/schemas/%(type_name)s");
+    if (!schema_link) {
+        object_property_add_str(qidl_data_%(type_name)s.schema_obj, "json_text",
+                                %(type_name)s_get_schema, NULL, NULL);
+    }
+}
+
+qidl_init(%(type_name)s_register_qidl)
+''',
+                 type_name=type_name, schema_text=schema_text, visitor=visitor,
+                 properties=generate_properties(type_name, prop_list))
+
+def main(argv=[]):
+    try:
+        opts, args = getopt.gnu_getopt(argv[1:], "o:cd:I:",
+                                       ["output-filepath=", "schema-filepath=",
+                                        "include="])
+    except getopt.GetoptError, err:
+        print >> sys.stderr, err
+        return 1
+
+    output_filepath = None
+    schema_filepath = None
+    includes = []
+    for o, a in opts:
+        if o in ("-f", "--output-filepath"):
+            output_filepath = a
+        if o in ("-s", "--schema-filepath"):
+            schema_filepath = a
+        elif o in ("-I", "--include"):
+            includes.append(a)
+
+    nodes = parse_file(sys.stdin)
+    if not nodes:
+        return 2
+
+    if os.path.dirname(output_filepath) != "":
+        try:
+            os.makedirs(os.path.dirname(output_filepath))
+        except os.error, e:
+            if e.errno != errno.EEXIST:
+                raise
+    output = ""
+    schema_text_all = ""
+    for include in includes:
+        output += generate_include(include)
+    for node in nodes:
+        do_state = False
+        schema = qapi_schema(node)
+        prop_list = []
+        # qapi parser expects iteration to be line-by-line
+        schema_text = schema.to_json()
+        expr = parse_schema(schema_text.split("\n"))[0]
+        schema_text_all += schema_text + "\n\n"
+
+        if node.has_key('do_state') and node['do_state']:
+            do_state = True
+            output += generate_visit_struct(expr['type'], expr['data'], True)
+        if node.has_key('do_properties') and node['do_properties']:
+            prop_list = property_list(node)
+
+        output += generate_qidl_registration(expr['type'], schema, do_state, prop_list)
+
+    if schema_filepath and schema_text_all != "":
+        write_file(schema_text_all, schema_filepath)
+    write_file(output, output_filepath)
+
+    return 0
+
+if __name__ == '__main__':
+    sys.exit(main(sys.argv))
-- 
1.7.9.5

[Qemu-devel] [PATCH v4 25/26] qidl: qidl.h, definitions for qidl annotations

[Michael Roth]

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qidl.h |  105 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 105 insertions(+)
 create mode 100644 qidl.h

diff --git a/qidl.h b/qidl.h
new file mode 100644
index 0000000..b4c9b02
--- /dev/null
+++ b/qidl.h
@@ -0,0 +1,105 @@
+/*
+ * 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"
+
+/* must be "called" in any C files that make use of QIDL-generated code */
+#define QIDL_ENABLE()
+
+/* 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
+ */
+#ifdef QIDL_GEN
+
+#define QIDL(...) QIDL(__VA_ARGS__)
+#define QIDL_START(name, ...) QIDL_START(name, ##__VA_ARGS__)
+
+#else
+
+#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 /* QIDL_ENABLED */
+
+#endif /* QIDL_GEN */
+
+/* QIDL annotations/markers
+ *
+ * q_immutable: state is fully restorable via device
+ *   [re-]initialization/realization
+ *
+ * q_derived: state can be fully reconstructed from other fields (and will be,
+ *   via [re-]initialization of the device or a separate hook)
+ *
+ * q_broken: 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
+ *
+ * q_optional: <field> should only be serialized if the field by the name of
+ *   has_<field> is true
+ *
+ * q_elsewhere: state should be serialized, but is done so elsewhere (for
+ *   instance, by another device with a pointer to the same data)
+ *
+ * q_size(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.
+ *
+ * q_property(<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 q_immutable QIDL(immutable)
+#define q_derived QIDL(derived)
+#define q_broken QIDL(broken)
+#define q_optional QIDL(optional)
+#define q_elsewhere QIDL(elsewhere)
+#define q_size(...) QIDL(size_is, ##__VA_ARGS__)
+#define q_property(name, ...) QIDL(property, name, ##__VA_ARGS__)
+
+#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
-- 
1.7.9.5

[Qemu-devel] [PATCH v4 26/26] qidl: unit tests and build infrastructure

[Michael Roth]

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 Makefile                   |    2 +
 configure                  |    1 +
 rules.mak                  |   25 ++-
 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          |  376 ++++++++++++++++++++++++++++++++++++++++++++
 8 files changed, 551 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 da4360f..d3405e0 100644
--- a/Makefile
+++ b/Makefile
@@ -236,6 +236,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)
 
@@ -405,6 +406,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/configure b/configure
index 77b2462..39e34e8 100755
--- a/configure
+++ b/configure
@@ -3235,6 +3235,7 @@ if test "$debug_tcg" = "yes" ; then
 fi
 if test "$debug" = "yes" ; then
   echo "CONFIG_DEBUG_EXEC=y" >> $config_host_mak
+  echo "CONFIG_DEBUG_QIDL=y" >> $config_host_mak
 fi
 if test "$strip_opt" = "yes" ; then
   echo "STRIP=${strip}" >> $config_host_mak
diff --git a/rules.mak b/rules.mak
index 1b173aa..a1dd73c 100644
--- a/rules.mak
+++ b/rules.mak
@@ -14,8 +14,31 @@ MAKEFLAGS += -rR
 # Flags for dependency generation
 QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(*D)/$(*F).d
 
+# Debug options for QIDL
+ifdef CONFIG_DEBUG_QIDL
+QIDL_FLAGS = --schema-filepath=$(*D)/qidl-generated/$(*F).qidl.schema
+endif
+
 %.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.*)
+	$(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 $(QIDL_FLAGS) \
+	    --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..535a153
--- /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 q_immutable a;
+    int32_t b;
+    uint32_t q_immutable c;
+    uint32_t d;
+    uint64_t q_immutable e;
+    uint64_t q_property("f", 42) f;
+    char q_property("g") *g;
+    char q_property("h") q_immutable *h;
+};
+
+#endif
diff --git a/tests/test-qidl-linked.c b/tests/test-qidl-linked.c
new file mode 100644
index 0000000..641e561
--- /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 q_immutable a;
+    int32_t b;
+    uint32_t q_immutable c;
+    uint32_t d;
+    uint64_t q_immutable e;
+    uint64_t q_property("f", 42) f;
+    char q_property("g") *g;
+    char q_property("h") q_immutable *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..f2616c4
--- /dev/null
+++ b/tests/test-qidl.c
@@ -0,0 +1,376 @@
+/*
+ * 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) {
+    int32_t q_immutable a;
+    int32_t b;
+    uint32_t q_immutable c;
+    uint32_t d;
+    uint64_t q_immutable e;
+    uint64_t q_property("f", 42) f;
+    char q_property("g") *g;
+    char q_property("h") q_immutable *h;
+};
+
+static void fill_test_struct_main(TestStructMain *s)
+{
+    s->a = 42;
+    s->b = INT32_MAX;
+    s->c = 43;
+    s->d = UINT32_MAX;
+    s->e = 44;
+    s->f = UINT64_MAX;
+    s->g = g_strdup("test string g");
+    s->h = g_strdup("test string h");
+}
+
+static void check_test_struct_main(TestStructMain *s)
+{
+    g_assert_cmpint(s->a, ==, 0);
+    g_assert_cmpint(s->b, ==, INT32_MAX);
+    g_assert_cmpint(s->c, ==, 0);
+    g_assert_cmpint(s->d, ==, UINT32_MAX);
+    g_assert_cmpint(s->e, ==, 0);
+    g_assert_cmpint(s->f, ==, UINT64_MAX);
+    g_assert_cmpstr(s->g, ==, "test string g");
+    g_assert(s->h == NULL);
+}
+
+static void free_test_struct_main(TestStructMain *s)
+{
+    g_free(s->g);
+    g_free(s->h);
+    g_free(s);
+}
+
+static void check_test_struct_main_properties(void)
+{
+    Property *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 annotations in main() object file */
+static void test_main_object_annotations(gconstpointer opaque)
+{
+    TestStructMain *s1, *s2 = NULL;
+    QmpInputVisitor *qiv;
+    QmpOutputVisitor *qov;
+    QObject *s1_obj;
+    Error *err = NULL;
+
+    s1 = g_malloc0(sizeof(TestStructMain));
+    fill_test_struct_main(s1);
+
+    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);
+    free_test_struct_main(s1);
+
+    s2 = g_malloc0(sizeof(TestStructMain));
+    QIDL_VISIT_TYPE(TestStructMain, qmp_input_get_visitor(qiv), &s2, NULL, &err);
+    g_assert(err == NULL);
+    check_test_struct_main(s2);
+
+    qmp_input_visitor_cleanup(qiv);
+    free_test_struct_main(s2);
+
+    check_test_struct_main_properties();
+}
+
+static void fill_test_struct_included(TestStructIncluded *s)
+{
+    fill_test_struct_main((TestStructMain *)s);
+}
+
+static void check_test_struct_included(TestStructIncluded *s)
+{
+    check_test_struct_main((TestStructMain *)s);
+}
+
+static void free_test_struct_included(TestStructIncluded *s)
+{
+    free_test_struct_main((TestStructMain *)s);
+}
+
+static void check_test_struct_included_properties(void)
+{
+    Property *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);
+}
+
+/* exercise generated code from annotations in included header files */
+static void test_header_file_annotations(gconstpointer opaque)
+{
+    TestStructIncluded *s1, *s2 = NULL;
+    QmpInputVisitor *qiv;
+    QmpOutputVisitor *qov;
+    QObject *s1_obj;
+    Error *err = NULL;
+
+    s1 = g_malloc0(sizeof(TestStructIncluded));
+    fill_test_struct_included(s1);
+
+    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);
+    free_test_struct_included(s1);
+
+    s2 = g_malloc0(sizeof(TestStructIncluded));
+    QIDL_VISIT_TYPE(TestStructIncluded, qmp_input_get_visitor(qiv), &s2, NULL, &err);
+    g_assert(err == NULL);
+    check_test_struct_included(s2);
+
+    qmp_input_visitor_cleanup(qiv);
+    free_test_struct_included(s2);
+
+    check_test_struct_included_properties();
+}
+
+typedef struct TestStructComplex TestStructComplex;
+
+QIDL_DECLARE(TestStructComplex) {
+    int8_t q_size(2) array1[4];
+    size_t array2_count;
+    int32_t q_size(array2_count) array2[8];
+    int16_t q_size((2*3)) array3[16];
+    bool has_optional_array;
+    int32_t optional_array_count;
+    int8_t q_optional q_size(optional_array_count) optional_array[16];
+    TestStructMain struct_array[32];
+    int64_t struct_array2_count;
+    TestStructMain q_size(struct_array2_count) struct_array2[32];
+};
+
+static void fill_test_struct_complex(TestStructComplex *s)
+{
+    int i;
+
+    for (i = 0; i < 4; i++) {
+        s->array1[i] = i*2;
+    }
+
+    s->array2_count = 6;
+    for (i = 0; i < s->array2_count; i++) {
+        s->array2[i] = i*2;
+    }
+
+    for (i = 0; i < 6; i++) {
+        s->array3[i] = i*2;
+    }
+
+    s->has_optional_array = true;
+    s->optional_array_count = 15;
+    for (i = 0; i < s->optional_array_count; i++) {
+        s->optional_array[i] = i*2;
+    }
+
+    for (i = 0; i < 32; i++) {
+        fill_test_struct_main(&s->struct_array[i]);
+    }
+
+    s->struct_array2_count = 31;
+    for (i = 0; i < s->struct_array2_count; i++) {
+        fill_test_struct_main(&s->struct_array2[i]);
+    }
+}
+
+static void check_test_struct_complex(TestStructComplex *s)
+{
+    int i;
+
+    for (i = 0; i < 4; i++) {
+        if (i < 2) {
+            g_assert_cmpint(s->array1[i], ==, i*2);
+        } else {
+            g_assert_cmpint(s->array1[i], ==, 0);
+        }
+    }
+
+    g_assert_cmpint(s->array2_count, ==, 6);
+    for (i = 0; i < sizeof(s->array2)/sizeof(int32_t); i++) {
+        if (i < s->array2_count) {
+            g_assert_cmpint(s->array2[i], ==, i*2);
+        } else {
+            g_assert_cmpint(s->array2[i], ==, 0);
+        }
+    }
+
+    g_assert(s->has_optional_array);
+    g_assert_cmpint(s->optional_array_count, ==, 15);
+    for (i = 0; i < sizeof(s->optional_array)/sizeof(int16_t); i++) {
+        if (i < s->optional_array_count) {
+            g_assert_cmpint(s->optional_array[i], ==, i*2);
+        } else {
+            g_assert_cmpint(s->optional_array[i], ==, 0);
+        }
+    }
+
+    for (i = 0; i < 32; i++) {
+        check_test_struct_main(&s->struct_array[i]);
+    }
+
+    g_assert_cmpint(s->struct_array2_count, ==, 31);
+    for (i = 0; i < sizeof(s->struct_array2)/sizeof(TestStructMain); i++) {
+        if (i < s->struct_array2_count) {
+            check_test_struct_main(&s->struct_array2[i]);
+        } else {
+            int j;
+            uint8_t *ptr = (uint8_t *)&s->struct_array2[i];
+            for (j = 0; j < sizeof(TestStructMain); j++) {
+                g_assert_cmpint(ptr[0], ==, 0);
+            }
+        }
+    }
+}
+
+static void test_array_annotations(gconstpointer opaque)
+{
+    TestStructComplex *s1, *s2 = NULL;
+    QmpInputVisitor *qiv;
+    QmpOutputVisitor *qov;
+    QObject *s1_obj;
+    Error *err = NULL;
+
+    s1 = g_malloc0(sizeof(TestStructComplex));
+    fill_test_struct_complex(s1);
+
+    qov = qmp_output_visitor_new();
+    QIDL_VISIT_TYPE(TestStructComplex, 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);
+
+    s2 = g_malloc0(sizeof(TestStructComplex));
+    QIDL_VISIT_TYPE(TestStructComplex, qmp_input_get_visitor(qiv), &s2, NULL, &err);
+    g_assert(err == NULL);
+    check_test_struct_complex(s2);
+
+    qmp_input_visitor_cleanup(qiv);
+    g_free(s2);
+}
+
+typedef struct TestStructComplex2 TestStructComplex2;
+
+QIDL_DECLARE(TestStructComplex2) {
+    bool has_struct1;
+    TestStructMain q_optional *struct1;
+    TestStructMain embedded_struct1;
+};
+
+static void test_complex_annotations(gconstpointer opaque)
+{
+    TestStructComplex2 *s1, *s2 = NULL;
+    QmpInputVisitor *qiv;
+    QmpOutputVisitor *qov;
+    QObject *s1_obj;
+    Error *err = NULL;
+
+    s1 = g_malloc0(sizeof(TestStructComplex));
+    s1->has_struct1 = true;
+    s1->struct1 = g_malloc0(sizeof(TestStructMain));
+    fill_test_struct_main(s1->struct1);
+    fill_test_struct_main(&s1->embedded_struct1);
+
+    qov = qmp_output_visitor_new();
+    QIDL_VISIT_TYPE(TestStructComplex2, 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);
+    free_test_struct_main(s1->struct1);
+    g_free(s1);
+
+    s2 = g_malloc0(sizeof(TestStructComplex2));
+    QIDL_VISIT_TYPE(TestStructComplex2, qmp_input_get_visitor(qiv), &s2, NULL, &err);
+    g_assert(err == NULL);
+    check_test_struct_main(s2->struct1);
+    check_test_struct_main(&s2->embedded_struct1);
+
+    qmp_input_visitor_cleanup(qiv);
+    free_test_struct_main(s2->struct1);
+    g_free(s2);
+}
+
+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/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_add_data_func("/qidl/build_test/array_annotations", NULL,
+                         test_array_annotations);
+    g_test_add_data_func("/qidl/build_test/complex_annotations", NULL,
+                         test_complex_annotations);
+
+    g_test_run();
+
+    return 0;
+}
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH v4 00/26] Add infrastructure for QIDL-based device serialization

[Paolo Bonzini]

Il 12/10/2012 23:10, Michael Roth ha scritto:
> 
> These patches add infrastructure and unit tests for QIDL, which provides
> a serialization framework for QEMU device structures by generating visitor
> routines for device structs based on simple field annotations. Examples of
> how this is done are included in patch 17, but, for brevity, a sample struct
> such as this:
> 
>     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;
> 
> can now be made serializable with the following changes:
> 
>     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 q_derived int_pending; /* whether we have a pending queued interrupt */
>         CharDriverState q_immutable *chr; /* backend */
>     };
> 
> To make use of generated visitor code, and .c file need only call the
> QIDL_ENABLE() somewhere in the code body, which will then give it access to
> visitor routines for any QIDL-ified device structures in that file, or included
> from other files. These routines can then be used for
> serialization/deserialization of the device state in a manner suitable for tasks
> such as introspection/testing (generally via a r/w 'state' QOM property) or
> migration.
> 
> The overall goal is to expose all migrateable device state in this manner so
> that we can decouple serialization duties from savevm/VMState and convert them
> into a stable, code-agnostic wire protocol, relying instead on intermediate
> translation routines to handle the work of massaging serialized state into a
> format suitable for the wire and vice-versa.
> 
> The following WIP branch contains the first set of QIDL conversions:
> 
> https://github.com/mdroth/qemu/commits/qidl
> 
> So far i440fx, pcibus, cirrus_vga, uhci, rtc, and isa/piix ide have been
> converted, and I'm hoping to have most common PC devices converted over
> within the next few weeks.

Do you have any plan for scripted conversion?

Paolo

Re: [Qemu-devel] [PATCH v4 24/26] qidl: add QAPI-based code generator

[Paolo Bonzini]

Il 12/10/2012 23:11, Michael Roth ha scritto:
> +        elif field['type'].startswith('enum '):
> +            typename = 'int'

Note that there is support for enum properties in qdev.  Please consider
adding it, though it can be done as a follow-up.

I'm going to play a bit with the series and convert 1 or 2 devices
myself to see how it looks, then I'll give my acked-by.

Paolo

Re: [Qemu-devel] [PATCH v4 26/26] qidl: unit tests and build infrastructure

[Paolo Bonzini]

Il 12/10/2012 23:11, Michael Roth ha scritto:
> +%.qidl.c: %.c $(SRC_PATH)/qidl.h $(addprefix $(SRC_PATH)/scripts/,lexer.py qidl.py qidl_parser.py qapi.py qapi_visit.py)

The rule here is wrong, because %.qidl.c is never produced by the
commands.  The output is in the qidl-generated subdirectory, and that
cannot be expressed with pattern rules.

I was worried that this causes many greps on every invocation of the
Makefile---even though the way you hid them in makefile functions would
hide them from the "make" output.

However, this is not the case.  What happens is quite complex.
hw/foo.qidl.c (as opposed to hw/qidl-generated/foo.qidl.c) will never
exist, but it is a dependency of hw/foo.o, so it is always rebuilt
before it.  But it is not rebuilt on every invocation even though it
doesn't exist, because it only comes into play via implicit rules.

I think this is quite clever and does exactly what you want.  If you
want to keep the qidl-generated directory, I cannot imagine any other
way to do it.  However, please rename %.qidl.c to something QIDL-PP-% so
that it is clear that it is not a "real" file name.

If we decide this is too clever (and it's not all of it, it took me a
while to understand that Makefile functions are needed because
quiet-command expands to a @-prefixed command...), we can drop the
qidl-generated directory.

> +	$(call rm -f $(*D)/qidl-generated/$(*F).qidl.*)

I think this $(call) is not what you want, you need just "@rm -f ..."

Paolo

> +	$(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 $(QIDL_FLAGS) \
> +	    --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    $@"))

Re: [Qemu-devel] [PATCH v4 24/26] qidl: add QAPI-based code generator

[Paolo Bonzini]

Il 15/10/2012 10:12, Paolo Bonzini ha scritto:
> Il 12/10/2012 23:11, Michael Roth ha scritto:
>> +        elif field['type'].startswith('enum '):
>> +            typename = 'int'
> 
> Note that there is support for enum properties in qdev.  Please consider
> adding it, though it can be done as a follow-up.
> 
> I'm going to play a bit with the series and convert 1 or 2 devices
> myself to see how it looks, then I'll give my acked-by.

Ok, so now I played with it a bit.  My main comments, which can all be
tackled as a follow-up, are:

- immutable/derived/broken/elsewhere (and the default, let's call it
serialized) are really five cases of the same QIDL property.  Perhaps
this could be enforced in the extended syntax like this:

    #define q_immutable QIDL(serialize(immutable))
    #define q_derived QIDL(serialize(derived))
    #define q_broken QIDL(serialize(broken))
    #define q_elsewhere QIDL(serialize(elsewhere))

I would also make it possible to explicitly specify the fifth state, if
only for symmetry.

I'm not sure what your plans are for q_derived vs. VMState.  If a field
X is set in pre_save hooks based on field Y, how should the fields be
set?  X is usually not up-to-date, so it should be q_derived.  But Y
cannot be serialized as is, so it should be q_elsewhere.  One of the
two is wrong, which one? :)

- q_properties are also always q_immutable.  I think this should be
enforced in the code generator.

- it would be _much_ better if you could automatically derive properties
information for embedded structs.  For example, Notifiers and qemu_irqs
are always q_immutable.  NotifierLists probably are always q_elsewhere,
because the owner of the notifiers should add themselves back.

In general, if struct X is QIDL_DECLAREd and only has q_immutable
fields, it can be taken as q_immutable.  Hence for example the base
class should not need any decoration; ISADevice will be seen as
q_immutable, but PCIDevice will be seen as serialized.  But even if a
struct is not QIDL_DECLAREd, it  should be possible to apply a tag to a
typedef, and have it always applied to the members.

Paolo

Re: [Qemu-devel] [PATCH v4 24/26] qidl: add QAPI-based code generator

[Michael Roth]

On Mon, Oct 15, 2012 at 03:08:51PM +0200, Paolo Bonzini wrote:
> Il 15/10/2012 10:12, Paolo Bonzini ha scritto:
> > Il 12/10/2012 23:11, Michael Roth ha scritto:
> >> +        elif field['type'].startswith('enum '):
> >> +            typename = 'int'
> > 
> > Note that there is support for enum properties in qdev.  Please consider
> > adding it, though it can be done as a follow-up.
> > 
> > I'm going to play a bit with the series and convert 1 or 2 devices
> > myself to see how it looks, then I'll give my acked-by.
> 
> Ok, so now I played with it a bit.  My main comments, which can all be
> tackled as a follow-up, are:
> 
> - immutable/derived/broken/elsewhere (and the default, let's call it
> serialized) are really five cases of the same QIDL property.  Perhaps
> this could be enforced in the extended syntax like this:
> 
>     #define q_immutable QIDL(serialize(immutable))
>     #define q_derived QIDL(serialize(derived))
>     #define q_broken QIDL(serialize(broken))
>     #define q_elsewhere QIDL(serialize(elsewhere))
> 
> I would also make it possible to explicitly specify the fifth state, if
> only for symmetry.

Agreed, that's a more proper grouping. Though, for consistency with
QIDL(property, ...), I would do QIDL(serialize, ...)

> 
> I'm not sure what your plans are for q_derived vs. VMState.  If a field
> X is set in pre_save hooks based on field Y, how should the fields be
> set?  X is usually not up-to-date, so it should be q_derived.  But Y
> cannot be serialized as is, so it should be q_elsewhere.  One of the
> two is wrong, which one? :)
> 
> - q_properties are also always q_immutable.  I think this should be
> enforced in the code generator.

Agreed.

> 
> - it would be _much_ better if you could automatically derive properties
> information for embedded structs.  For example, Notifiers and qemu_irqs
> are always q_immutable.  NotifierLists probably are always q_elsewhere,
> because the owner of the notifiers should add themselves back.

q_inherit maybe? Otherwise we're overriding "q_default" in subtle
ways that may not always be desired.

> 
> In general, if struct X is QIDL_DECLAREd and only has q_immutable
> fields, it can be taken as q_immutable.  Hence for example the base
> class should not need any decoration; ISADevice will be seen as
> q_immutable, but PCIDevice will be seen as serialized.  But even if a
> struct is not QIDL_DECLAREd, it  should be possible to apply a tag to a
> typedef, and have it always applied to the members.

I'd prefer to stick with a common declaration macro to handle tagging.
How about QIDL_DECLARE(MyImmutableType, q_immutable) to apply a tag to
all members?

> 
> Paolo
> 

Re: [Qemu-devel] [PATCH v4 26/26] qidl: unit tests and build infrastructure

[Michael Roth]

On Mon, Oct 15, 2012 at 12:05:09PM +0200, Paolo Bonzini wrote:
> Il 12/10/2012 23:11, Michael Roth ha scritto:
> > +%.qidl.c: %.c $(SRC_PATH)/qidl.h $(addprefix $(SRC_PATH)/scripts/,lexer.py qidl.py qidl_parser.py qapi.py qapi_visit.py)
> 
> The rule here is wrong, because %.qidl.c is never produced by the
> commands.  The output is in the qidl-generated subdirectory, and that
> cannot be expressed with pattern rules.
> 
> I was worried that this causes many greps on every invocation of the
> Makefile---even though the way you hid them in makefile functions would
> hide them from the "make" output.
> 
> However, this is not the case.  What happens is quite complex.
> hw/foo.qidl.c (as opposed to hw/qidl-generated/foo.qidl.c) will never
> exist, but it is a dependency of hw/foo.o, so it is always rebuilt
> before it.  But it is not rebuilt on every invocation even though it
> doesn't exist, because it only comes into play via implicit rules.
> 
> I think this is quite clever and does exactly what you want.  If you
> want to keep the qidl-generated directory, I cannot imagine any other
> way to do it.  However, please rename %.qidl.c to something QIDL-PP-% so
> that it is clear that it is not a "real" file name.

At the time I was in battle to the death with make to have it let me use
qidl-generated/ directories to store the output, but I don't care too
much anymore :P I think it's worth having if it doesn't cause any problems
though.

There is another way to do this (still using a "dummy" target)  that is a
bit less cryptic:

QIDL-PP-%: %.c qidl.h ...
    <grep and create %.qidl.c>

%.o: QIDL-PP-%
    <build normal or qidl CC>

But make detects that QIDL-PP-% is an intermediate target and removes
the *.qidl.c files after the build, which ends up forcing a re-build
each time. I played around with .INTERMEDIATE/.PRECIOUS directives to
have it keep the files but couldn't get it to cooperate, which is why I
ended up with the current approach.

> 
> If we decide this is too clever (and it's not all of it, it took me a
> while to understand that Makefile functions are needed because
> quiet-command expands to a @-prefixed command...), we can drop the
> qidl-generated directory.
> 
> > +	$(call rm -f $(*D)/qidl-generated/$(*F).qidl.*)

Yes, and in fact this doesn't work. I hadn't noticed it because the old
files get clobbered by the code gen, so it only comes into play if you
take a QIDL'd file and then un-QIDL it (since the %.o rule sees the
.qidl.c file still there and tries to build it accordingly).

I'll go ahead and do a v5 with these changes in place.

> 
> I think this $(call) is not what you want, you need just "@rm -f ..."
> 
> Paolo
> 
> > +	$(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 $(QIDL_FLAGS) \
> > +	    --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    $@"))
> 

Re: [Qemu-devel] [PATCH v4 24/26] qidl: add QAPI-based code generator

[Michael Roth]

On Mon, Oct 15, 2012 at 11:35:46AM -0500, Michael Roth wrote:
> On Mon, Oct 15, 2012 at 03:08:51PM +0200, Paolo Bonzini wrote:
> > Il 15/10/2012 10:12, Paolo Bonzini ha scritto:
> > > Il 12/10/2012 23:11, Michael Roth ha scritto:
> > >> +        elif field['type'].startswith('enum '):
> > >> +            typename = 'int'
> > > 
> > > Note that there is support for enum properties in qdev.  Please consider
> > > adding it, though it can be done as a follow-up.
> > > 
> > > I'm going to play a bit with the series and convert 1 or 2 devices
> > > myself to see how it looks, then I'll give my acked-by.
> > 
> > Ok, so now I played with it a bit.  My main comments, which can all be
> > tackled as a follow-up, are:
> > 
> > - immutable/derived/broken/elsewhere (and the default, let's call it
> > serialized) are really five cases of the same QIDL property.  Perhaps
> > this could be enforced in the extended syntax like this:
> > 
> >     #define q_immutable QIDL(serialize(immutable))
> >     #define q_derived QIDL(serialize(derived))
> >     #define q_broken QIDL(serialize(broken))
> >     #define q_elsewhere QIDL(serialize(elsewhere))
> > 
> > I would also make it possible to explicitly specify the fifth state, if
> > only for symmetry.
> 
> Agreed, that's a more proper grouping. Though, for consistency with
> QIDL(property, ...), I would do QIDL(serialize, ...)
> 

Er, meant to respond to this in my previous reply:

> > 
> > I'm not sure what your plans are for q_derived vs. VMState.  If a field
> > X is set in pre_save hooks based on field Y, how should the fields be
> > set?  X is usually not up-to-date, so it should be q_derived.  But Y
> > cannot be serialized as is, so it should be q_elsewhere.  One of the
> > two is wrong, which one? :)

Why is it that Y can't be serialized as is in this example? X's
derived state depends on it, so Y should be serialized in some form.

Are you referring to a case where VMState sets/sends X currently, but
not Y?

If so, my goal is that serialization is done properly and independently
of VMState. Then, for migration, we serialize it into a QObject (state_obj)
from which a VMState-compatible QObject (wire_obj) can be built and fed to
the wire (either via modified/re-interpreted VMSD, or a re-modeling of the
VMSD into a wire schema (possibly automagically via QIDL at first) from
which a savevm visitor could be generated).

So, for example, a post_serialize() migration hook would have access to an
up-to-date Y in the state_obj, since:

 a) Y is up-date (by virtue of not being q_derived).
 b) Y is q_elsewhere, but our post_serialize() hook has (or can be
    given) access to the Object* that serializes Y, since we're able to
    obtain a pointer to it or one of it's members.

One exception might be that we send X unecessarilly, since it's derived
from Y which is actually q_immutable, but there's no restriction that
q_immutable fields cannot be marked as, say, q_default, so we can add
those in.

Hopefully in most cases the mapping from state_obj <-> wire_obj is
trivial though and is little more than a qdict_copy(), but we can make
the mapping from state_obj <-> wire_obj as flexible as we need to cover
these cases, and potentially other compatibility requirements.

> > 
> > Paolo
> > 

Re: [Qemu-devel] [PATCH v4 24/26] qidl: add QAPI-based code generator

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 2744 bytes --]

Il 15/10/2012 18:35, Michael Roth ha scritto:
>> - immutable/derived/broken/elsewhere (and the default, let's call it
>> serialized) are really five cases of the same QIDL property.  Perhaps
>> this could be enforced in the extended syntax like this:
>>
>>     #define q_immutable QIDL(serialize(immutable))
>>     #define q_derived QIDL(serialize(derived))
>>     #define q_broken QIDL(serialize(broken))
>>     #define q_elsewhere QIDL(serialize(elsewhere))
>>
>> I would also make it possible to explicitly specify the fifth state, if
>> only for symmetry.
> 
> Agreed, that's a more proper grouping. Though, for consistency with
> QIDL(property, ...), I would do QIDL(serialize, ...)

Sure.

>> - it would be _much_ better if you could automatically derive properties
>> information for embedded structs.  For example, Notifiers and qemu_irqs
>> are always q_immutable.  NotifierLists probably are always q_elsewhere,
>> because the owner of the notifiers should add themselves back.
> 
> q_inherit maybe? Otherwise we're overriding "q_default" in subtle
> ways that may not always be desired.

I think the default should be "whatever makes more sense", which for
QIDL_DECLAREd types means making the member q_immutable if it makes
sense for the type.

It's too fragile to expect all subclasses to know whether their
superclass is immutable or has to be serialized, so q_inherit should be
the default.  For atomic types, q_inherit is the same as q_serialized.

That said, an alternative is just to never declare the superclass
q_immutable.  That would work as long as you do not restrict QIDL to
DeviceState subclasses---see attached patch.

>> In general, if struct X is QIDL_DECLAREd and only has q_immutable
>> fields, it can be taken as q_immutable.  Hence for example the base
>> class should not need any decoration; ISADevice will be seen as
>> q_immutable, but PCIDevice will be seen as serialized.  But even if a
>> struct is not QIDL_DECLAREd, it  should be possible to apply a tag to a
>> typedef, and have it always applied to the members.

Hmm, this wasn't the best choice of words.  What I actually meant was
"to apply a tag to a typedef, and have it always applied to members of
that type in other structs".  Like

typedef struct Notifier Notifier q_immutable;

Note that Notifier will never have any serializable state, hence it will
not be QIDL_DECLAREd.  It is just a proxy that specifies a function to call.

While in principle it is possible to change the function at run-time,
that's not the way that Notifiers are used.  That can still be
documented using q_elsewhere, but I think that sane defaults other than
q_serialized are useful to avoid cluttering the declarations.  However,
this is a very minor qualm.

Paolo

[-- Attachment #2: 0001-qidl-allow-marking-Object-as-QIDL_DECLARE.patch --]
[-- Type: text/x-patch, Size: 4444 bytes --]

>From 0629eab1d8cff0764d135b85052dd3ba47a1a198 Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Tue, 16 Oct 2012 09:19:04 +0200
Subject: [PATCH] qidl: allow marking Object as QIDL_DECLARE

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/isa.h              |  8 ++++----
 hw/mc146818rtc.c      |  2 +-
 hw/qdev.h             | 28 ++++++++++++++--------------
 include/qemu/object.h | 11 ++++++-----
 qidl.h                |  6 ++----
 scripts/qidl.py       |  2 +-
 6 file modificati, 28 inserzioni(+), 29 rimozioni(-)

diff --git a/hw/isa.h b/hw/isa.h
index 8fb498a..6c7d815 100644
--- a/hw/isa.h
+++ b/hw/isa.h
@@ -31,11 +31,11 @@ struct ISABus {
     qemu_irq *irqs;
 };
 
-struct ISADevice {
+QIDL_DECLARE(ISADevice) {
     DeviceState qdev;
-    uint32_t isairq[2];
-    int nirqs;
-    int ioport_id;
+    uint32_t q_immutable isairq[2];
+    int q_immutable nirqs;
+    int q_immutable ioport_id;
 };
 
 ISABus *isa_bus_new(DeviceState *dev, MemoryRegion *address_space_io);
diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index f5f4bb4..b29cce5 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -62,7 +62,7 @@ QIDL_ENABLE()
 typedef struct RTCState RTCState;
 
 QIDL_DECLARE(RTCState) {
-    ISADevice q_immutable dev;
+    ISADevice dev;
     MemoryRegion q_immutable io;
     uint8_t cmos_data[128];
     uint8_t cmos_index;
diff --git a/hw/qdev.h b/hw/qdev.h
index 6f2c7f2..3dba377 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -57,22 +57,22 @@ typedef struct DeviceClass {
 
 /* This structure should not be accessed directly.  We declare it here
    so that it can be embedded in individual device state structures.  */
-struct DeviceState {
+QIDL_DECLARE(DeviceState) {
     Object parent_obj;
 
-    const char *id;
-    enum DevState state;
-    QemuOpts *opts;
-    int hotplugged;
-    BusState *parent_bus;
-    int num_gpio_out;
-    qemu_irq *gpio_out;
-    int num_gpio_in;
-    qemu_irq *gpio_in;
-    QLIST_HEAD(, BusState) child_bus;
-    int num_child_bus;
-    int instance_id_alias;
-    int alias_required_for_version;
+    const char q_immutable *id;
+    enum DevState q_immutable state;
+    QemuOpts q_immutable *opts;
+    int q_immutable hotplugged;
+    BusState q_immutable *parent_bus;
+    int q_immutable num_gpio_out;
+    qemu_irq q_immutable *gpio_out;
+    int q_immutable num_gpio_in;
+    qemu_irq q_immutable *gpio_in;
+    QLIST_HEAD(, BusState) q_immutable child_bus;
+    int q_immutable num_child_bus;
+    int q_immutable instance_id_alias;
+    int q_immutable alias_required_for_version;
 };
 
 #define TYPE_BUS "bus"
diff --git a/include/qemu/object.h b/include/qemu/object.h
index cc75fee..cb4a89b 100644
--- a/include/qemu/object.h
+++ b/include/qemu/object.h
@@ -18,6 +18,7 @@
 #include <stdint.h>
 #include <stdbool.h>
 #include "qemu-queue.h"
+#include "qidl.h"
 
 struct Visitor;
 struct Error;
@@ -257,13 +258,13 @@ struct ObjectClass
  * #Object also contains a list of #Interfaces that this object
  * implements.
  */
-struct Object
+QIDL_DECLARE(Object)
 {
     /*< private >*/
-    ObjectClass *class;
-    QTAILQ_HEAD(, ObjectProperty) properties;
-    uint32_t ref;
-    Object *parent;
+    ObjectClass q_immutable *class;
+    QTAILQ_HEAD(, ObjectProperty) q_immutable properties;
+    uint32_t q_elsewhere ref;
+    Object q_immutable *parent;
 };
 
 /**
diff --git a/qidl.h b/qidl.h
index b4c9b02..abff6ba 100644
--- a/qidl.h
+++ b/qidl.h
@@ -18,8 +18,6 @@
 
 #include <glib.h>
 #include "qapi/qapi-visit-core.h"
-#include "qemu/object.h"
-#include "hw/qdev-properties.h"
 
 /* must be "called" in any C files that make use of QIDL-generated code */
 #define QIDL_ENABLE()
@@ -42,8 +40,8 @@
     static struct { \
         void (*visitor)(Visitor *, struct name **, const char *, Error **); \
         const char *schema_json_text; \
-        Object *schema_obj; \
-        Property *properties; \
+        struct Object *schema_obj; \
+        struct Property *properties; \
     } qidl_data_##name;
 #else
 #define QIDL_START(name,  ...)
diff --git a/scripts/qidl.py b/scripts/qidl.py
index 4e0880e..2544829 100644
--- a/scripts/qidl.py
+++ b/scripts/qidl.py
@@ -240,7 +240,7 @@ def main(argv=[]):
 
     output_filepath = None
     schema_filepath = None
-    includes = []
+    includes = ["hw/qdev-properties.h"]
     for o, a in opts:
         if o in ("-f", "--output-filepath"):
             output_filepath = a
-- 
1.7.12.1

Re: [Qemu-devel] [PATCH v4 26/26] qidl: unit tests and build infrastructure

[Paolo Bonzini]

Il 15/10/2012 18:37, Michael Roth ha scritto:
> There is another way to do this (still using a "dummy" target)  that is a
> bit less cryptic:
> 
> QIDL-PP-%: %.c qidl.h ...
>     <grep and create %.qidl.c>
> 
> %.o: QIDL-PP-%
>     <build normal or qidl CC>
> 
> But make detects that QIDL-PP-% is an intermediate target and removes
> the *.qidl.c files after the build

How so?  Make does not even _know_ that %.qidl.c exists.

Paolo

Re: [Qemu-devel] [PATCH v4 22/26] qidl: add lexer library (based on QC parser)

[Paolo Bonzini]

Il 12/10/2012 23:11, Michael Roth ha scritto:
> 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
> 
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>

Hmm, this does not print a filename and line, which makes it quite bad
from the usability PoV.

Can you squash in the following please?

diff --git a/scripts/lexer.py b/scripts/lexer.py
index 96c6c1a..f457292 100644
--- a/scripts/lexer.py
+++ b/scripts/lexer.py
@@ -16,16 +16,27 @@
 class Input(object):
     def __init__(self, fp):
         self.fp = fp
+        self.filename = fp.name
+        self.lineno = 0
         self.line = None
         self.offset = 0
         self.is_eof = False
         self.__fill_buf()
 
+    def __repr__(self):
+        return "%s:%d" % (str(self.filename), self.lineno)
+
     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
+            else:
+                self.lineno = self.lineno + 1
+
+    def set_next_line(self, filename, lineno):
+        self.filename = filename
+        self.lineno = lineno - 1
 
     def peek(self):
         if self.is_eof:
@@ -101,12 +112,12 @@ class Lexer(object):
     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))
+            raise Exception("%s: expected '%s', got %s %s" %
+                (self.input, 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))
+                raise Exception("%s: expected '%s', got %s" %
+                    (self.input, value_expected, self.current_value))
         return self.pop()
     
     def check_token(self, type_expected, value_expected=None):
@@ -300,7 +311,11 @@ class CLexer(Lexer):
                     token += ch
                     self.input.pop()
                     ch = self.input.peek()
-                return ('directive', token)
+                if token[1] == ' ':
+                    tokens = token.split()
+                    self.input.set_next_line(tokens[2][1:-1], int(tokens[1]))
+                else:
+                    return ('directive', token)
             else:
                 return ('unknown', ch)
         return (None, None)

plus perhaps something to generate errors with locations from qidl_parser.py.

Paolo

> ---
>  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..96c6c1a
> --- /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 or later.
> +# 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)
> 

Re: [Qemu-devel] [PATCH v4 24/26] qidl: add QAPI-based code generator

[Michael Roth]

On Tue, Oct 16, 2012 at 09:20:09AM +0200, Paolo Bonzini wrote:
> Il 15/10/2012 18:35, Michael Roth ha scritto:
> >> - immutable/derived/broken/elsewhere (and the default, let's call it
> >> serialized) are really five cases of the same QIDL property.  Perhaps
> >> this could be enforced in the extended syntax like this:
> >>
> >>     #define q_immutable QIDL(serialize(immutable))
> >>     #define q_derived QIDL(serialize(derived))
> >>     #define q_broken QIDL(serialize(broken))
> >>     #define q_elsewhere QIDL(serialize(elsewhere))
> >>
> >> I would also make it possible to explicitly specify the fifth state, if
> >> only for symmetry.
> > 
> > Agreed, that's a more proper grouping. Though, for consistency with
> > QIDL(property, ...), I would do QIDL(serialize, ...)
> 
> Sure.
> 
> >> - it would be _much_ better if you could automatically derive properties
> >> information for embedded structs.  For example, Notifiers and qemu_irqs
> >> are always q_immutable.  NotifierLists probably are always q_elsewhere,
> >> because the owner of the notifiers should add themselves back.
> > 
> > q_inherit maybe? Otherwise we're overriding "q_default" in subtle
> > ways that may not always be desired.
> 
> I think the default should be "whatever makes more sense", which for
> QIDL_DECLAREd types means making the member q_immutable if it makes
> sense for the type.
> 
> It's too fragile to expect all subclasses to know whether their
> superclass is immutable or has to be serialized, so q_inherit should be
> the default.  For atomic types, q_inherit is the same as q_serialized.

Totally agree. And over time, as more and more device-related structures are
QIDL_DECLAREd, this will help a lot on cutting down "boilerplate"
annotations.

> 
> That said, an alternative is just to never declare the superclass
> q_immutable.  That would work as long as you do not restrict QIDL to
> DeviceState subclasses---see attached patch.

Yah, I think the best general policy is to make as many of the fields
Just Work as possible by default. This can be done by using QIDL_DECLARE()
for as many relevant struct fields as possible (and the ones that aren't
QIDL_DECLAREd (or have an open-coded visitor) will generate errors that
can prompt further action (the optimal/encouraged one being to QIDL_DECLARE
the struct in question, or creating an open-coded visitor for the
simpler cases.

This how I'd prefer we handle making inherit-by-default work better, and
I'll focus on this more in the conversions.

> 
> >> In general, if struct X is QIDL_DECLAREd and only has q_immutable
> >> fields, it can be taken as q_immutable.  Hence for example the base
> >> class should not need any decoration; ISADevice will be seen as
> >> q_immutable, but PCIDevice will be seen as serialized.  But even if a
> >> struct is not QIDL_DECLAREd, it  should be possible to apply a tag to a
> >> typedef, and have it always applied to the members.
> 
> Hmm, this wasn't the best choice of words.  What I actually meant was
> "to apply a tag to a typedef, and have it always applied to members of
> that type in other structs".  Like
> 
> typedef struct Notifier Notifier q_immutable;

This would be really nice, but the parser is gonna need more hardening
before we can remove it's "scan for lines beginning with QIDL_START* and
parse what we expect to come afterward" behavior. There's also
build time implications. An alternative I'm considering is:

typedef struct Notifier Notifier;

QIDL_DECLARE_PUBLIC(Notifier, q_immutable) {
    ...
}

This will make all the fields q_immutable by default (individual fields
can override it in the future when the need arises). Then we teach the
code generator to drop serialization for any struct fields who don't
have any serializeable fields (mainly to avoid a bunch of nil fields in
the serialized code (e.g. 'notifier1': {})).

I'm planning on doing the above in the context of the device conversions
to get them a little more palatable.

Seem reasonable?

> 
> Note that Notifier will never have any serializable state, hence it will
> not be QIDL_DECLAREd.  It is just a proxy that specifies a function to call.
> 
> While in principle it is possible to change the function at run-time,
> that's not the way that Notifiers are used.  That can still be
> documented using q_elsewhere, but I think that sane defaults other than
> q_serialized are useful to avoid cluttering the declarations.  However,
> this is a very minor qualm.
> 
> Paolo

Re: [Qemu-devel] [PATCH v4 26/26] qidl: unit tests and build infrastructure

[Michael Roth]

On Tue, Oct 16, 2012 at 09:21:05AM +0200, Paolo Bonzini wrote:
> Il 15/10/2012 18:37, Michael Roth ha scritto:
> > There is another way to do this (still using a "dummy" target)  that is a
> > bit less cryptic:
> > 
> > QIDL-PP-%: %.c qidl.h ...
> >     <grep and create %.qidl.c>
> > 
> > %.o: QIDL-PP-%
> >     <build normal or qidl CC>
> > 
> > But make detects that QIDL-PP-% is an intermediate target and removes
> > the *.qidl.c files after the build
> 
> How so?  Make does not even _know_ that %.qidl.c exists.

Sorry, wasn't remembering things correctly. That's how I had things
working *before* I'd switched to using qidl-generated subdirs, and it
was actually this:

 %.qidl.c: %.c qidl.h ...
     <grep and create %.qidl.c>
 
 %.o: %.c %.qidl.c
     <build normal or qidl CC>

which isn't doing anything special.

This is actually what I ended up going back to in v5 due to some issues
I noticed after fixing the bogus rm -f %.qidl.* call you caught. Using
.SECONDARY: addresses the issue I had with it deleting intermediate
(*.qidl.c) files.
> 
> Paolo
> 

Re: [Qemu-devel] [PATCH v4 24/26] qidl: add QAPI-based code generator

[Paolo Bonzini]

> > >> In general, if struct X is QIDL_DECLAREd and only has
> > >> q_immutable
> > >> fields, it can be taken as q_immutable.  Hence for example the
> > >> base
> > >> class should not need any decoration; ISADevice will be seen as
> > >> q_immutable, but PCIDevice will be seen as serialized.  But even
> > >> if a
> > >> struct is not QIDL_DECLAREd, it  should be possible to apply a
> > >> tag to a
> > >> typedef, and have it always applied to the members.
> > 
> > Hmm, this wasn't the best choice of words.  What I actually meant
> > was "to apply a tag to a typedef, and have it always applied to members
> > of that type in other structs".  Like
> > 
> > typedef struct Notifier Notifier q_immutable;
> 
> This would be really nice, but the parser is gonna need more hardening
> before we can remove it's "scan for lines beginning with QIDL_START*
> and parse what we expect to come afterward" behavior. There's also
> build time implications. An alternative I'm considering is:
> 
> typedef struct Notifier Notifier;
> 
> QIDL_DECLARE_PUBLIC(Notifier, q_immutable) {
>     ...
> }
> 
> This will make all the fields q_immutable by default (individual fields
> can override it in the future when the need arises). Then we teach the
> code generator to drop serialization for any struct fields who don't
> have any serializeable fields (mainly to avoid a bunch of nil fields
> in the serialized code (e.g. 'notifier1': {})).
> 
> Seem reasonable?

Optimal, no... reasonable, yes. :)

Paolo