All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH RFC 0/3] add direct support of event in qapi schema
@ 2013-11-13  1:44 Wenchao Xia
  2013-11-13  1:44 ` [Qemu-devel] [PATCH RFC 1/3] os-posix: include sys/time.h Wenchao Xia
                   ` (4 more replies)
  0 siblings, 5 replies; 30+ messages in thread
From: Wenchao Xia @ 2013-11-13  1:44 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, mdroth, armbru, pbonzini, lcapitulino, Wenchao Xia

This series add support for tag/keyword 'event' in qapi-schema.
The implemention doesn't generate a struture and visit function
in the background for every event, so it doesn't support nested
structure in the define to avoid trouble.

It is on top of series:
http://lists.nongnu.org/archive/html/qemu-devel/2013-11/msg01574.html

Wenchao Xia (3):
  1 os-posix: include sys/time.h
  2 qapi script: add support of event
  3 tests: add test cases for qapi event support

 Makefile                                |    9 +-
 Makefile.objs                           |    2 +-
 include/sysemu/os-posix.h               |    2 +
 qapi/Makefile.objs                      |    1 +
 scripts/qapi-event.py                   |  355 +++++++++++++++++++++++++++++++
 tests/Makefile                          |   14 +-
 tests/qapi-schema/qapi-schema-test.json |   12 +
 tests/qapi-schema/qapi-schema-test.out  |   10 +-
 tests/test-qmp-event.c                  |  250 ++++++++++++++++++++++
 9 files changed, 646 insertions(+), 9 deletions(-)
 create mode 100644 scripts/qapi-event.py
 create mode 100644 tests/test-qmp-event.c

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [Qemu-devel] [PATCH RFC 1/3] os-posix: include sys/time.h
  2013-11-13  1:44 [Qemu-devel] [PATCH RFC 0/3] add direct support of event in qapi schema Wenchao Xia
@ 2013-11-13  1:44 ` Wenchao Xia
  2013-12-13 13:26   ` Eric Blake
  2013-11-13  1:44 ` [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event Wenchao Xia
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 30+ messages in thread
From: Wenchao Xia @ 2013-11-13  1:44 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, mdroth, armbru, pbonzini, lcapitulino, Wenchao Xia

Since gettimeofday() is used in this header file as a macro define,
include the function's define header file, to avoid compile warning
when other file include os-posix.h.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 include/sysemu/os-posix.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/include/sysemu/os-posix.h b/include/sysemu/os-posix.h
index 25d0b2a..f131521 100644
--- a/include/sysemu/os-posix.h
+++ b/include/sysemu/os-posix.h
@@ -26,6 +26,8 @@
 #ifndef QEMU_OS_POSIX_H
 #define QEMU_OS_POSIX_H
 
+#include <sys/time.h>
+
 void os_set_line_buffering(void);
 void os_set_proc_name(const char *s);
 void os_setup_signal_handling(void);
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
  2013-11-13  1:44 [Qemu-devel] [PATCH RFC 0/3] add direct support of event in qapi schema Wenchao Xia
  2013-11-13  1:44 ` [Qemu-devel] [PATCH RFC 1/3] os-posix: include sys/time.h Wenchao Xia
@ 2013-11-13  1:44 ` Wenchao Xia
  2013-11-28  0:48   ` Luiz Capitulino
  2013-12-13 13:31   ` Eric Blake
  2013-11-13  1:44 ` [Qemu-devel] [PATCH 2/2] try Wenchao Xia
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 30+ messages in thread
From: Wenchao Xia @ 2013-11-13  1:44 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, mdroth, armbru, pbonzini, lcapitulino, Wenchao Xia

Nested structure is not supported now, so following define is not valid:
{ 'event': 'EVENT_C',
  'data': { 'a': { 'a_a', 'str', 'a_b', 'str' }, 'b': 'int' }

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 Makefile              |    9 +-
 Makefile.objs         |    2 +-
 qapi/Makefile.objs    |    1 +
 scripts/qapi-event.py |  355 +++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 363 insertions(+), 4 deletions(-)
 create mode 100644 scripts/qapi-event.py

diff --git a/Makefile b/Makefile
index b15003f..a3e465f 100644
--- a/Makefile
+++ b/Makefile
@@ -38,8 +38,8 @@ endif
 endif
 
 GENERATED_HEADERS = config-host.h qemu-options.def
-GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h
-GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c
+GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h qapi-event.h
+GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c qapi-event.c
 
 GENERATED_HEADERS += trace/generated-events.h
 GENERATED_SOURCES += trace/generated-events.c
@@ -178,7 +178,7 @@ Makefile: $(version-obj-y) $(version-lobj-y)
 # Build libraries
 
 libqemustub.a: $(stub-obj-y)
-libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o
+libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o qapi-event.o
 
 ######################################################################
 
@@ -219,6 +219,9 @@ $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
 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 "." -b < $<, "  GEN   $@")
+qapi-event.c qapi-event.h :\
+$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-event.py $(qapi-py)
+	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-event.py $(gen-out-type) -o "." -b < $<, "  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/Makefile.objs b/Makefile.objs
index 2b6c1fe..33f5950 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -12,7 +12,7 @@ block-obj-y += main-loop.o iohandler.o qemu-timer.o
 block-obj-$(CONFIG_POSIX) += aio-posix.o
 block-obj-$(CONFIG_WIN32) += aio-win32.o
 block-obj-y += block/
-block-obj-y += qapi-types.o qapi-visit.o
+block-obj-y += qapi-types.o qapi-visit.o qapi-event.o
 block-obj-y += qemu-io-cmds.o
 
 block-obj-y += qemu-coroutine.o qemu-coroutine-lock.o qemu-coroutine-io.o
diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs
index 1f9c973..d14b769 100644
--- a/qapi/Makefile.objs
+++ b/qapi/Makefile.objs
@@ -3,3 +3,4 @@ util-obj-y += qmp-output-visitor.o qmp-registry.o qmp-dispatch.o
 util-obj-y += string-input-visitor.o string-output-visitor.o
 
 util-obj-y += opts-visitor.o
+util-obj-y += qmp-event.o
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
new file mode 100644
index 0000000..4c6a0fe
--- /dev/null
+++ b/scripts/qapi-event.py
@@ -0,0 +1,355 @@
+#
+# QAPI event generator
+#
+# Copyright IBM, Corp. 2013
+#
+# Authors:
+#  Wenchao Xia <xiawenc@linux.vnet.ibm.com>
+#
+# This work is licensed under the terms of the GNU GPLv2.
+# See the COPYING.LIB file in the top-level directory.
+
+from ordereddict import OrderedDict
+from qapi import *
+import sys
+import os
+import getopt
+import errno
+
+def _generate_event_api_name_with_params(event_name, params):
+    api_name = "void qapi_event_gen_%s(" % c_fun(event_name);
+    l = len(api_name)
+
+    for argname, argentry, optional, structured in parse_args(params):
+        if structured:
+            sys.stderr.write("Nested structure define in event is not "
+                             "supported now, event '%s', argname '%s'\n" %
+                             (event_name, argname))
+            sys.exit(1)
+            continue
+
+        if optional:
+            api_name += "bool has_%s,\n" % c_var(argname)
+            api_name += "".ljust(l)
+
+        if argentry == "str":
+            api_name += "const "
+        api_name += "%s %s,\n" % (c_type(argentry), c_var(argname))
+        api_name += "".ljust(l)
+
+    api_name += "Error **errp)"
+    return api_name;
+
+def _generate_event_implement_with_params(api_name, event_name, params):
+    ret = mcgen("""
+
+%(api_name)s
+{
+    QmpOutputVisitor *qov;
+    Visitor *v;
+    Error *err = NULL;
+    QObject *obj;
+    QDict *qmp = NULL;
+
+    if (!qapi_event_functions.emit) {
+        return;
+    }
+
+    qmp = qdict_new();
+    qdict_put(qmp, "event", qstring_from_str("%(event_name)s"));
+    timestamp_put(qmp);
+
+    qov = qmp_output_visitor_new();
+    g_assert(qov);
+
+    v = qmp_output_get_visitor(qov);
+    g_assert(v);
+
+    /* Fake visit, as if all member are under a structure */
+    visit_start_struct(v, NULL, "", "%(event_name)s", 0, &err);
+    if (error_is_set(&err)) {
+        goto clean;
+    }
+
+""",
+                api_name = api_name,
+                event_name = event_name)
+
+    for argname, argentry, optional, structured in parse_args(params):
+        if structured:
+            sys.stderr.write("Nested structure define in event is not "
+                             "supported now, event '%s', argname '%s'\n" %
+                             (event_name, argname))
+            sys.exit(1)
+            continue
+
+        if optional:
+            ret += mcgen("""
+    if (has_%(var)s) {
+""",
+                         var = c_var(argname))
+            push_indent()
+
+        if argentry == "str":
+            var_type = "(char **)"
+        else:
+            var_type = ""
+
+        ret += mcgen("""
+    visit_type_%(type)s(v, %(var_type)s&%(var)s, "%(name)s", &err);
+    if (error_is_set(&err)) {
+        goto clean;
+    }
+""",
+                     var_type = var_type,
+                     var = c_var(argname),
+                     type = type_name(argentry),
+                     name = argname)
+
+        if optional:
+            pop_indent()
+            ret += mcgen("""
+    }
+""")
+
+    ret += mcgen("""
+
+    visit_end_struct(v, &err);
+    if (error_is_set(&err)) {
+        goto clean;
+    }
+
+    obj = qmp_output_get_qobject(qov);
+    g_assert(obj != NULL);
+
+    qdict_put_obj(qmp, "data", obj);
+
+    qapi_event_functions.emit(qmp, &err);
+
+ clean:
+    error_propagate(errp, err);
+    qmp_output_visitor_cleanup(qov);
+    QDECREF(qmp);
+}
+""")
+
+    return ret
+
+def _generate_event_api_name(event_name):
+    return "void qapi_event_gen_%s(Error **errp)" % c_fun(event_name);
+
+def _generate_event_implement(api_name, event_name):
+    return mcgen("""
+
+%(api_name)s
+{
+    QDict *qmp = NULL;
+
+    if (!qapi_event_functions.emit) {
+        return;
+    }
+
+    qmp = qdict_new();
+    qdict_put(qmp, "event", qstring_from_str("%(event_name)s"));
+    timestamp_put(qmp);
+
+    qapi_event_functions.emit(qmp, errp);
+
+    QDECREF(qmp);
+}
+""",
+                api_name = api_name,
+                event_name = event_name)
+
+
+def generate_event_declaration(event_name, params):
+    if params and len(params) > 0:
+        api_name = _generate_event_api_name_with_params(event_name, params)
+    else:
+        api_name = _generate_event_api_name(event_name)
+
+    return mcgen('''
+
+%(api_name)s;
+''',
+                 api_name = api_name)
+
+def generate_event_implement(event_name, params):
+    if params and len(params) > 0:
+        api_name = _generate_event_api_name_with_params(event_name, params)
+        ret =  _generate_event_implement_with_params(api_name,
+                                                     event_name,
+                                                     params)
+
+    else:
+        api_name = _generate_event_api_name(event_name)
+        ret =  _generate_event_implement(api_name,
+                                         event_name)
+    return ret
+
+try:
+    opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:o:",
+                                   ["source", "header", "builtins", "prefix=",
+                                    "output-dir="])
+except getopt.GetoptError, err:
+    print str(err)
+    sys.exit(1)
+
+output_dir = ""
+prefix = ""
+c_file = 'qapi-event.c'
+h_file = 'qapi-event.h'
+
+do_c = False
+do_h = False
+do_builtins = 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
+    elif o in ("-b", "--builtins"):
+        do_builtins = 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
+
+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()
+
+fdef = maybe_open(do_c, c_file, 'w')
+fdecl = maybe_open(do_h, h_file, 'w')
+
+fdef.write(mcgen('''
+/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
+
+/*
+ * schema-defined QAPI event functions
+ *
+ * Copyright IBM, Corp. 2013
+ *
+ * Authors:
+ *  Wenchao Xia   <xiawenc@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include "qemu-common.h"
+#include "%(header)s"
+#include "%(prefix)sqapi-visit.h"
+#include "qapi/qmp-output-visitor.h"
+#include "qapi/qmp/qstring.h"
+#include "qapi/qmp/qjson.h"
+
+#ifdef _WIN32
+#include "sysemu/os-win32.h"
+#endif
+
+#ifdef CONFIG_POSIX
+#include "sysemu/os-posix.h"
+#endif
+
+typedef struct QAPIEventFunctions {
+    void (*emit)(QDict *dict, Error **errp);
+} QAPIEventFunctions;
+
+QAPIEventFunctions qapi_event_functions;
+
+void qapi_event_set_func_emit(qapi_event_emit emit)
+{
+    qapi_event_functions.emit = emit;
+}
+
+static void timestamp_put(QDict *qdict)
+{
+    int err;
+    QObject *obj;
+    qemu_timeval tv;
+
+    err = qemu_gettimeofday(&tv);
+    if (err < 0)
+        return;
+
+    obj = qobject_from_jsonf("{ 'seconds': %(p)s" PRId64 ", "
+                                "'microseconds': %(p)s" PRId64 " }",
+                                (int64_t) tv.tv_sec, (int64_t) tv.tv_usec);
+    qdict_put_obj(qdict, "timestamp", obj);
+}
+
+''',
+                 prefix=prefix, header=basename(h_file), p="%"))
+
+fdecl.write(mcgen('''
+/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
+
+/*
+ * schema-defined QAPI event function
+ *
+ * Copyright IBM, Corp. 2013
+ *
+ * Authors:
+ *  Wenchao Xia  <xiawenc@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef %(guard)s
+#define %(guard)s
+
+#include "qapi/qmp/qdict.h"
+#include "qapi/error.h"
+#include "qapi/visitor.h"
+#include "%(prefix)sqapi-types.h"
+
+typedef void (*qapi_event_emit)(QDict *d, Error **errp);
+
+void qapi_event_set_func_emit(qapi_event_emit emit);
+
+''',
+                  prefix=prefix, guard=guardname(h_file)))
+
+exprs = parse_schema(sys.stdin)
+
+for expr in exprs:
+    if expr.has_key('event'):
+        event_name = expr['event']
+        params = expr.get('data')
+
+        ret = generate_event_declaration(event_name, params)
+        fdecl.write(ret)
+
+        ret = generate_event_implement(event_name, params)
+        fdef.write(ret)
+
+fdecl.write('''
+#endif
+''')
+
+fdecl.flush()
+fdecl.close()
+
+fdef.flush()
+fdef.close()
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [Qemu-devel] [PATCH 2/2] try
  2013-11-13  1:44 [Qemu-devel] [PATCH RFC 0/3] add direct support of event in qapi schema Wenchao Xia
  2013-11-13  1:44 ` [Qemu-devel] [PATCH RFC 1/3] os-posix: include sys/time.h Wenchao Xia
  2013-11-13  1:44 ` [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event Wenchao Xia
@ 2013-11-13  1:44 ` Wenchao Xia
  2013-11-13  1:47   ` Wenchao Xia
  2013-11-13  1:44 ` [Qemu-devel] [PATCH RFC 3/3] tests: add test cases for qapi event support Wenchao Xia
  2013-11-22  2:03 ` [Qemu-devel] [PATCH RFC 0/3] add direct support of event in qapi schema Wenchao Xia
  4 siblings, 1 reply; 30+ messages in thread
From: Wenchao Xia @ 2013-11-13  1:44 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, mdroth, armbru, pbonzini, lcapitulino, Wenchao Xia

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 Makefile                                |    9 +-
 Makefile.objs                           |    2 +-
 include/qapi/qmp-event.h                |   29 +++
 qapi/Makefile.objs                      |    1 +
 qapi/qmp-event.c                        |   45 ++++
 scripts/qapi-event.py                   |  355 +++++++++++++++++++++++++++++++
 tests/Makefile                          |   14 +-
 tests/qapi-schema/qapi-schema-test.json |   12 +
 tests/qapi-schema/qapi-schema-test.out  |   10 +-
 tests/test-qmp-event.c                  |  250 ++++++++++++++++++++++
 10 files changed, 718 insertions(+), 9 deletions(-)
 create mode 100644 include/qapi/qmp-event.h
 create mode 100644 qapi/qmp-event.c
 create mode 100644 scripts/qapi-event.py
 create mode 100644 tests/test-qmp-event.c

diff --git a/Makefile b/Makefile
index b15003f..a3e465f 100644
--- a/Makefile
+++ b/Makefile
@@ -38,8 +38,8 @@ endif
 endif
 
 GENERATED_HEADERS = config-host.h qemu-options.def
-GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h
-GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c
+GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h qapi-event.h
+GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c qapi-event.c
 
 GENERATED_HEADERS += trace/generated-events.h
 GENERATED_SOURCES += trace/generated-events.c
@@ -178,7 +178,7 @@ Makefile: $(version-obj-y) $(version-lobj-y)
 # Build libraries
 
 libqemustub.a: $(stub-obj-y)
-libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o
+libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o qapi-event.o
 
 ######################################################################
 
@@ -219,6 +219,9 @@ $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
 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 "." -b < $<, "  GEN   $@")
+qapi-event.c qapi-event.h :\
+$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-event.py $(qapi-py)
+	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-event.py $(gen-out-type) -o "." -b < $<, "  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/Makefile.objs b/Makefile.objs
index 2b6c1fe..33f5950 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -12,7 +12,7 @@ block-obj-y += main-loop.o iohandler.o qemu-timer.o
 block-obj-$(CONFIG_POSIX) += aio-posix.o
 block-obj-$(CONFIG_WIN32) += aio-win32.o
 block-obj-y += block/
-block-obj-y += qapi-types.o qapi-visit.o
+block-obj-y += qapi-types.o qapi-visit.o qapi-event.o
 block-obj-y += qemu-io-cmds.o
 
 block-obj-y += qemu-coroutine.o qemu-coroutine-lock.o qemu-coroutine-io.o
diff --git a/include/qapi/qmp-event.h b/include/qapi/qmp-event.h
new file mode 100644
index 0000000..6922693
--- /dev/null
+++ b/include/qapi/qmp-event.h
@@ -0,0 +1,29 @@
+/*
+ * QMP Event related
+ *
+ * Copyright IBM, Corp. 2013
+ *
+ * Authors:
+ *  Wenchao Xia   <xiawenc@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef QMP_EVENT_H
+#define QMP_EVENT_H
+
+#include "qapi-types.h"
+#include "qapi/error.h"
+#include "qapi/qmp/qdict.h"
+
+typedef struct QMPEventHooks {
+    void (*emit)(QDict *dict, Error **errp);
+} QMPEventHooks;
+
+void qmp_event_set_hooks(QMPEventHooks *hooks);
+
+void qmp_event_generate(Error **errp);
+
+#endif
diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs
index 1f9c973..d14b769 100644
--- a/qapi/Makefile.objs
+++ b/qapi/Makefile.objs
@@ -3,3 +3,4 @@ util-obj-y += qmp-output-visitor.o qmp-registry.o qmp-dispatch.o
 util-obj-y += string-input-visitor.o string-output-visitor.o
 
 util-obj-y += opts-visitor.o
+util-obj-y += qmp-event.o
diff --git a/qapi/qmp-event.c b/qapi/qmp-event.c
new file mode 100644
index 0000000..3f2f5da
--- /dev/null
+++ b/qapi/qmp-event.c
@@ -0,0 +1,45 @@
+/*
+ * QMP Event related
+ *
+ * Copyright IBM, Corp. 2013
+ *
+ * Authors:
+ *  Wenchao Xia   <xiawenc@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include "glib.h"
+
+#include "qapi/qmp-event.h"
+#include "qapi/qmp/qobject.h"
+#include "qapi/qmp/qstring.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qjson.h"
+#include "qapi/qmp-output-visitor.h"
+#include "qapi-visit.h"
+
+#ifdef _WIN32
+#include "sysemu/os-win32.h"
+#endif
+
+#ifdef CONFIG_POSIX
+#include "sysemu/os-posix.h"
+#endif
+
+static QMPEventHooks qmp_event_hooks;
+
+void qmp_event_set_hooks(QMPEventHooks *hooks)
+{
+    qmp_event_hooks = *hooks;
+}
+
+/*
+ * Fill in the timestamp if caller haven't set it, and then try emit.
+ */
+void qmp_event_generate(Error **errp)
+{
+    return;
+}
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
new file mode 100644
index 0000000..4c6a0fe
--- /dev/null
+++ b/scripts/qapi-event.py
@@ -0,0 +1,355 @@
+#
+# QAPI event generator
+#
+# Copyright IBM, Corp. 2013
+#
+# Authors:
+#  Wenchao Xia <xiawenc@linux.vnet.ibm.com>
+#
+# This work is licensed under the terms of the GNU GPLv2.
+# See the COPYING.LIB file in the top-level directory.
+
+from ordereddict import OrderedDict
+from qapi import *
+import sys
+import os
+import getopt
+import errno
+
+def _generate_event_api_name_with_params(event_name, params):
+    api_name = "void qapi_event_gen_%s(" % c_fun(event_name);
+    l = len(api_name)
+
+    for argname, argentry, optional, structured in parse_args(params):
+        if structured:
+            sys.stderr.write("Nested structure define in event is not "
+                             "supported now, event '%s', argname '%s'\n" %
+                             (event_name, argname))
+            sys.exit(1)
+            continue
+
+        if optional:
+            api_name += "bool has_%s,\n" % c_var(argname)
+            api_name += "".ljust(l)
+
+        if argentry == "str":
+            api_name += "const "
+        api_name += "%s %s,\n" % (c_type(argentry), c_var(argname))
+        api_name += "".ljust(l)
+
+    api_name += "Error **errp)"
+    return api_name;
+
+def _generate_event_implement_with_params(api_name, event_name, params):
+    ret = mcgen("""
+
+%(api_name)s
+{
+    QmpOutputVisitor *qov;
+    Visitor *v;
+    Error *err = NULL;
+    QObject *obj;
+    QDict *qmp = NULL;
+
+    if (!qapi_event_functions.emit) {
+        return;
+    }
+
+    qmp = qdict_new();
+    qdict_put(qmp, "event", qstring_from_str("%(event_name)s"));
+    timestamp_put(qmp);
+
+    qov = qmp_output_visitor_new();
+    g_assert(qov);
+
+    v = qmp_output_get_visitor(qov);
+    g_assert(v);
+
+    /* Fake visit, as if all member are under a structure */
+    visit_start_struct(v, NULL, "", "%(event_name)s", 0, &err);
+    if (error_is_set(&err)) {
+        goto clean;
+    }
+
+""",
+                api_name = api_name,
+                event_name = event_name)
+
+    for argname, argentry, optional, structured in parse_args(params):
+        if structured:
+            sys.stderr.write("Nested structure define in event is not "
+                             "supported now, event '%s', argname '%s'\n" %
+                             (event_name, argname))
+            sys.exit(1)
+            continue
+
+        if optional:
+            ret += mcgen("""
+    if (has_%(var)s) {
+""",
+                         var = c_var(argname))
+            push_indent()
+
+        if argentry == "str":
+            var_type = "(char **)"
+        else:
+            var_type = ""
+
+        ret += mcgen("""
+    visit_type_%(type)s(v, %(var_type)s&%(var)s, "%(name)s", &err);
+    if (error_is_set(&err)) {
+        goto clean;
+    }
+""",
+                     var_type = var_type,
+                     var = c_var(argname),
+                     type = type_name(argentry),
+                     name = argname)
+
+        if optional:
+            pop_indent()
+            ret += mcgen("""
+    }
+""")
+
+    ret += mcgen("""
+
+    visit_end_struct(v, &err);
+    if (error_is_set(&err)) {
+        goto clean;
+    }
+
+    obj = qmp_output_get_qobject(qov);
+    g_assert(obj != NULL);
+
+    qdict_put_obj(qmp, "data", obj);
+
+    qapi_event_functions.emit(qmp, &err);
+
+ clean:
+    error_propagate(errp, err);
+    qmp_output_visitor_cleanup(qov);
+    QDECREF(qmp);
+}
+""")
+
+    return ret
+
+def _generate_event_api_name(event_name):
+    return "void qapi_event_gen_%s(Error **errp)" % c_fun(event_name);
+
+def _generate_event_implement(api_name, event_name):
+    return mcgen("""
+
+%(api_name)s
+{
+    QDict *qmp = NULL;
+
+    if (!qapi_event_functions.emit) {
+        return;
+    }
+
+    qmp = qdict_new();
+    qdict_put(qmp, "event", qstring_from_str("%(event_name)s"));
+    timestamp_put(qmp);
+
+    qapi_event_functions.emit(qmp, errp);
+
+    QDECREF(qmp);
+}
+""",
+                api_name = api_name,
+                event_name = event_name)
+
+
+def generate_event_declaration(event_name, params):
+    if params and len(params) > 0:
+        api_name = _generate_event_api_name_with_params(event_name, params)
+    else:
+        api_name = _generate_event_api_name(event_name)
+
+    return mcgen('''
+
+%(api_name)s;
+''',
+                 api_name = api_name)
+
+def generate_event_implement(event_name, params):
+    if params and len(params) > 0:
+        api_name = _generate_event_api_name_with_params(event_name, params)
+        ret =  _generate_event_implement_with_params(api_name,
+                                                     event_name,
+                                                     params)
+
+    else:
+        api_name = _generate_event_api_name(event_name)
+        ret =  _generate_event_implement(api_name,
+                                         event_name)
+    return ret
+
+try:
+    opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:o:",
+                                   ["source", "header", "builtins", "prefix=",
+                                    "output-dir="])
+except getopt.GetoptError, err:
+    print str(err)
+    sys.exit(1)
+
+output_dir = ""
+prefix = ""
+c_file = 'qapi-event.c'
+h_file = 'qapi-event.h'
+
+do_c = False
+do_h = False
+do_builtins = 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
+    elif o in ("-b", "--builtins"):
+        do_builtins = 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
+
+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()
+
+fdef = maybe_open(do_c, c_file, 'w')
+fdecl = maybe_open(do_h, h_file, 'w')
+
+fdef.write(mcgen('''
+/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
+
+/*
+ * schema-defined QAPI event functions
+ *
+ * Copyright IBM, Corp. 2013
+ *
+ * Authors:
+ *  Wenchao Xia   <xiawenc@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include "qemu-common.h"
+#include "%(header)s"
+#include "%(prefix)sqapi-visit.h"
+#include "qapi/qmp-output-visitor.h"
+#include "qapi/qmp/qstring.h"
+#include "qapi/qmp/qjson.h"
+
+#ifdef _WIN32
+#include "sysemu/os-win32.h"
+#endif
+
+#ifdef CONFIG_POSIX
+#include "sysemu/os-posix.h"
+#endif
+
+typedef struct QAPIEventFunctions {
+    void (*emit)(QDict *dict, Error **errp);
+} QAPIEventFunctions;
+
+QAPIEventFunctions qapi_event_functions;
+
+void qapi_event_set_func_emit(qapi_event_emit emit)
+{
+    qapi_event_functions.emit = emit;
+}
+
+static void timestamp_put(QDict *qdict)
+{
+    int err;
+    QObject *obj;
+    qemu_timeval tv;
+
+    err = qemu_gettimeofday(&tv);
+    if (err < 0)
+        return;
+
+    obj = qobject_from_jsonf("{ 'seconds': %(p)s" PRId64 ", "
+                                "'microseconds': %(p)s" PRId64 " }",
+                                (int64_t) tv.tv_sec, (int64_t) tv.tv_usec);
+    qdict_put_obj(qdict, "timestamp", obj);
+}
+
+''',
+                 prefix=prefix, header=basename(h_file), p="%"))
+
+fdecl.write(mcgen('''
+/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
+
+/*
+ * schema-defined QAPI event function
+ *
+ * Copyright IBM, Corp. 2013
+ *
+ * Authors:
+ *  Wenchao Xia  <xiawenc@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef %(guard)s
+#define %(guard)s
+
+#include "qapi/qmp/qdict.h"
+#include "qapi/error.h"
+#include "qapi/visitor.h"
+#include "%(prefix)sqapi-types.h"
+
+typedef void (*qapi_event_emit)(QDict *d, Error **errp);
+
+void qapi_event_set_func_emit(qapi_event_emit emit);
+
+''',
+                  prefix=prefix, guard=guardname(h_file)))
+
+exprs = parse_schema(sys.stdin)
+
+for expr in exprs:
+    if expr.has_key('event'):
+        event_name = expr['event']
+        params = expr.get('data')
+
+        ret = generate_event_declaration(event_name, params)
+        fdecl.write(ret)
+
+        ret = generate_event_implement(event_name, params)
+        fdef.write(ret)
+
+fdecl.write('''
+#endif
+''')
+
+fdecl.flush()
+fdecl.close()
+
+fdef.flush()
+fdef.close()
diff --git a/tests/Makefile b/tests/Makefile
index f414f2c..3c22f0a 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -23,6 +23,8 @@ check-unit-y += tests/test-string-input-visitor$(EXESUF)
 gcov-files-test-string-input-visitor-y = qapi/string-input-visitor.c
 check-unit-y += tests/test-string-output-visitor$(EXESUF)
 gcov-files-test-string-output-visitor-y = qapi/string-output-visitor.c
+check-unit-y += tests/test-qmp-event$(EXESUF)
+gcov-files-test-qmp-event-y += qapi/qmp-event.c
 check-unit-y += tests/test-opts-visitor$(EXESUF)
 gcov-files-test-opts-visitor-y = qapi/opts-visitor.c
 check-unit-y += tests/test-coroutine$(EXESUF)
@@ -120,7 +122,8 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
         trailing-comma-list.json trailing-comma-object.json \
         unclosed-list.json unclosed-object.json unclosed-string.json)
 
-GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h tests/test-qmp-commands.h
+GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h \
+                     tests/test-qapi-event.h tests/test-qmp-commands.h
 
 test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \
 	tests/check-qlist.o tests/check-qfloat.o tests/check-qjson.o \
@@ -129,9 +132,10 @@ test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.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-x86-cpuid.o tests/test-mul64.o tests/test-int128.o \
-	tests/test-opts-visitor.o
+	tests/test-opts-visitor.o tests/test-qmp-event.o
 
-test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o
+test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o \
+                  tests/test-qapi-event.o
 
 $(test-obj-y): QEMU_INCLUDES += -Itests
 QEMU_CFLAGS += -I$(SRC_PATH)/tests
@@ -167,12 +171,16 @@ $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-typ
 tests/test-qapi-visit.c tests/test-qapi-visit.h :\
 $(SRC_PATH)/tests/qapi-schema/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-qapi-event.c tests/test-qapi-event.h :\
+$(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-event.py
+	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-event.py $(gen-out-type) -o tests -p "test-" < $<, "  GEN   $@")
 tests/test-qmp-commands.h tests/test-qmp-marshal.c :\
 $(SRC_PATH)/tests/qapi-schema/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) libqemuutil.a libqemustub.a
 tests/test-string-input-visitor$(EXESUF): tests/test-string-input-visitor.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
+tests/test-qmp-event$(EXESUF): tests/test-qmp-event.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
 tests/test-qmp-output-visitor$(EXESUF): tests/test-qmp-output-visitor.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
 tests/test-qmp-input-visitor$(EXESUF): tests/test-qmp-input-visitor.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
 tests/test-qmp-input-strict$(EXESUF): tests/test-qmp-input-strict.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 1e23d21..3fd7902 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -93,3 +93,15 @@
     '*u16' : [ 'uint16' ],
     '*i64x':   'int'     ,
     '*u64x':   'uint64'  } }
+
+# testing event
+{ 'type': 'EventStructOne',
+  'data': { 'struct1': 'UserDefOne', 'string': 'str', '*enum2': 'EnumOne' } }
+
+{ 'event': 'EVENT_A' }
+{ 'event': 'EVENT_B',
+  'data': { } }
+{ 'event': 'EVENT_C',
+  'data': { '*a': 'int', '*b': 'UserDefOne', 'c': 'str' } }
+{ 'event': 'EVENT_D',
+  'data': { 'a' : 'EventStructOne', 'b' : 'str', '*c': 'str', '*enum3': 'EnumOne' } }
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index ce174a2..22430ba 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -16,7 +16,12 @@
  OrderedDict([('command', 'user_def_cmd'), ('data', OrderedDict())]),
  OrderedDict([('command', 'user_def_cmd1'), ('data', OrderedDict([('ud1a', 'UserDefOne')]))]),
  OrderedDict([('command', 'user_def_cmd2'), ('data', OrderedDict([('ud1a', 'UserDefOne'), ('ud1b', 'UserDefOne')])), ('returns', 'UserDefTwo')]),
- OrderedDict([('type', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))])]
+ OrderedDict([('type', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))]),
+ OrderedDict([('type', 'EventStructOne'), ('data', OrderedDict([('struct1', 'UserDefOne'), ('string', 'str'), ('*enum2', 'EnumOne')]))]),
+ OrderedDict([('event', 'EVENT_A')]),
+ OrderedDict([('event', 'EVENT_B'), ('data', OrderedDict())]),
+ OrderedDict([('event', 'EVENT_C'), ('data', OrderedDict([('*a', 'int'), ('*b', 'UserDefOne'), ('c', 'str')]))]),
+ OrderedDict([('event', 'EVENT_D'), ('data', OrderedDict([('a', 'EventStructOne'), ('b', 'str'), ('*c', 'str'), ('*enum3', 'EnumOne')]))])]
 [{'enum_name': 'EnumOne', 'enum_values': ['value1', 'value2', 'value3']},
  {'enum_name': 'UserDefUnionKind', 'enum_values': None},
  {'enum_name': 'UserDefBaseUnionKind', 'enum_values': None},
@@ -31,4 +36,5 @@
  OrderedDict([('type', 'UserDefBase0'), ('data', OrderedDict([('base-string0', 'str'), ('base-enum0', 'EnumOne')]))]),
  OrderedDict([('type', 'UserDefA'), ('data', OrderedDict([('boolean', 'bool')]))]),
  OrderedDict([('type', 'UserDefB'), ('data', OrderedDict([('integer', 'int')]))]),
- OrderedDict([('type', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))])]
+ OrderedDict([('type', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))]),
+ OrderedDict([('type', 'EventStructOne'), ('data', OrderedDict([('struct1', 'UserDefOne'), ('string', 'str'), ('*enum2', 'EnumOne')]))])]
diff --git a/tests/test-qmp-event.c b/tests/test-qmp-event.c
new file mode 100644
index 0000000..3b2c43e
--- /dev/null
+++ b/tests/test-qmp-event.c
@@ -0,0 +1,250 @@
+/*
+ * QMP Input Visitor unit-tests.
+ *
+ * Copyright (C) 2011 Red Hat Inc.
+ *
+ * Authors:
+ *  Luiz Capitulino <lcapitulino@redhat.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 <stdarg.h>
+
+#include "qemu-common.h"
+#include "test-qapi-types.h"
+#include "test-qapi-visit.h"
+#include "test-qapi-event.h"
+#include "qapi/qmp/types.h"
+#include "qapi/qmp/qobject.h"
+
+typedef struct TestEventData {
+    QDict *expect;
+} TestEventData;
+
+typedef struct QDictCmpData {
+    QDict *expect;
+    bool result;
+} QDictCmpData;
+
+TestEventData *test_event_data;
+static GStaticMutex test_event_lock = G_STATIC_MUTEX_INIT;
+
+/* Only compares bool, int, string */
+static
+void qdict_cmp_do_simple(const char *key, QObject *obj1, void *opaque)
+
+{
+    QObject *obj2;
+    QDictCmpData d_new, *d = opaque;
+
+    if (!d->result) {
+        return;
+    }
+
+    obj2 = qdict_get(d->expect, key);
+    if (!obj2) {
+        d->result = false;
+        return;
+    }
+
+    if (qobject_type(obj1) != qobject_type(obj2)) {
+        d->result = false;
+        return;
+    }
+
+    switch (qobject_type(obj1)) {
+    case QTYPE_QBOOL:
+        d->result = (qbool_get_int(qobject_to_qbool(obj1)) ==
+                     qbool_get_int(qobject_to_qbool(obj2)));
+        return;
+    case QTYPE_QINT:
+        d->result = (qint_get_int(qobject_to_qint(obj1)) ==
+                     qint_get_int(qobject_to_qint(obj2)));
+        return;
+    case QTYPE_QSTRING:
+        if (!g_strcmp0(qstring_get_str(qobject_to_qstring(obj1)),
+                       qstring_get_str(qobject_to_qstring(obj2)))) {
+            d->result = true;
+        } else {
+            d->result = false;
+        }
+        return;
+    case QTYPE_QDICT:
+        d_new.expect = qobject_to_qdict(obj2);
+        d_new.result = true;
+        qdict_iter(qobject_to_qdict(obj1), qdict_cmp_do_simple, &d_new);
+        d->result = d_new.result;
+        return;
+    default:
+        abort();
+    }
+}
+
+static bool qdict_cmp_simple(QDict *a, QDict *b)
+{
+    QDictCmpData d;
+
+    d.expect = b;
+    d.result = true;
+    qdict_iter(a, qdict_cmp_do_simple, &d);
+    return d.result;
+}
+
+static void event_prepare(TestEventData *data,
+                          const void *unused)
+{
+    /* Global variable test_event_data was used to pass the expectation, so
+       test cases can't be executed at same time. */
+    g_static_mutex_lock(&test_event_lock);
+
+    data->expect = qdict_new();
+    test_event_data = data;
+}
+
+static void event_teardown(TestEventData *data,
+                           const void *unused)
+{
+    QDECREF(data->expect);
+    test_event_data = NULL;
+
+    g_static_mutex_unlock(&test_event_lock);
+}
+
+static void test_event_a(TestEventData *data,
+                         const void *unused)
+{
+    QDict *d;
+    d = data->expect;
+    qdict_put(d, "event", qstring_from_str("EVENT_A"));
+    qapi_event_gen_EVENT_A(NULL);
+}
+
+static void test_event_b(TestEventData *data,
+                         const void *unused)
+{
+    QDict *d;
+    d = data->expect;
+    qdict_put(d, "event", qstring_from_str("EVENT_B"));
+    qapi_event_gen_EVENT_B(NULL);
+}
+
+static void test_event_c(TestEventData *data,
+                         const void *unused)
+{
+    QDict *d, *d_data, *d_b;
+
+    UserDefOne b;
+    b.integer = 2;
+    b.string = g_strdup("test1");
+    b.has_enum1 = false;
+
+    d_b = qdict_new();
+    qdict_put(d_b, "integer", qint_from_int(2));
+    qdict_put(d_b, "string", qstring_from_str("test1"));
+
+    d_data = qdict_new();
+    qdict_put(d_data, "a", qint_from_int(1));
+    qdict_put(d_data, "b", d_b);
+    qdict_put(d_data, "c", qstring_from_str("test2"));
+
+    d = data->expect;
+    qdict_put(d, "event", qstring_from_str("EVENT_C"));
+    qdict_put(d, "data", d_data);
+
+    qapi_event_gen_EVENT_C(true, 1, true, &b, "test2", NULL);
+
+    g_free(b.string);
+}
+
+/* Complex type */
+static void test_event_d(TestEventData *data,
+                         const void *unused)
+{
+    UserDefOne struct1;
+    EventStructOne a;
+    QDict *d, *d_data, *d_a, *d_struct1;
+
+    struct1.integer = 2;
+    struct1.string = g_strdup("test1");
+    struct1.has_enum1 = true;
+    struct1.enum1 = ENUM_ONE_VALUE1;
+
+    a.struct1 = &struct1;
+    a.string = g_strdup("test2");
+    a.has_enum2 = true;
+    a.enum2 = ENUM_ONE_VALUE2;
+
+    d_struct1 = qdict_new();
+    qdict_put(d_struct1, "integer", qint_from_int(2));
+    qdict_put(d_struct1, "string", qstring_from_str("test1"));
+    qdict_put(d_struct1, "enum1", qstring_from_str("value1"));
+
+    d_a = qdict_new();
+    qdict_put(d_a, "struct1", d_struct1);
+    qdict_put(d_a, "string", qstring_from_str("test2"));
+    qdict_put(d_a, "enum2", qstring_from_str("value2"));
+
+    d_data = qdict_new();
+    qdict_put(d_data, "a", d_a);
+    qdict_put(d_data, "b", qstring_from_str("test3"));
+    qdict_put(d_data, "enum3", qstring_from_str("value3"));
+
+    d = data->expect;
+    qdict_put(d, "event", qstring_from_str("EVENT_D"));
+    qdict_put(d, "data", d_data);
+
+    qapi_event_gen_EVENT_D(&a, "test3", false, NULL, true, ENUM_ONE_VALUE3,
+                           NULL);
+
+    g_free(struct1.string);
+    g_free(a.string);
+}
+
+static void event_test_emit(QDict *d, Error **errp)
+{
+    QObject *obj;
+    QDict *t;
+
+    /* Verify that we have timestamp, then remove it to compare other field */
+    obj = qdict_get(d, "timestamp");
+    g_assert(obj);
+    t = qobject_to_qdict(obj);
+    g_assert(t);
+    obj = qdict_get(t, "seconds");
+    g_assert(obj && qobject_type(obj) == QTYPE_QINT);
+    obj = qdict_get(t, "microseconds");
+    g_assert(obj && qobject_type(obj) == QTYPE_QINT);
+
+    qdict_del(d, "timestamp");
+
+    g_assert(qdict_cmp_simple(d, test_event_data->expect));
+
+}
+
+static void event_test_add(const char *testpath,
+                           void (*test_func)(TestEventData *data,
+                                             const void *user_data))
+{
+    g_test_add(testpath, TestEventData, NULL, event_prepare, test_func,
+               event_teardown);
+}
+
+
+
+int main(int argc, char **argv)
+{
+    qapi_event_set_func_emit(event_test_emit);
+
+    g_test_init(&argc, &argv, NULL);
+
+    event_test_add("/event/event_a", test_event_a);
+    event_test_add("/event/event_b", test_event_b);
+    event_test_add("/event/event_c", test_event_c);
+    event_test_add("/event/event_d", test_event_d);
+    g_test_run();
+
+    return 0;
+}
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [Qemu-devel] [PATCH RFC 3/3] tests: add test cases for qapi event support
  2013-11-13  1:44 [Qemu-devel] [PATCH RFC 0/3] add direct support of event in qapi schema Wenchao Xia
                   ` (2 preceding siblings ...)
  2013-11-13  1:44 ` [Qemu-devel] [PATCH 2/2] try Wenchao Xia
@ 2013-11-13  1:44 ` Wenchao Xia
  2013-12-13 13:36   ` Eric Blake
  2013-11-22  2:03 ` [Qemu-devel] [PATCH RFC 0/3] add direct support of event in qapi schema Wenchao Xia
  4 siblings, 1 reply; 30+ messages in thread
From: Wenchao Xia @ 2013-11-13  1:44 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, mdroth, armbru, pbonzini, lcapitulino, Wenchao Xia

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 tests/Makefile                          |   14 ++-
 tests/qapi-schema/qapi-schema-test.json |   12 ++
 tests/qapi-schema/qapi-schema-test.out  |   10 +-
 tests/test-qmp-event.c                  |  250 +++++++++++++++++++++++++++++++
 4 files changed, 281 insertions(+), 5 deletions(-)
 create mode 100644 tests/test-qmp-event.c

diff --git a/tests/Makefile b/tests/Makefile
index f414f2c..3c22f0a 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -23,6 +23,8 @@ check-unit-y += tests/test-string-input-visitor$(EXESUF)
 gcov-files-test-string-input-visitor-y = qapi/string-input-visitor.c
 check-unit-y += tests/test-string-output-visitor$(EXESUF)
 gcov-files-test-string-output-visitor-y = qapi/string-output-visitor.c
+check-unit-y += tests/test-qmp-event$(EXESUF)
+gcov-files-test-qmp-event-y += qapi/qmp-event.c
 check-unit-y += tests/test-opts-visitor$(EXESUF)
 gcov-files-test-opts-visitor-y = qapi/opts-visitor.c
 check-unit-y += tests/test-coroutine$(EXESUF)
@@ -120,7 +122,8 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
         trailing-comma-list.json trailing-comma-object.json \
         unclosed-list.json unclosed-object.json unclosed-string.json)
 
-GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h tests/test-qmp-commands.h
+GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h \
+                     tests/test-qapi-event.h tests/test-qmp-commands.h
 
 test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \
 	tests/check-qlist.o tests/check-qfloat.o tests/check-qjson.o \
@@ -129,9 +132,10 @@ test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.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-x86-cpuid.o tests/test-mul64.o tests/test-int128.o \
-	tests/test-opts-visitor.o
+	tests/test-opts-visitor.o tests/test-qmp-event.o
 
-test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o
+test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o \
+                  tests/test-qapi-event.o
 
 $(test-obj-y): QEMU_INCLUDES += -Itests
 QEMU_CFLAGS += -I$(SRC_PATH)/tests
@@ -167,12 +171,16 @@ $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-typ
 tests/test-qapi-visit.c tests/test-qapi-visit.h :\
 $(SRC_PATH)/tests/qapi-schema/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-qapi-event.c tests/test-qapi-event.h :\
+$(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-event.py
+	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-event.py $(gen-out-type) -o tests -p "test-" < $<, "  GEN   $@")
 tests/test-qmp-commands.h tests/test-qmp-marshal.c :\
 $(SRC_PATH)/tests/qapi-schema/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) libqemuutil.a libqemustub.a
 tests/test-string-input-visitor$(EXESUF): tests/test-string-input-visitor.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
+tests/test-qmp-event$(EXESUF): tests/test-qmp-event.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
 tests/test-qmp-output-visitor$(EXESUF): tests/test-qmp-output-visitor.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
 tests/test-qmp-input-visitor$(EXESUF): tests/test-qmp-input-visitor.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
 tests/test-qmp-input-strict$(EXESUF): tests/test-qmp-input-strict.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 1e23d21..3fd7902 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -93,3 +93,15 @@
     '*u16' : [ 'uint16' ],
     '*i64x':   'int'     ,
     '*u64x':   'uint64'  } }
+
+# testing event
+{ 'type': 'EventStructOne',
+  'data': { 'struct1': 'UserDefOne', 'string': 'str', '*enum2': 'EnumOne' } }
+
+{ 'event': 'EVENT_A' }
+{ 'event': 'EVENT_B',
+  'data': { } }
+{ 'event': 'EVENT_C',
+  'data': { '*a': 'int', '*b': 'UserDefOne', 'c': 'str' } }
+{ 'event': 'EVENT_D',
+  'data': { 'a' : 'EventStructOne', 'b' : 'str', '*c': 'str', '*enum3': 'EnumOne' } }
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index ce174a2..22430ba 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -16,7 +16,12 @@
  OrderedDict([('command', 'user_def_cmd'), ('data', OrderedDict())]),
  OrderedDict([('command', 'user_def_cmd1'), ('data', OrderedDict([('ud1a', 'UserDefOne')]))]),
  OrderedDict([('command', 'user_def_cmd2'), ('data', OrderedDict([('ud1a', 'UserDefOne'), ('ud1b', 'UserDefOne')])), ('returns', 'UserDefTwo')]),
- OrderedDict([('type', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))])]
+ OrderedDict([('type', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))]),
+ OrderedDict([('type', 'EventStructOne'), ('data', OrderedDict([('struct1', 'UserDefOne'), ('string', 'str'), ('*enum2', 'EnumOne')]))]),
+ OrderedDict([('event', 'EVENT_A')]),
+ OrderedDict([('event', 'EVENT_B'), ('data', OrderedDict())]),
+ OrderedDict([('event', 'EVENT_C'), ('data', OrderedDict([('*a', 'int'), ('*b', 'UserDefOne'), ('c', 'str')]))]),
+ OrderedDict([('event', 'EVENT_D'), ('data', OrderedDict([('a', 'EventStructOne'), ('b', 'str'), ('*c', 'str'), ('*enum3', 'EnumOne')]))])]
 [{'enum_name': 'EnumOne', 'enum_values': ['value1', 'value2', 'value3']},
  {'enum_name': 'UserDefUnionKind', 'enum_values': None},
  {'enum_name': 'UserDefBaseUnionKind', 'enum_values': None},
@@ -31,4 +36,5 @@
  OrderedDict([('type', 'UserDefBase0'), ('data', OrderedDict([('base-string0', 'str'), ('base-enum0', 'EnumOne')]))]),
  OrderedDict([('type', 'UserDefA'), ('data', OrderedDict([('boolean', 'bool')]))]),
  OrderedDict([('type', 'UserDefB'), ('data', OrderedDict([('integer', 'int')]))]),
- OrderedDict([('type', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))])]
+ OrderedDict([('type', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))]),
+ OrderedDict([('type', 'EventStructOne'), ('data', OrderedDict([('struct1', 'UserDefOne'), ('string', 'str'), ('*enum2', 'EnumOne')]))])]
diff --git a/tests/test-qmp-event.c b/tests/test-qmp-event.c
new file mode 100644
index 0000000..3b2c43e
--- /dev/null
+++ b/tests/test-qmp-event.c
@@ -0,0 +1,250 @@
+/*
+ * QMP Input Visitor unit-tests.
+ *
+ * Copyright (C) 2011 Red Hat Inc.
+ *
+ * Authors:
+ *  Luiz Capitulino <lcapitulino@redhat.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 <stdarg.h>
+
+#include "qemu-common.h"
+#include "test-qapi-types.h"
+#include "test-qapi-visit.h"
+#include "test-qapi-event.h"
+#include "qapi/qmp/types.h"
+#include "qapi/qmp/qobject.h"
+
+typedef struct TestEventData {
+    QDict *expect;
+} TestEventData;
+
+typedef struct QDictCmpData {
+    QDict *expect;
+    bool result;
+} QDictCmpData;
+
+TestEventData *test_event_data;
+static GStaticMutex test_event_lock = G_STATIC_MUTEX_INIT;
+
+/* Only compares bool, int, string */
+static
+void qdict_cmp_do_simple(const char *key, QObject *obj1, void *opaque)
+
+{
+    QObject *obj2;
+    QDictCmpData d_new, *d = opaque;
+
+    if (!d->result) {
+        return;
+    }
+
+    obj2 = qdict_get(d->expect, key);
+    if (!obj2) {
+        d->result = false;
+        return;
+    }
+
+    if (qobject_type(obj1) != qobject_type(obj2)) {
+        d->result = false;
+        return;
+    }
+
+    switch (qobject_type(obj1)) {
+    case QTYPE_QBOOL:
+        d->result = (qbool_get_int(qobject_to_qbool(obj1)) ==
+                     qbool_get_int(qobject_to_qbool(obj2)));
+        return;
+    case QTYPE_QINT:
+        d->result = (qint_get_int(qobject_to_qint(obj1)) ==
+                     qint_get_int(qobject_to_qint(obj2)));
+        return;
+    case QTYPE_QSTRING:
+        if (!g_strcmp0(qstring_get_str(qobject_to_qstring(obj1)),
+                       qstring_get_str(qobject_to_qstring(obj2)))) {
+            d->result = true;
+        } else {
+            d->result = false;
+        }
+        return;
+    case QTYPE_QDICT:
+        d_new.expect = qobject_to_qdict(obj2);
+        d_new.result = true;
+        qdict_iter(qobject_to_qdict(obj1), qdict_cmp_do_simple, &d_new);
+        d->result = d_new.result;
+        return;
+    default:
+        abort();
+    }
+}
+
+static bool qdict_cmp_simple(QDict *a, QDict *b)
+{
+    QDictCmpData d;
+
+    d.expect = b;
+    d.result = true;
+    qdict_iter(a, qdict_cmp_do_simple, &d);
+    return d.result;
+}
+
+static void event_prepare(TestEventData *data,
+                          const void *unused)
+{
+    /* Global variable test_event_data was used to pass the expectation, so
+       test cases can't be executed at same time. */
+    g_static_mutex_lock(&test_event_lock);
+
+    data->expect = qdict_new();
+    test_event_data = data;
+}
+
+static void event_teardown(TestEventData *data,
+                           const void *unused)
+{
+    QDECREF(data->expect);
+    test_event_data = NULL;
+
+    g_static_mutex_unlock(&test_event_lock);
+}
+
+static void test_event_a(TestEventData *data,
+                         const void *unused)
+{
+    QDict *d;
+    d = data->expect;
+    qdict_put(d, "event", qstring_from_str("EVENT_A"));
+    qapi_event_gen_EVENT_A(NULL);
+}
+
+static void test_event_b(TestEventData *data,
+                         const void *unused)
+{
+    QDict *d;
+    d = data->expect;
+    qdict_put(d, "event", qstring_from_str("EVENT_B"));
+    qapi_event_gen_EVENT_B(NULL);
+}
+
+static void test_event_c(TestEventData *data,
+                         const void *unused)
+{
+    QDict *d, *d_data, *d_b;
+
+    UserDefOne b;
+    b.integer = 2;
+    b.string = g_strdup("test1");
+    b.has_enum1 = false;
+
+    d_b = qdict_new();
+    qdict_put(d_b, "integer", qint_from_int(2));
+    qdict_put(d_b, "string", qstring_from_str("test1"));
+
+    d_data = qdict_new();
+    qdict_put(d_data, "a", qint_from_int(1));
+    qdict_put(d_data, "b", d_b);
+    qdict_put(d_data, "c", qstring_from_str("test2"));
+
+    d = data->expect;
+    qdict_put(d, "event", qstring_from_str("EVENT_C"));
+    qdict_put(d, "data", d_data);
+
+    qapi_event_gen_EVENT_C(true, 1, true, &b, "test2", NULL);
+
+    g_free(b.string);
+}
+
+/* Complex type */
+static void test_event_d(TestEventData *data,
+                         const void *unused)
+{
+    UserDefOne struct1;
+    EventStructOne a;
+    QDict *d, *d_data, *d_a, *d_struct1;
+
+    struct1.integer = 2;
+    struct1.string = g_strdup("test1");
+    struct1.has_enum1 = true;
+    struct1.enum1 = ENUM_ONE_VALUE1;
+
+    a.struct1 = &struct1;
+    a.string = g_strdup("test2");
+    a.has_enum2 = true;
+    a.enum2 = ENUM_ONE_VALUE2;
+
+    d_struct1 = qdict_new();
+    qdict_put(d_struct1, "integer", qint_from_int(2));
+    qdict_put(d_struct1, "string", qstring_from_str("test1"));
+    qdict_put(d_struct1, "enum1", qstring_from_str("value1"));
+
+    d_a = qdict_new();
+    qdict_put(d_a, "struct1", d_struct1);
+    qdict_put(d_a, "string", qstring_from_str("test2"));
+    qdict_put(d_a, "enum2", qstring_from_str("value2"));
+
+    d_data = qdict_new();
+    qdict_put(d_data, "a", d_a);
+    qdict_put(d_data, "b", qstring_from_str("test3"));
+    qdict_put(d_data, "enum3", qstring_from_str("value3"));
+
+    d = data->expect;
+    qdict_put(d, "event", qstring_from_str("EVENT_D"));
+    qdict_put(d, "data", d_data);
+
+    qapi_event_gen_EVENT_D(&a, "test3", false, NULL, true, ENUM_ONE_VALUE3,
+                           NULL);
+
+    g_free(struct1.string);
+    g_free(a.string);
+}
+
+static void event_test_emit(QDict *d, Error **errp)
+{
+    QObject *obj;
+    QDict *t;
+
+    /* Verify that we have timestamp, then remove it to compare other field */
+    obj = qdict_get(d, "timestamp");
+    g_assert(obj);
+    t = qobject_to_qdict(obj);
+    g_assert(t);
+    obj = qdict_get(t, "seconds");
+    g_assert(obj && qobject_type(obj) == QTYPE_QINT);
+    obj = qdict_get(t, "microseconds");
+    g_assert(obj && qobject_type(obj) == QTYPE_QINT);
+
+    qdict_del(d, "timestamp");
+
+    g_assert(qdict_cmp_simple(d, test_event_data->expect));
+
+}
+
+static void event_test_add(const char *testpath,
+                           void (*test_func)(TestEventData *data,
+                                             const void *user_data))
+{
+    g_test_add(testpath, TestEventData, NULL, event_prepare, test_func,
+               event_teardown);
+}
+
+
+
+int main(int argc, char **argv)
+{
+    qapi_event_set_func_emit(event_test_emit);
+
+    g_test_init(&argc, &argv, NULL);
+
+    event_test_add("/event/event_a", test_event_a);
+    event_test_add("/event/event_b", test_event_b);
+    event_test_add("/event/event_c", test_event_c);
+    event_test_add("/event/event_d", test_event_d);
+    g_test_run();
+
+    return 0;
+}
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] try
  2013-11-13  1:44 ` [Qemu-devel] [PATCH 2/2] try Wenchao Xia
@ 2013-11-13  1:47   ` Wenchao Xia
  0 siblings, 0 replies; 30+ messages in thread
From: Wenchao Xia @ 2013-11-13  1:47 UTC (permalink / raw)
  To: Wenchao Xia; +Cc: kwolf, qemu-devel, armbru, mdroth, lcapitulino, pbonzini

Sorry I sent this draft patch by mistake, please ignore this one and 
have a look at the rest
3 patches.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 0/3] add direct support of event in qapi schema
  2013-11-13  1:44 [Qemu-devel] [PATCH RFC 0/3] add direct support of event in qapi schema Wenchao Xia
                   ` (3 preceding siblings ...)
  2013-11-13  1:44 ` [Qemu-devel] [PATCH RFC 3/3] tests: add test cases for qapi event support Wenchao Xia
@ 2013-11-22  2:03 ` Wenchao Xia
  2013-11-22 14:33   ` Luiz Capitulino
  4 siblings, 1 reply; 30+ messages in thread
From: Wenchao Xia @ 2013-11-22  2:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, mdroth, armbru, pbonzini, lcapitulino, Wenchao Xia

于 2013/11/13 9:44, Wenchao Xia 写道:
> This series add support for tag/keyword 'event' in qapi-schema.
> The implemention doesn't generate a struture and visit function
> in the background for every event, so it doesn't support nested
> structure in the define to avoid trouble.
> 
> It is on top of series:
> http://lists.nongnu.org/archive/html/qemu-devel/2013-11/msg01574.html
> 
> Wenchao Xia (3):
>    1 os-posix: include sys/time.h
>    2 qapi script: add support of event
>    3 tests: add test cases for qapi event support
> 
>   Makefile                                |    9 +-
>   Makefile.objs                           |    2 +-
>   include/sysemu/os-posix.h               |    2 +
>   qapi/Makefile.objs                      |    1 +
>   scripts/qapi-event.py                   |  355 +++++++++++++++++++++++++++++++
>   tests/Makefile                          |   14 +-
>   tests/qapi-schema/qapi-schema-test.json |   12 +
>   tests/qapi-schema/qapi-schema-test.out  |   10 +-
>   tests/test-qmp-event.c                  |  250 ++++++++++++++++++++++
>   9 files changed, 646 insertions(+), 9 deletions(-)
>   create mode 100644 scripts/qapi-event.py
>   create mode 100644 tests/test-qmp-event.c
> 
  Luiz, do you think this series is in the right direction?

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 0/3] add direct support of event in qapi schema
  2013-11-22  2:03 ` [Qemu-devel] [PATCH RFC 0/3] add direct support of event in qapi schema Wenchao Xia
@ 2013-11-22 14:33   ` Luiz Capitulino
  0 siblings, 0 replies; 30+ messages in thread
From: Luiz Capitulino @ 2013-11-22 14:33 UTC (permalink / raw)
  To: Wenchao Xia; +Cc: kwolf, armbru, mdroth, qemu-devel, pbonzini

On Fri, 22 Nov 2013 10:03:44 +0800
Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:

> 于 2013/11/13 9:44, Wenchao Xia 写道:
> > This series add support for tag/keyword 'event' in qapi-schema.
> > The implemention doesn't generate a struture and visit function
> > in the background for every event, so it doesn't support nested
> > structure in the define to avoid trouble.
> > 
> > It is on top of series:
> > http://lists.nongnu.org/archive/html/qemu-devel/2013-11/msg01574.html
> > 
> > Wenchao Xia (3):
> >    1 os-posix: include sys/time.h
> >    2 qapi script: add support of event
> >    3 tests: add test cases for qapi event support
> > 
> >   Makefile                                |    9 +-
> >   Makefile.objs                           |    2 +-
> >   include/sysemu/os-posix.h               |    2 +
> >   qapi/Makefile.objs                      |    1 +
> >   scripts/qapi-event.py                   |  355 +++++++++++++++++++++++++++++++
> >   tests/Makefile                          |   14 +-
> >   tests/qapi-schema/qapi-schema-test.json |   12 +
> >   tests/qapi-schema/qapi-schema-test.out  |   10 +-
> >   tests/test-qmp-event.c                  |  250 ++++++++++++++++++++++
> >   9 files changed, 646 insertions(+), 9 deletions(-)
> >   create mode 100644 scripts/qapi-event.py
> >   create mode 100644 tests/test-qmp-event.c
> > 
>   Luiz, do you think this series is in the right direction?

Sorry, but I haven't reviewed it yet. Will do it next week.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
  2013-11-13  1:44 ` [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event Wenchao Xia
@ 2013-11-28  0:48   ` Luiz Capitulino
  2013-11-28  7:16     ` Wenchao Xia
  2013-12-13 13:31   ` Eric Blake
  1 sibling, 1 reply; 30+ messages in thread
From: Luiz Capitulino @ 2013-11-28  0:48 UTC (permalink / raw)
  To: Wenchao Xia; +Cc: kwolf, armbru, mdroth, qemu-devel, pbonzini

On Wed, 13 Nov 2013 09:44:52 +0800
Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:

> Nested structure is not supported now, so following define is not valid:
> { 'event': 'EVENT_C',
>   'data': { 'a': { 'a_a', 'str', 'a_b', 'str' }, 'b': 'int' }

I think your general approach is reasonable, but there are a number of
details to fix.

The first one is documentation. This patch's changelog is quite bad,
you don't say anything about what the does. You just mention a corner
case which doesn't happen to work. Please, add full changelog explaining
what this patch is about and please add examples of how an event
entry would look like in the schema and maybe you could also add the
important parts of a generated event function. Also, please add a
"event" section to docs/writing-qmp-commands.txt (in a different patch).

Secondly, for changes like this it's a very good idea to provide
conversion examples (as patches, not as changelog doc) at least
one or two so that we can see how it will look like.

More below.

> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  Makefile              |    9 +-
>  Makefile.objs         |    2 +-
>  qapi/Makefile.objs    |    1 +
>  scripts/qapi-event.py |  355 +++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 363 insertions(+), 4 deletions(-)
>  create mode 100644 scripts/qapi-event.py
> 
> diff --git a/Makefile b/Makefile
> index b15003f..a3e465f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -38,8 +38,8 @@ endif
>  endif
>  
>  GENERATED_HEADERS = config-host.h qemu-options.def
> -GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h
> -GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c
> +GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h qapi-event.h
> +GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c qapi-event.c
>  
>  GENERATED_HEADERS += trace/generated-events.h
>  GENERATED_SOURCES += trace/generated-events.c
> @@ -178,7 +178,7 @@ Makefile: $(version-obj-y) $(version-lobj-y)
>  # Build libraries
>  
>  libqemustub.a: $(stub-obj-y)
> -libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o
> +libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o qapi-event.o
>  
>  ######################################################################
>  
> @@ -219,6 +219,9 @@ $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
>  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 "." -b < $<, "  GEN   $@")
> +qapi-event.c qapi-event.h :\
> +$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-event.py $(qapi-py)
> +	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-event.py $(gen-out-type) -o "." -b < $<, "  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/Makefile.objs b/Makefile.objs
> index 2b6c1fe..33f5950 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -12,7 +12,7 @@ block-obj-y += main-loop.o iohandler.o qemu-timer.o
>  block-obj-$(CONFIG_POSIX) += aio-posix.o
>  block-obj-$(CONFIG_WIN32) += aio-win32.o
>  block-obj-y += block/
> -block-obj-y += qapi-types.o qapi-visit.o
> +block-obj-y += qapi-types.o qapi-visit.o qapi-event.o
>  block-obj-y += qemu-io-cmds.o
>  
>  block-obj-y += qemu-coroutine.o qemu-coroutine-lock.o qemu-coroutine-io.o
> diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs
> index 1f9c973..d14b769 100644
> --- a/qapi/Makefile.objs
> +++ b/qapi/Makefile.objs
> @@ -3,3 +3,4 @@ util-obj-y += qmp-output-visitor.o qmp-registry.o qmp-dispatch.o
>  util-obj-y += string-input-visitor.o string-output-visitor.o
>  
>  util-obj-y += opts-visitor.o
> +util-obj-y += qmp-event.o
> diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
> new file mode 100644
> index 0000000..4c6a0fe

I didn't review this hunk, but this series doesn't build for me:

  CC    qapi/string-output-visitor.o
  CC    qapi/opts-visitor.o
make: *** No rule to make target `qapi/qmp-event.o', needed by `libqemuutil.a'.  Stop.
~/work/src/upstream/qmp-unstable/build (tmp|AM)/ 

> --- /dev/null
> +++ b/scripts/qapi-event.py
> @@ -0,0 +1,355 @@
> +#
> +# QAPI event generator
> +#
> +# Copyright IBM, Corp. 2013
> +#
> +# Authors:
> +#  Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> +#
> +# This work is licensed under the terms of the GNU GPLv2.
> +# See the COPYING.LIB file in the top-level directory.
> +
> +from ordereddict import OrderedDict
> +from qapi import *
> +import sys
> +import os
> +import getopt
> +import errno
> +
> +def _generate_event_api_name_with_params(event_name, params):
> +    api_name = "void qapi_event_gen_%s(" % c_fun(event_name);

I'd prefer a name like qmp_notify_NAME() or qmp_send_event_NAME().

> +    l = len(api_name)
> +
> +    for argname, argentry, optional, structured in parse_args(params):
> +        if structured:
> +            sys.stderr.write("Nested structure define in event is not "
> +                             "supported now, event '%s', argname '%s'\n" %
> +                             (event_name, argname))
> +            sys.exit(1)
> +            continue
> +
> +        if optional:
> +            api_name += "bool has_%s,\n" % c_var(argname)
> +            api_name += "".ljust(l)
> +
> +        if argentry == "str":
> +            api_name += "const "
> +        api_name += "%s %s,\n" % (c_type(argentry), c_var(argname))
> +        api_name += "".ljust(l)
> +
> +    api_name += "Error **errp)"
> +    return api_name;
> +
> +def _generate_event_implement_with_params(api_name, event_name, params):
> +    ret = mcgen("""
> +
> +%(api_name)s
> +{
> +    QmpOutputVisitor *qov;
> +    Visitor *v;
> +    Error *err = NULL;

We usually call it *local_err;

> +    QObject *obj;
> +    QDict *qmp = NULL;

It's not needed to initialize qmp.

> +
> +    if (!qapi_event_functions.emit) {

Better to return an error here instead of silently failing.

> +        return;
> +    }
> +
> +    qmp = qdict_new();
> +    qdict_put(qmp, "event", qstring_from_str("%(event_name)s"));
> +    timestamp_put(qmp);

Maybe it's a good idea to move this to a function? Say
Qdict *qmp_build_event_dict(const char *event_name)?

> +
> +    qov = qmp_output_visitor_new();
> +    g_assert(qov);
> +
> +    v = qmp_output_get_visitor(qov);
> +    g_assert(v);
> +
> +    /* Fake visit, as if all member are under a structure */
> +    visit_start_struct(v, NULL, "", "%(event_name)s", 0, &err);
> +    if (error_is_set(&err)) {
> +        goto clean;
> +    }
> +
> +""",
> +                api_name = api_name,
> +                event_name = event_name)
> +
> +    for argname, argentry, optional, structured in parse_args(params):
> +        if structured:
> +            sys.stderr.write("Nested structure define in event is not "
> +                             "supported now, event '%s', argname '%s'\n" %
> +                             (event_name, argname))
> +            sys.exit(1)
> +            continue
> +
> +        if optional:
> +            ret += mcgen("""
> +    if (has_%(var)s) {
> +""",
> +                         var = c_var(argname))
> +            push_indent()
> +
> +        if argentry == "str":
> +            var_type = "(char **)"
> +        else:
> +            var_type = ""
> +
> +        ret += mcgen("""
> +    visit_type_%(type)s(v, %(var_type)s&%(var)s, "%(name)s", &err);
> +    if (error_is_set(&err)) {
> +        goto clean;
> +    }
> +""",
> +                     var_type = var_type,
> +                     var = c_var(argname),
> +                     type = type_name(argentry),
> +                     name = argname)
> +
> +        if optional:
> +            pop_indent()
> +            ret += mcgen("""
> +    }
> +""")
> +
> +    ret += mcgen("""
> +
> +    visit_end_struct(v, &err);
> +    if (error_is_set(&err)) {
> +        goto clean;
> +    }
> +
> +    obj = qmp_output_get_qobject(qov);
> +    g_assert(obj != NULL);
> +
> +    qdict_put_obj(qmp, "data", obj);
> +
> +    qapi_event_functions.emit(qmp, &err);
> +
> + clean:
> +    error_propagate(errp, err);
> +    qmp_output_visitor_cleanup(qov);
> +    QDECREF(qmp);
> +}
> +""")
> +
> +    return ret
> +
> +def _generate_event_api_name(event_name):
> +    return "void qapi_event_gen_%s(Error **errp)" % c_fun(event_name);
> +
> +def _generate_event_implement(api_name, event_name):
> +    return mcgen("""
> +
> +%(api_name)s
> +{
> +    QDict *qmp = NULL;

It's not needed to initialize qmp.

> +
> +    if (!qapi_event_functions.emit) {
> +        return;
> +    }
> +
> +    qmp = qdict_new();
> +    qdict_put(qmp, "event", qstring_from_str("%(event_name)s"));
> +    timestamp_put(qmp);
> +
> +    qapi_event_functions.emit(qmp, errp);
> +
> +    QDECREF(qmp);
> +}
> +""",
> +                api_name = api_name,
> +                event_name = event_name)
> +
> +
> +def generate_event_declaration(event_name, params):
> +    if params and len(params) > 0:
> +        api_name = _generate_event_api_name_with_params(event_name, params)
> +    else:
> +        api_name = _generate_event_api_name(event_name)
> +
> +    return mcgen('''
> +
> +%(api_name)s;
> +''',
> +                 api_name = api_name)
> +
> +def generate_event_implement(event_name, params):
> +    if params and len(params) > 0:
> +        api_name = _generate_event_api_name_with_params(event_name, params)
> +        ret =  _generate_event_implement_with_params(api_name,
> +                                                     event_name,
> +                                                     params)
> +
> +    else:
> +        api_name = _generate_event_api_name(event_name)
> +        ret =  _generate_event_implement(api_name,
> +                                         event_name)
> +    return ret
> +
> +try:
> +    opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:o:",
> +                                   ["source", "header", "builtins", "prefix=",
> +                                    "output-dir="])
> +except getopt.GetoptError, err:
> +    print str(err)
> +    sys.exit(1)
> +
> +output_dir = ""
> +prefix = ""
> +c_file = 'qapi-event.c'
> +h_file = 'qapi-event.h'
> +
> +do_c = False
> +do_h = False
> +do_builtins = 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
> +    elif o in ("-b", "--builtins"):
> +        do_builtins = 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
> +
> +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()
> +
> +fdef = maybe_open(do_c, c_file, 'w')
> +fdecl = maybe_open(do_h, h_file, 'w')
> +
> +fdef.write(mcgen('''
> +/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
> +
> +/*
> + * schema-defined QAPI event functions
> + *
> + * Copyright IBM, Corp. 2013
> + *
> + * Authors:
> + *  Wenchao Xia   <xiawenc@linux.vnet.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#include "qemu-common.h"
> +#include "%(header)s"
> +#include "%(prefix)sqapi-visit.h"
> +#include "qapi/qmp-output-visitor.h"
> +#include "qapi/qmp/qstring.h"
> +#include "qapi/qmp/qjson.h"
> +
> +#ifdef _WIN32
> +#include "sysemu/os-win32.h"
> +#endif
> +
> +#ifdef CONFIG_POSIX
> +#include "sysemu/os-posix.h"
> +#endif
> +
> +typedef struct QAPIEventFunctions {
> +    void (*emit)(QDict *dict, Error **errp);
> +} QAPIEventFunctions;
> +
> +QAPIEventFunctions qapi_event_functions;
> +
> +void qapi_event_set_func_emit(qapi_event_emit emit)
> +{
> +    qapi_event_functions.emit = emit;
> +}
> +
> +static void timestamp_put(QDict *qdict)
> +{
> +    int err;
> +    QObject *obj;
> +    qemu_timeval tv;
> +
> +    err = qemu_gettimeofday(&tv);
> +    if (err < 0)
> +        return;
> +
> +    obj = qobject_from_jsonf("{ 'seconds': %(p)s" PRId64 ", "
> +                                "'microseconds': %(p)s" PRId64 " }",
> +                                (int64_t) tv.tv_sec, (int64_t) tv.tv_usec);
> +    qdict_put_obj(qdict, "timestamp", obj);
> +}

Any special reason to generate these functions? Maybe they could be
put in qmp.c instead?

> +
> +''',
> +                 prefix=prefix, header=basename(h_file), p="%"))
> +
> +fdecl.write(mcgen('''
> +/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
> +
> +/*
> + * schema-defined QAPI event function
> + *
> + * Copyright IBM, Corp. 2013
> + *
> + * Authors:
> + *  Wenchao Xia  <xiawenc@linux.vnet.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#ifndef %(guard)s
> +#define %(guard)s
> +
> +#include "qapi/qmp/qdict.h"
> +#include "qapi/error.h"
> +#include "qapi/visitor.h"
> +#include "%(prefix)sqapi-types.h"
> +
> +typedef void (*qapi_event_emit)(QDict *d, Error **errp);
> +
> +void qapi_event_set_func_emit(qapi_event_emit emit);
> +
> +''',
> +                  prefix=prefix, guard=guardname(h_file)))
> +
> +exprs = parse_schema(sys.stdin)
> +
> +for expr in exprs:
> +    if expr.has_key('event'):
> +        event_name = expr['event']
> +        params = expr.get('data')
> +
> +        ret = generate_event_declaration(event_name, params)
> +        fdecl.write(ret)
> +
> +        ret = generate_event_implement(event_name, params)
> +        fdef.write(ret)
> +
> +fdecl.write('''
> +#endif
> +''')
> +
> +fdecl.flush()
> +fdecl.close()
> +
> +fdef.flush()
> +fdef.close()

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
  2013-11-28  0:48   ` Luiz Capitulino
@ 2013-11-28  7:16     ` Wenchao Xia
  2013-11-28 14:31       ` Luiz Capitulino
  0 siblings, 1 reply; 30+ messages in thread
From: Wenchao Xia @ 2013-11-28  7:16 UTC (permalink / raw)
  To: Luiz Capitulino; +Cc: kwolf, armbru, mdroth, qemu-devel, pbonzini

于 2013/11/28 8:48, Luiz Capitulino 写道:
> On Wed, 13 Nov 2013 09:44:52 +0800
> Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>
>> Nested structure is not supported now, so following define is not valid:
>> { 'event': 'EVENT_C',
>>    'data': { 'a': { 'a_a', 'str', 'a_b', 'str' }, 'b': 'int' }
>
> I think your general approach is reasonable, but there are a number of
> details to fix.
>
> The first one is documentation. This patch's changelog is quite bad,
> you don't say anything about what the does. You just mention a corner
> case which doesn't happen to work. Please, add full changelog explaining
> what this patch is about and please add examples of how an event
> entry would look like in the schema and maybe you could also add the
> important parts of a generated event function. Also, please add a
> "event" section to docs/writing-qmp-commands.txt (in a different patch).

   OK.

>
> Secondly, for changes like this it's a very good idea to provide
> conversion examples (as patches, not as changelog doc) at least
> one or two so that we can see how it will look like.
>

   Will add some in the intro part. By the way I think test
cases in patch 3 shows a bit.

> More below.
>

>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> ---
>>   Makefile              |    9 +-
>>   Makefile.objs         |    2 +-
>>   qapi/Makefile.objs    |    1 +
>>   scripts/qapi-event.py |  355 +++++++++++++++++++++++++++++++++++++++++++++++++
>>   4 files changed, 363 insertions(+), 4 deletions(-)
>>   create mode 100644 scripts/qapi-event.py
>>
>> diff --git a/Makefile b/Makefile
>> index b15003f..a3e465f 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -38,8 +38,8 @@ endif
>>   endif
>>
>>   GENERATED_HEADERS = config-host.h qemu-options.def
>> -GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h
>> -GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c
>> +GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h qapi-event.h
>> +GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c qapi-event.c
>>
>>   GENERATED_HEADERS += trace/generated-events.h
>>   GENERATED_SOURCES += trace/generated-events.c
>> @@ -178,7 +178,7 @@ Makefile: $(version-obj-y) $(version-lobj-y)
>>   # Build libraries
>>
>>   libqemustub.a: $(stub-obj-y)
>> -libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o
>> +libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o qapi-event.o
>>
>>   ######################################################################
>>
>> @@ -219,6 +219,9 @@ $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
>>   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 "." -b < $<, "  GEN   $@")
>> +qapi-event.c qapi-event.h :\
>> +$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-event.py $(qapi-py)
>> +	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-event.py $(gen-out-type) -o "." -b < $<, "  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/Makefile.objs b/Makefile.objs
>> index 2b6c1fe..33f5950 100644
>> --- a/Makefile.objs
>> +++ b/Makefile.objs
>> @@ -12,7 +12,7 @@ block-obj-y += main-loop.o iohandler.o qemu-timer.o
>>   block-obj-$(CONFIG_POSIX) += aio-posix.o
>>   block-obj-$(CONFIG_WIN32) += aio-win32.o
>>   block-obj-y += block/
>> -block-obj-y += qapi-types.o qapi-visit.o
>> +block-obj-y += qapi-types.o qapi-visit.o qapi-event.o
>>   block-obj-y += qemu-io-cmds.o
>>
>>   block-obj-y += qemu-coroutine.o qemu-coroutine-lock.o qemu-coroutine-io.o
>> diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs
>> index 1f9c973..d14b769 100644
>> --- a/qapi/Makefile.objs
>> +++ b/qapi/Makefile.objs
>> @@ -3,3 +3,4 @@ util-obj-y += qmp-output-visitor.o qmp-registry.o qmp-dispatch.o
>>   util-obj-y += string-input-visitor.o string-output-visitor.o
>>
>>   util-obj-y += opts-visitor.o
>> +util-obj-y += qmp-event.o
>> diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
>> new file mode 100644
>> index 0000000..4c6a0fe
>
> I didn't review this hunk, but this series doesn't build for me:
>
>    CC    qapi/string-output-visitor.o
>    CC    qapi/opts-visitor.o
> make: *** No rule to make target `qapi/qmp-event.o', needed by `libqemuutil.a'.  Stop.
> ~/work/src/upstream/qmp-unstable/build (tmp|AM)/
>
   A draft file I forgot to remove in Makefile, will fix.

>> --- /dev/null
>> +++ b/scripts/qapi-event.py
>> @@ -0,0 +1,355 @@
>> +#
>> +# QAPI event generator
>> +#
>> +# Copyright IBM, Corp. 2013
>> +#
>> +# Authors:
>> +#  Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> +#
>> +# This work is licensed under the terms of the GNU GPLv2.
>> +# See the COPYING.LIB file in the top-level directory.
>> +
>> +from ordereddict import OrderedDict
>> +from qapi import *
>> +import sys
>> +import os
>> +import getopt
>> +import errno
>> +
>> +def _generate_event_api_name_with_params(event_name, params):
>> +    api_name = "void qapi_event_gen_%s(" % c_fun(event_name);
>
> I'd prefer a name like qmp_notify_NAME() or qmp_send_event_NAME().
>

   do you think NAME should be capitalized as:
qmp_notify_SHUTDOWN()?

>> +    l = len(api_name)
>> +
>> +    for argname, argentry, optional, structured in parse_args(params):
>> +        if structured:
>> +            sys.stderr.write("Nested structure define in event is not "
>> +                             "supported now, event '%s', argname '%s'\n" %
>> +                             (event_name, argname))
>> +            sys.exit(1)
>> +            continue
>> +
>> +        if optional:
>> +            api_name += "bool has_%s,\n" % c_var(argname)
>> +            api_name += "".ljust(l)
>> +
>> +        if argentry == "str":
>> +            api_name += "const "
>> +        api_name += "%s %s,\n" % (c_type(argentry), c_var(argname))
>> +        api_name += "".ljust(l)
>> +
>> +    api_name += "Error **errp)"
>> +    return api_name;
>> +
>> +def _generate_event_implement_with_params(api_name, event_name, params):
>> +    ret = mcgen("""
>> +
>> +%(api_name)s
>> +{
>> +    QmpOutputVisitor *qov;
>> +    Visitor *v;
>> +    Error *err = NULL;
>
> We usually call it *local_err;
>

   OK.

>> +    QObject *obj;
>> +    QDict *qmp = NULL;
>
> It's not needed to initialize qmp.
>

   Will fix.

>> +
>> +    if (!qapi_event_functions.emit) {
>
> Better to return an error here instead of silently failing.
>

   The purpose is allowing emit=NULL and skip event code in that case.
If err is set, caller can't distinguish it from real error case.
caller:
     qmp_event_notify_SHUTDOWN(&err);
     if (error_is_set(&err)) {
         ...
     }

     err is always set when emit=NULL, but we may allow emit=NULL,
for example, qemu-img will have emit=NULL.

>> +        return;
>> +    }
>> +
>> +    qmp = qdict_new();
>> +    qdict_put(qmp, "event", qstring_from_str("%(event_name)s"));
>> +    timestamp_put(qmp);
>
> Maybe it's a good idea to move this to a function? Say
> Qdict *qmp_build_event_dict(const char *event_name)?
>

   Seems good, will use it.

>> +
>> +    qov = qmp_output_visitor_new();
>> +    g_assert(qov);
>> +
>> +    v = qmp_output_get_visitor(qov);
>> +    g_assert(v);
>> +
>> +    /* Fake visit, as if all member are under a structure */
>> +    visit_start_struct(v, NULL, "", "%(event_name)s", 0, &err);
>> +    if (error_is_set(&err)) {
>> +        goto clean;
>> +    }
>> +
>> +""",
>> +                api_name = api_name,
>> +                event_name = event_name)
>> +
>> +    for argname, argentry, optional, structured in parse_args(params):
>> +        if structured:
>> +            sys.stderr.write("Nested structure define in event is not "
>> +                             "supported now, event '%s', argname '%s'\n" %
>> +                             (event_name, argname))
>> +            sys.exit(1)
>> +            continue
>> +
>> +        if optional:
>> +            ret += mcgen("""
>> +    if (has_%(var)s) {
>> +""",
>> +                         var = c_var(argname))
>> +            push_indent()
>> +
>> +        if argentry == "str":
>> +            var_type = "(char **)"
>> +        else:
>> +            var_type = ""
>> +
>> +        ret += mcgen("""
>> +    visit_type_%(type)s(v, %(var_type)s&%(var)s, "%(name)s", &err);
>> +    if (error_is_set(&err)) {
>> +        goto clean;
>> +    }
>> +""",
>> +                     var_type = var_type,
>> +                     var = c_var(argname),
>> +                     type = type_name(argentry),
>> +                     name = argname)
>> +
>> +        if optional:
>> +            pop_indent()
>> +            ret += mcgen("""
>> +    }
>> +""")
>> +
>> +    ret += mcgen("""
>> +
>> +    visit_end_struct(v, &err);
>> +    if (error_is_set(&err)) {
>> +        goto clean;
>> +    }
>> +
>> +    obj = qmp_output_get_qobject(qov);
>> +    g_assert(obj != NULL);
>> +
>> +    qdict_put_obj(qmp, "data", obj);
>> +
>> +    qapi_event_functions.emit(qmp, &err);
>> +
>> + clean:
>> +    error_propagate(errp, err);
>> +    qmp_output_visitor_cleanup(qov);
>> +    QDECREF(qmp);
>> +}
>> +""")
>> +
>> +    return ret
>> +
>> +def _generate_event_api_name(event_name):
>> +    return "void qapi_event_gen_%s(Error **errp)" % c_fun(event_name);
>> +
>> +def _generate_event_implement(api_name, event_name):
>> +    return mcgen("""
>> +
>> +%(api_name)s
>> +{
>> +    QDict *qmp = NULL;
>
> It's not needed to initialize qmp.
>

   OK.

>> +
>> +    if (!qapi_event_functions.emit) {
>> +        return;
>> +    }
>> +
>> +    qmp = qdict_new();
>> +    qdict_put(qmp, "event", qstring_from_str("%(event_name)s"));
>> +    timestamp_put(qmp);
>> +
>> +    qapi_event_functions.emit(qmp, errp);
>> +
>> +    QDECREF(qmp);
>> +}
>> +""",
>> +                api_name = api_name,
>> +                event_name = event_name)
>> +
>> +
>> +def generate_event_declaration(event_name, params):
>> +    if params and len(params) > 0:
>> +        api_name = _generate_event_api_name_with_params(event_name, params)
>> +    else:
>> +        api_name = _generate_event_api_name(event_name)
>> +
>> +    return mcgen('''
>> +
>> +%(api_name)s;
>> +''',
>> +                 api_name = api_name)
>> +
>> +def generate_event_implement(event_name, params):
>> +    if params and len(params) > 0:
>> +        api_name = _generate_event_api_name_with_params(event_name, params)
>> +        ret =  _generate_event_implement_with_params(api_name,
>> +                                                     event_name,
>> +                                                     params)
>> +
>> +    else:
>> +        api_name = _generate_event_api_name(event_name)
>> +        ret =  _generate_event_implement(api_name,
>> +                                         event_name)
>> +    return ret
>> +
>> +try:
>> +    opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:o:",
>> +                                   ["source", "header", "builtins", "prefix=",
>> +                                    "output-dir="])
>> +except getopt.GetoptError, err:
>> +    print str(err)
>> +    sys.exit(1)
>> +
>> +output_dir = ""
>> +prefix = ""
>> +c_file = 'qapi-event.c'
>> +h_file = 'qapi-event.h'
>> +
>> +do_c = False
>> +do_h = False
>> +do_builtins = 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
>> +    elif o in ("-b", "--builtins"):
>> +        do_builtins = 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
>> +
>> +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()
>> +
>> +fdef = maybe_open(do_c, c_file, 'w')
>> +fdecl = maybe_open(do_h, h_file, 'w')
>> +
>> +fdef.write(mcgen('''
>> +/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
>> +
>> +/*
>> + * schema-defined QAPI event functions
>> + *
>> + * Copyright IBM, Corp. 2013
>> + *
>> + * Authors:
>> + *  Wenchao Xia   <xiawenc@linux.vnet.ibm.com>
>> + *
>> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
>> + * See the COPYING.LIB file in the top-level directory.
>> + *
>> + */
>> +
>> +#include "qemu-common.h"
>> +#include "%(header)s"
>> +#include "%(prefix)sqapi-visit.h"
>> +#include "qapi/qmp-output-visitor.h"
>> +#include "qapi/qmp/qstring.h"
>> +#include "qapi/qmp/qjson.h"
>> +
>> +#ifdef _WIN32
>> +#include "sysemu/os-win32.h"
>> +#endif
>> +
>> +#ifdef CONFIG_POSIX
>> +#include "sysemu/os-posix.h"
>> +#endif
>> +
>> +typedef struct QAPIEventFunctions {
>> +    void (*emit)(QDict *dict, Error **errp);
>> +} QAPIEventFunctions;
>> +
>> +QAPIEventFunctions qapi_event_functions;
>> +
>> +void qapi_event_set_func_emit(qapi_event_emit emit)
>> +{
>> +    qapi_event_functions.emit = emit;
>> +}
>> +
>> +static void timestamp_put(QDict *qdict)
>> +{
>> +    int err;
>> +    QObject *obj;
>> +    qemu_timeval tv;
>> +
>> +    err = qemu_gettimeofday(&tv);
>> +    if (err < 0)
>> +        return;
>> +
>> +    obj = qobject_from_jsonf("{ 'seconds': %(p)s" PRId64 ", "
>> +                                "'microseconds': %(p)s" PRId64 " }",
>> +                                (int64_t) tv.tv_sec, (int64_t) tv.tv_usec);
>> +    qdict_put_obj(qdict, "timestamp", obj);
>> +}
>
> Any special reason to generate these functions? Maybe they could be
> put in qmp.c instead?
>

   No, I just found no better place to go. qmp.c seems irrelevent to
event, how about new file qapi/qmp-event.c?(which I used before and
triggered the build error you met)

>> +
>> +''',
>> +                 prefix=prefix, header=basename(h_file), p="%"))
>> +
>> +fdecl.write(mcgen('''
>> +/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
>> +
>> +/*
>> + * schema-defined QAPI event function
>> + *
>> + * Copyright IBM, Corp. 2013
>> + *
>> + * Authors:
>> + *  Wenchao Xia  <xiawenc@linux.vnet.ibm.com>
>> + *
>> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
>> + * See the COPYING.LIB file in the top-level directory.
>> + *
>> + */
>> +
>> +#ifndef %(guard)s
>> +#define %(guard)s
>> +
>> +#include "qapi/qmp/qdict.h"
>> +#include "qapi/error.h"
>> +#include "qapi/visitor.h"
>> +#include "%(prefix)sqapi-types.h"
>> +
>> +typedef void (*qapi_event_emit)(QDict *d, Error **errp);
>> +
>> +void qapi_event_set_func_emit(qapi_event_emit emit);
>> +
>> +''',
>> +                  prefix=prefix, guard=guardname(h_file)))
>> +
>> +exprs = parse_schema(sys.stdin)
>> +
>> +for expr in exprs:
>> +    if expr.has_key('event'):
>> +        event_name = expr['event']
>> +        params = expr.get('data')
>> +
>> +        ret = generate_event_declaration(event_name, params)
>> +        fdecl.write(ret)
>> +
>> +        ret = generate_event_implement(event_name, params)
>> +        fdef.write(ret)
>> +
>> +fdecl.write('''
>> +#endif
>> +''')
>> +
>> +fdecl.flush()
>> +fdecl.close()
>> +
>> +fdef.flush()
>> +fdef.close()
>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
  2013-11-28  7:16     ` Wenchao Xia
@ 2013-11-28 14:31       ` Luiz Capitulino
  2013-12-02  6:48         ` Wenchao Xia
  0 siblings, 1 reply; 30+ messages in thread
From: Luiz Capitulino @ 2013-11-28 14:31 UTC (permalink / raw)
  To: Wenchao Xia; +Cc: kwolf, armbru, mdroth, qemu-devel, pbonzini

On Thu, 28 Nov 2013 15:16:08 +0800
Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:

> 于 2013/11/28 8:48, Luiz Capitulino 写道:
> > On Wed, 13 Nov 2013 09:44:52 +0800
> > Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
> >
> >> Nested structure is not supported now, so following define is not valid:
> >> { 'event': 'EVENT_C',
> >>    'data': { 'a': { 'a_a', 'str', 'a_b', 'str' }, 'b': 'int' }
> >
> > I think your general approach is reasonable, but there are a number of
> > details to fix.
> >
> > The first one is documentation. This patch's changelog is quite bad,
> > you don't say anything about what the does. You just mention a corner
> > case which doesn't happen to work. Please, add full changelog explaining
> > what this patch is about and please add examples of how an event
> > entry would look like in the schema and maybe you could also add the
> > important parts of a generated event function. Also, please add a
> > "event" section to docs/writing-qmp-commands.txt (in a different patch).
> 
>    OK.
> 
> >
> > Secondly, for changes like this it's a very good idea to provide
> > conversion examples (as patches, not as changelog doc) at least
> > one or two so that we can see how it will look like.
> >
> 
>    Will add some in the intro part. By the way I think test
> cases in patch 3 shows a bit.

I didn't look at the test to be honest (it didn't apply and I
concentrated on the second patch). Having a test is awesome, but
you still have to provide at least one conversion so that we can
see how it will look like.

> 
> > More below.
> >
> 
> >> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> >> ---
> >>   Makefile              |    9 +-
> >>   Makefile.objs         |    2 +-
> >>   qapi/Makefile.objs    |    1 +
> >>   scripts/qapi-event.py |  355 +++++++++++++++++++++++++++++++++++++++++++++++++
> >>   4 files changed, 363 insertions(+), 4 deletions(-)
> >>   create mode 100644 scripts/qapi-event.py
> >>
> >> diff --git a/Makefile b/Makefile
> >> index b15003f..a3e465f 100644
> >> --- a/Makefile
> >> +++ b/Makefile
> >> @@ -38,8 +38,8 @@ endif
> >>   endif
> >>
> >>   GENERATED_HEADERS = config-host.h qemu-options.def
> >> -GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h
> >> -GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c
> >> +GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h qapi-event.h
> >> +GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c qapi-event.c
> >>
> >>   GENERATED_HEADERS += trace/generated-events.h
> >>   GENERATED_SOURCES += trace/generated-events.c
> >> @@ -178,7 +178,7 @@ Makefile: $(version-obj-y) $(version-lobj-y)
> >>   # Build libraries
> >>
> >>   libqemustub.a: $(stub-obj-y)
> >> -libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o
> >> +libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o qapi-event.o
> >>
> >>   ######################################################################
> >>
> >> @@ -219,6 +219,9 @@ $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
> >>   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 "." -b < $<, "  GEN   $@")
> >> +qapi-event.c qapi-event.h :\
> >> +$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-event.py $(qapi-py)
> >> +	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-event.py $(gen-out-type) -o "." -b < $<, "  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/Makefile.objs b/Makefile.objs
> >> index 2b6c1fe..33f5950 100644
> >> --- a/Makefile.objs
> >> +++ b/Makefile.objs
> >> @@ -12,7 +12,7 @@ block-obj-y += main-loop.o iohandler.o qemu-timer.o
> >>   block-obj-$(CONFIG_POSIX) += aio-posix.o
> >>   block-obj-$(CONFIG_WIN32) += aio-win32.o
> >>   block-obj-y += block/
> >> -block-obj-y += qapi-types.o qapi-visit.o
> >> +block-obj-y += qapi-types.o qapi-visit.o qapi-event.o
> >>   block-obj-y += qemu-io-cmds.o
> >>
> >>   block-obj-y += qemu-coroutine.o qemu-coroutine-lock.o qemu-coroutine-io.o
> >> diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs
> >> index 1f9c973..d14b769 100644
> >> --- a/qapi/Makefile.objs
> >> +++ b/qapi/Makefile.objs
> >> @@ -3,3 +3,4 @@ util-obj-y += qmp-output-visitor.o qmp-registry.o qmp-dispatch.o
> >>   util-obj-y += string-input-visitor.o string-output-visitor.o
> >>
> >>   util-obj-y += opts-visitor.o
> >> +util-obj-y += qmp-event.o
> >> diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
> >> new file mode 100644
> >> index 0000000..4c6a0fe
> >
> > I didn't review this hunk, but this series doesn't build for me:
> >
> >    CC    qapi/string-output-visitor.o
> >    CC    qapi/opts-visitor.o
> > make: *** No rule to make target `qapi/qmp-event.o', needed by `libqemuutil.a'.  Stop.
> > ~/work/src/upstream/qmp-unstable/build (tmp|AM)/
> >
>    A draft file I forgot to remove in Makefile, will fix.
> 
> >> --- /dev/null
> >> +++ b/scripts/qapi-event.py
> >> @@ -0,0 +1,355 @@
> >> +#
> >> +# QAPI event generator
> >> +#
> >> +# Copyright IBM, Corp. 2013
> >> +#
> >> +# Authors:
> >> +#  Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> >> +#
> >> +# This work is licensed under the terms of the GNU GPLv2.
> >> +# See the COPYING.LIB file in the top-level directory.
> >> +
> >> +from ordereddict import OrderedDict
> >> +from qapi import *
> >> +import sys
> >> +import os
> >> +import getopt
> >> +import errno
> >> +
> >> +def _generate_event_api_name_with_params(event_name, params):
> >> +    api_name = "void qapi_event_gen_%s(" % c_fun(event_name);
> >
> > I'd prefer a name like qmp_notify_NAME() or qmp_send_event_NAME().
> >
> 
>    do you think NAME should be capitalized as:
> qmp_notify_SHUTDOWN()?

No way :) This kind of detail of the wire format shouldn't be part of
the C interface (and this is an unfortunate detail, btw).

> >> +    l = len(api_name)
> >> +
> >> +    for argname, argentry, optional, structured in parse_args(params):
> >> +        if structured:
> >> +            sys.stderr.write("Nested structure define in event is not "
> >> +                             "supported now, event '%s', argname '%s'\n" %
> >> +                             (event_name, argname))
> >> +            sys.exit(1)
> >> +            continue
> >> +
> >> +        if optional:
> >> +            api_name += "bool has_%s,\n" % c_var(argname)
> >> +            api_name += "".ljust(l)
> >> +
> >> +        if argentry == "str":
> >> +            api_name += "const "
> >> +        api_name += "%s %s,\n" % (c_type(argentry), c_var(argname))
> >> +        api_name += "".ljust(l)
> >> +
> >> +    api_name += "Error **errp)"
> >> +    return api_name;
> >> +
> >> +def _generate_event_implement_with_params(api_name, event_name, params):
> >> +    ret = mcgen("""
> >> +
> >> +%(api_name)s
> >> +{
> >> +    QmpOutputVisitor *qov;
> >> +    Visitor *v;
> >> +    Error *err = NULL;
> >
> > We usually call it *local_err;
> >
> 
>    OK.
> 
> >> +    QObject *obj;
> >> +    QDict *qmp = NULL;
> >
> > It's not needed to initialize qmp.
> >
> 
>    Will fix.
> 
> >> +
> >> +    if (!qapi_event_functions.emit) {
> >
> > Better to return an error here instead of silently failing.
> >
> 
>    The purpose is allowing emit=NULL and skip event code in that case.

But the code will do nothing and the caller won't know that.

Actually, I wonder if the code should even abort() in such a case,
as emit=NULL would be a programming today.

> If err is set, caller can't distinguish it from real error case.
> caller:
>      qmp_event_notify_SHUTDOWN(&err);
>      if (error_is_set(&err)) {
>          ...
>      }
> 
>      err is always set when emit=NULL, but we may allow emit=NULL,
> for example, qemu-img will have emit=NULL.
> 
> >> +        return;
> >> +    }
> >> +
> >> +    qmp = qdict_new();
> >> +    qdict_put(qmp, "event", qstring_from_str("%(event_name)s"));
> >> +    timestamp_put(qmp);
> >
> > Maybe it's a good idea to move this to a function? Say
> > Qdict *qmp_build_event_dict(const char *event_name)?
> >
> 
>    Seems good, will use it.
> 
> >> +
> >> +    qov = qmp_output_visitor_new();
> >> +    g_assert(qov);
> >> +
> >> +    v = qmp_output_get_visitor(qov);
> >> +    g_assert(v);
> >> +
> >> +    /* Fake visit, as if all member are under a structure */
> >> +    visit_start_struct(v, NULL, "", "%(event_name)s", 0, &err);
> >> +    if (error_is_set(&err)) {
> >> +        goto clean;
> >> +    }
> >> +
> >> +""",
> >> +                api_name = api_name,
> >> +                event_name = event_name)
> >> +
> >> +    for argname, argentry, optional, structured in parse_args(params):
> >> +        if structured:
> >> +            sys.stderr.write("Nested structure define in event is not "
> >> +                             "supported now, event '%s', argname '%s'\n" %
> >> +                             (event_name, argname))
> >> +            sys.exit(1)
> >> +            continue
> >> +
> >> +        if optional:
> >> +            ret += mcgen("""
> >> +    if (has_%(var)s) {
> >> +""",
> >> +                         var = c_var(argname))
> >> +            push_indent()
> >> +
> >> +        if argentry == "str":
> >> +            var_type = "(char **)"
> >> +        else:
> >> +            var_type = ""
> >> +
> >> +        ret += mcgen("""
> >> +    visit_type_%(type)s(v, %(var_type)s&%(var)s, "%(name)s", &err);
> >> +    if (error_is_set(&err)) {
> >> +        goto clean;
> >> +    }
> >> +""",
> >> +                     var_type = var_type,
> >> +                     var = c_var(argname),
> >> +                     type = type_name(argentry),
> >> +                     name = argname)
> >> +
> >> +        if optional:
> >> +            pop_indent()
> >> +            ret += mcgen("""
> >> +    }
> >> +""")
> >> +
> >> +    ret += mcgen("""
> >> +
> >> +    visit_end_struct(v, &err);
> >> +    if (error_is_set(&err)) {
> >> +        goto clean;
> >> +    }
> >> +
> >> +    obj = qmp_output_get_qobject(qov);
> >> +    g_assert(obj != NULL);
> >> +
> >> +    qdict_put_obj(qmp, "data", obj);
> >> +
> >> +    qapi_event_functions.emit(qmp, &err);
> >> +
> >> + clean:
> >> +    error_propagate(errp, err);
> >> +    qmp_output_visitor_cleanup(qov);
> >> +    QDECREF(qmp);
> >> +}
> >> +""")
> >> +
> >> +    return ret
> >> +
> >> +def _generate_event_api_name(event_name):
> >> +    return "void qapi_event_gen_%s(Error **errp)" % c_fun(event_name);
> >> +
> >> +def _generate_event_implement(api_name, event_name):
> >> +    return mcgen("""
> >> +
> >> +%(api_name)s
> >> +{
> >> +    QDict *qmp = NULL;
> >
> > It's not needed to initialize qmp.
> >
> 
>    OK.
> 
> >> +
> >> +    if (!qapi_event_functions.emit) {
> >> +        return;
> >> +    }
> >> +
> >> +    qmp = qdict_new();
> >> +    qdict_put(qmp, "event", qstring_from_str("%(event_name)s"));
> >> +    timestamp_put(qmp);
> >> +
> >> +    qapi_event_functions.emit(qmp, errp);
> >> +
> >> +    QDECREF(qmp);
> >> +}
> >> +""",
> >> +                api_name = api_name,
> >> +                event_name = event_name)
> >> +
> >> +
> >> +def generate_event_declaration(event_name, params):
> >> +    if params and len(params) > 0:
> >> +        api_name = _generate_event_api_name_with_params(event_name, params)
> >> +    else:
> >> +        api_name = _generate_event_api_name(event_name)
> >> +
> >> +    return mcgen('''
> >> +
> >> +%(api_name)s;
> >> +''',
> >> +                 api_name = api_name)
> >> +
> >> +def generate_event_implement(event_name, params):
> >> +    if params and len(params) > 0:
> >> +        api_name = _generate_event_api_name_with_params(event_name, params)
> >> +        ret =  _generate_event_implement_with_params(api_name,
> >> +                                                     event_name,
> >> +                                                     params)
> >> +
> >> +    else:
> >> +        api_name = _generate_event_api_name(event_name)
> >> +        ret =  _generate_event_implement(api_name,
> >> +                                         event_name)
> >> +    return ret
> >> +
> >> +try:
> >> +    opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:o:",
> >> +                                   ["source", "header", "builtins", "prefix=",
> >> +                                    "output-dir="])
> >> +except getopt.GetoptError, err:
> >> +    print str(err)
> >> +    sys.exit(1)
> >> +
> >> +output_dir = ""
> >> +prefix = ""
> >> +c_file = 'qapi-event.c'
> >> +h_file = 'qapi-event.h'
> >> +
> >> +do_c = False
> >> +do_h = False
> >> +do_builtins = 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
> >> +    elif o in ("-b", "--builtins"):
> >> +        do_builtins = 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
> >> +
> >> +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()
> >> +
> >> +fdef = maybe_open(do_c, c_file, 'w')
> >> +fdecl = maybe_open(do_h, h_file, 'w')
> >> +
> >> +fdef.write(mcgen('''
> >> +/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
> >> +
> >> +/*
> >> + * schema-defined QAPI event functions
> >> + *
> >> + * Copyright IBM, Corp. 2013
> >> + *
> >> + * Authors:
> >> + *  Wenchao Xia   <xiawenc@linux.vnet.ibm.com>
> >> + *
> >> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
> >> + * See the COPYING.LIB file in the top-level directory.
> >> + *
> >> + */
> >> +
> >> +#include "qemu-common.h"
> >> +#include "%(header)s"
> >> +#include "%(prefix)sqapi-visit.h"
> >> +#include "qapi/qmp-output-visitor.h"
> >> +#include "qapi/qmp/qstring.h"
> >> +#include "qapi/qmp/qjson.h"
> >> +
> >> +#ifdef _WIN32
> >> +#include "sysemu/os-win32.h"
> >> +#endif
> >> +
> >> +#ifdef CONFIG_POSIX
> >> +#include "sysemu/os-posix.h"
> >> +#endif
> >> +
> >> +typedef struct QAPIEventFunctions {
> >> +    void (*emit)(QDict *dict, Error **errp);
> >> +} QAPIEventFunctions;
> >> +
> >> +QAPIEventFunctions qapi_event_functions;
> >> +
> >> +void qapi_event_set_func_emit(qapi_event_emit emit)
> >> +{
> >> +    qapi_event_functions.emit = emit;
> >> +}
> >> +
> >> +static void timestamp_put(QDict *qdict)
> >> +{
> >> +    int err;
> >> +    QObject *obj;
> >> +    qemu_timeval tv;
> >> +
> >> +    err = qemu_gettimeofday(&tv);
> >> +    if (err < 0)
> >> +        return;
> >> +
> >> +    obj = qobject_from_jsonf("{ 'seconds': %(p)s" PRId64 ", "
> >> +                                "'microseconds': %(p)s" PRId64 " }",
> >> +                                (int64_t) tv.tv_sec, (int64_t) tv.tv_usec);
> >> +    qdict_put_obj(qdict, "timestamp", obj);
> >> +}
> >
> > Any special reason to generate these functions? Maybe they could be
> > put in qmp.c instead?
> >
> 
>    No, I just found no better place to go. qmp.c seems irrelevent to
> event, how about new file qapi/qmp-event.c?(which I used before and
> triggered the build error you met)

That works for me.

> 
> >> +
> >> +''',
> >> +                 prefix=prefix, header=basename(h_file), p="%"))
> >> +
> >> +fdecl.write(mcgen('''
> >> +/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
> >> +
> >> +/*
> >> + * schema-defined QAPI event function
> >> + *
> >> + * Copyright IBM, Corp. 2013
> >> + *
> >> + * Authors:
> >> + *  Wenchao Xia  <xiawenc@linux.vnet.ibm.com>
> >> + *
> >> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
> >> + * See the COPYING.LIB file in the top-level directory.
> >> + *
> >> + */
> >> +
> >> +#ifndef %(guard)s
> >> +#define %(guard)s
> >> +
> >> +#include "qapi/qmp/qdict.h"
> >> +#include "qapi/error.h"
> >> +#include "qapi/visitor.h"
> >> +#include "%(prefix)sqapi-types.h"
> >> +
> >> +typedef void (*qapi_event_emit)(QDict *d, Error **errp);
> >> +
> >> +void qapi_event_set_func_emit(qapi_event_emit emit);
> >> +
> >> +''',
> >> +                  prefix=prefix, guard=guardname(h_file)))
> >> +
> >> +exprs = parse_schema(sys.stdin)
> >> +
> >> +for expr in exprs:
> >> +    if expr.has_key('event'):
> >> +        event_name = expr['event']
> >> +        params = expr.get('data')
> >> +
> >> +        ret = generate_event_declaration(event_name, params)
> >> +        fdecl.write(ret)
> >> +
> >> +        ret = generate_event_implement(event_name, params)
> >> +        fdef.write(ret)
> >> +
> >> +fdecl.write('''
> >> +#endif
> >> +''')
> >> +
> >> +fdecl.flush()
> >> +fdecl.close()
> >> +
> >> +fdef.flush()
> >> +fdef.close()
> >
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
  2013-11-28 14:31       ` Luiz Capitulino
@ 2013-12-02  6:48         ` Wenchao Xia
  2013-12-13  3:00           ` Wenchao Xia
  0 siblings, 1 reply; 30+ messages in thread
From: Wenchao Xia @ 2013-12-02  6:48 UTC (permalink / raw)
  To: Luiz Capitulino; +Cc: kwolf, pbonzini, qemu-devel, armbru, mdroth


>>
>>>> +
>>>> +    if (!qapi_event_functions.emit) {
>>>
>>> Better to return an error here instead of silently failing.
>>>
>>
>>     The purpose is allowing emit=NULL and skip event code in that case.
>
> But the code will do nothing and the caller won't know that.
>
   Now the caller also won't know that useless code will be executed,
when qemu-img link with stub of monitor_event functions. :)

> Actually, I wonder if the code should even abort() in such a case,
> as emit=NULL would be a programming today.
>

   I am not sure why the code should always do something. The code may
actually take CPU resource to do nothing meanful, such as build up a
qdict and release it later, when emit is not a valid function. So I
did this as an improvement: check emit function ahead to escape useless
work.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
  2013-12-02  6:48         ` Wenchao Xia
@ 2013-12-13  3:00           ` Wenchao Xia
  0 siblings, 0 replies; 30+ messages in thread
From: Wenchao Xia @ 2013-12-13  3:00 UTC (permalink / raw)
  To: Luiz Capitulino; +Cc: kwolf, pbonzini, mdroth, qemu-devel, armbru

于 2013/12/2 14:48, Wenchao Xia 写道:
>
>>>
>>>>> +
>>>>> +    if (!qapi_event_functions.emit) {
>>>>
>>>> Better to return an error here instead of silently failing.
>>>>
>>>
>>>     The purpose is allowing emit=NULL and skip event code in that case.
>>
>> But the code will do nothing and the caller won't know that.
>>
>    Now the caller also won't know that useless code will be executed,
> when qemu-img link with stub of monitor_event functions. :)
>
>> Actually, I wonder if the code should even abort() in such a case,
>> as emit=NULL would be a programming today.
>>
>
>    I am not sure why the code should always do something. The code may
> actually take CPU resource to do nothing meanful, such as build up a
> qdict and release it later, when emit is not a valid function. So I
> did this as an improvement: check emit function ahead to escape useless
> work.
>
>

   Luiz, do you agree with me?

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 1/3] os-posix: include sys/time.h
  2013-11-13  1:44 ` [Qemu-devel] [PATCH RFC 1/3] os-posix: include sys/time.h Wenchao Xia
@ 2013-12-13 13:26   ` Eric Blake
  0 siblings, 0 replies; 30+ messages in thread
From: Eric Blake @ 2013-12-13 13:26 UTC (permalink / raw)
  To: Wenchao Xia, qemu-devel; +Cc: mdroth, kwolf, pbonzini, armbru, lcapitulino

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

On 11/12/2013 06:44 PM, Wenchao Xia wrote:
> Since gettimeofday() is used in this header file as a macro define,
> include the function's define header file, to avoid compile warning
> when other file include os-posix.h.
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  include/sysemu/os-posix.h |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
  2013-11-13  1:44 ` [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event Wenchao Xia
  2013-11-28  0:48   ` Luiz Capitulino
@ 2013-12-13 13:31   ` Eric Blake
  2013-12-13 13:43     ` Kevin Wolf
  2013-12-16  2:51     ` Wenchao Xia
  1 sibling, 2 replies; 30+ messages in thread
From: Eric Blake @ 2013-12-13 13:31 UTC (permalink / raw)
  To: Wenchao Xia, qemu-devel; +Cc: mdroth, kwolf, pbonzini, armbru, lcapitulino

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

On 11/12/2013 06:44 PM, Wenchao Xia wrote:
> Nested structure is not supported now, so following define is not valid:
> { 'event': 'EVENT_C',
>   'data': { 'a': { 'a_a', 'str', 'a_b', 'str' }, 'b': 'int' }

But what IS valid?  You need to document this in docs/qapi-code-gen.txt
at a bare minimum.

This patch series is hard to review, and still has the RFC subject line.
 At this point, I think it's worth rebasing and resending what you have;
even if it needs more review, it will at least get it to a state that is
easier to review.

> +++ b/scripts/qapi-event.py
> @@ -0,0 +1,355 @@
> +#
> +# QAPI event generator
> +#
> +# Copyright IBM, Corp. 2013
> +#
> +# Authors:
> +#  Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> +#
> +# This work is licensed under the terms of the GNU GPLv2.

Can you please use GPLv2+ (that is, add the "or later" clause)?  We
already have GPLv2-only code, but I don't want to increase the size of
that unfortunate license choice.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 3/3] tests: add test cases for qapi event support
  2013-11-13  1:44 ` [Qemu-devel] [PATCH RFC 3/3] tests: add test cases for qapi event support Wenchao Xia
@ 2013-12-13 13:36   ` Eric Blake
  2013-12-16  2:55     ` Wenchao Xia
  0 siblings, 1 reply; 30+ messages in thread
From: Eric Blake @ 2013-12-13 13:36 UTC (permalink / raw)
  To: Wenchao Xia, qemu-devel; +Cc: mdroth, kwolf, pbonzini, armbru, lcapitulino

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

On 11/12/2013 06:44 PM, Wenchao Xia wrote:
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---

> +++ b/tests/qapi-schema/qapi-schema-test.json
> @@ -93,3 +93,15 @@
>      '*u16' : [ 'uint16' ],
>      '*i64x':   'int'     ,
>      '*u64x':   'uint64'  } }
> +
> +# testing event
> +{ 'type': 'EventStructOne',
> +  'data': { 'struct1': 'UserDefOne', 'string': 'str', '*enum2': 'EnumOne' } }
> +
> +{ 'event': 'EVENT_A' }
> +{ 'event': 'EVENT_B',
> +  'data': { } }

Do we really have events with no associated information (the mere name
of the event carrying the full data?)

> +{ 'event': 'EVENT_C',
> +  'data': { '*a': 'int', '*b': 'UserDefOne', 'c': 'str' } }
> +{ 'event': 'EVENT_D',
> +  'data': { 'a' : 'EventStructOne', 'b' : 'str', '*c': 'str', '*enum3': 'EnumOne' } }

Looks like reasonable support for encoding existing events.

> + OrderedDict([('event', 'EVENT_A')]),
> + OrderedDict([('event', 'EVENT_B'), ('data', OrderedDict())]),

Shouldn't the omission of 'data' be the same as 'data':{}?

> +++ b/tests/test-qmp-event.c
> @@ -0,0 +1,250 @@
> +/*
> + * QMP Input Visitor unit-tests.
> + *
> + * Copyright (C) 2011 Red Hat Inc.

Unusual choice of copyright year and ownership.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
  2013-12-13 13:31   ` Eric Blake
@ 2013-12-13 13:43     ` Kevin Wolf
  2013-12-16  2:50       ` Wenchao Xia
  2013-12-16  9:13       ` Markus Armbruster
  2013-12-16  2:51     ` Wenchao Xia
  1 sibling, 2 replies; 30+ messages in thread
From: Kevin Wolf @ 2013-12-13 13:43 UTC (permalink / raw)
  To: Eric Blake; +Cc: qemu-devel, mdroth, armbru, pbonzini, lcapitulino, Wenchao Xia

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

Am 13.12.2013 um 14:31 hat Eric Blake geschrieben:
> On 11/12/2013 06:44 PM, Wenchao Xia wrote:
> > +++ b/scripts/qapi-event.py
> > @@ -0,0 +1,355 @@
> > +#
> > +# QAPI event generator
> > +#
> > +# Copyright IBM, Corp. 2013
> > +#
> > +# Authors:
> > +#  Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> > +#
> > +# This work is licensed under the terms of the GNU GPLv2.
> 
> Can you please use GPLv2+ (that is, add the "or later" clause)?  We
> already have GPLv2-only code, but I don't want to increase the size of
> that unfortunate license choice.

In fact, it's even worse:

+# This work is licensed under the terms of the GNU GPLv2.
+# See the COPYING.LIB file in the top-level directory.

These two lines contradict each other, COPYING.LIB contains the
LGPL 2.1. The same bad license header is in the other QAPI generator
scripts, so it's only copy&paste here.

This doesn't make things easier, because if things are copied, the
license of the source must be respected. And it seems rather dubious to
me what this license actually is. If it's GPLv2-only, we can't just
change it in the new copy.

Kevin

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
  2013-12-13 13:43     ` Kevin Wolf
@ 2013-12-16  2:50       ` Wenchao Xia
  2013-12-16  9:04         ` Markus Armbruster
  2013-12-16  9:13       ` Markus Armbruster
  1 sibling, 1 reply; 30+ messages in thread
From: Wenchao Xia @ 2013-12-16  2:50 UTC (permalink / raw)
  To: Kevin Wolf, Eric Blake; +Cc: mdroth, pbonzini, lcapitulino, qemu-devel, armbru

于 2013/12/13 21:43, Kevin Wolf 写道:
> Am 13.12.2013 um 14:31 hat Eric Blake geschrieben:
>> On 11/12/2013 06:44 PM, Wenchao Xia wrote:
>>> +++ b/scripts/qapi-event.py
>>> @@ -0,0 +1,355 @@
>>> +#
>>> +# QAPI event generator
>>> +#
>>> +# Copyright IBM, Corp. 2013
>>> +#
>>> +# Authors:
>>> +#  Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>>> +#
>>> +# This work is licensed under the terms of the GNU GPLv2.
>>
>> Can you please use GPLv2+ (that is, add the "or later" clause)?  We
>> already have GPLv2-only code, but I don't want to increase the size of
>> that unfortunate license choice.
>
> In fact, it's even worse:
>
> +# This work is licensed under the terms of the GNU GPLv2.
> +# See the COPYING.LIB file in the top-level directory.
>
> These two lines contradict each other, COPYING.LIB contains the
> LGPL 2.1. The same bad license header is in the other QAPI generator
> scripts, so it's only copy&paste here.
>
> This doesn't make things easier, because if things are copied, the
> license of the source must be respected. And it seems rather dubious to
> me what this license actually is. If it's GPLv2-only, we can't just
> change it in the new copy.
>
> Kevin
>
   ah..I am bad in license problem, will use the doc as LGPL from other
file.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
  2013-12-13 13:31   ` Eric Blake
  2013-12-13 13:43     ` Kevin Wolf
@ 2013-12-16  2:51     ` Wenchao Xia
  1 sibling, 0 replies; 30+ messages in thread
From: Wenchao Xia @ 2013-12-16  2:51 UTC (permalink / raw)
  To: Eric Blake, qemu-devel; +Cc: mdroth, kwolf, pbonzini, armbru, lcapitulino

于 2013/12/13 21:31, Eric Blake 写道:
> On 11/12/2013 06:44 PM, Wenchao Xia wrote:
>> Nested structure is not supported now, so following define is not valid:
>> { 'event': 'EVENT_C',
>>    'data': { 'a': { 'a_a', 'str', 'a_b', 'str' }, 'b': 'int' }
>
> But what IS valid?  You need to document this in docs/qapi-code-gen.txt
> at a bare minimum.
>
> This patch series is hard to review, and still has the RFC subject line.
>   At this point, I think it's worth rebasing and resending what you have;
> even if it needs more review, it will at least get it to a state that is
> easier to review.
>
   Sure, I will wait some time and respin.


>> +++ b/scripts/qapi-event.py
>> @@ -0,0 +1,355 @@
>> +#
>> +# QAPI event generator
>> +#
>> +# Copyright IBM, Corp. 2013
>> +#
>> +# Authors:
>> +#  Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> +#
>> +# This work is licensed under the terms of the GNU GPLv2.
>
> Can you please use GPLv2+ (that is, add the "or later" clause)?  We
> already have GPLv2-only code, but I don't want to increase the size of
> that unfortunate license choice.
>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 3/3] tests: add test cases for qapi event support
  2013-12-13 13:36   ` Eric Blake
@ 2013-12-16  2:55     ` Wenchao Xia
  2013-12-16  3:12       ` Wenchao Xia
  0 siblings, 1 reply; 30+ messages in thread
From: Wenchao Xia @ 2013-12-16  2:55 UTC (permalink / raw)
  To: Eric Blake, qemu-devel; +Cc: mdroth, kwolf, pbonzini, armbru, lcapitulino

于 2013/12/13 21:36, Eric Blake 写道:
> On 11/12/2013 06:44 PM, Wenchao Xia wrote:
>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> ---
>
>> +++ b/tests/qapi-schema/qapi-schema-test.json
>> @@ -93,3 +93,15 @@
>>       '*u16' : [ 'uint16' ],
>>       '*i64x':   'int'     ,
>>       '*u64x':   'uint64'  } }
>> +
>> +# testing event
>> +{ 'type': 'EventStructOne',
>> +  'data': { 'struct1': 'UserDefOne', 'string': 'str', '*enum2': 'EnumOne' } }
>> +
>> +{ 'event': 'EVENT_A' }
>> +{ 'event': 'EVENT_B',
>> +  'data': { } }
>
> Do we really have events with no associated information (the mere name
> of the event carrying the full data?)
>

   I guess EVENT_SHUDDOWN is one example, that have no associated info,
except time stamp.


>> +{ 'event': 'EVENT_C',
>> +  'data': { '*a': 'int', '*b': 'UserDefOne', 'c': 'str' } }
>> +{ 'event': 'EVENT_D',
>> +  'data': { 'a' : 'EventStructOne', 'b' : 'str', '*c': 'str', '*enum3': 'EnumOne' } }
>
> Looks like reasonable support for encoding existing events.
>
>> + OrderedDict([('event', 'EVENT_A')]),
>> + OrderedDict([('event', 'EVENT_B'), ('data', OrderedDict())]),
>
> Shouldn't the omission of 'data' be the same as 'data':{}?
>

   I didn't touch the "{ }" parsing part, maybe an existing parsing
style, will check it.

>> +++ b/tests/test-qmp-event.c
>> @@ -0,0 +1,250 @@
>> +/*
>> + * QMP Input Visitor unit-tests.
>> + *
>> + * Copyright (C) 2011 Red Hat Inc.
>
> Unusual choice of copyright year and ownership.
>
  yep, will fix.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 3/3] tests: add test cases for qapi event support
  2013-12-16  2:55     ` Wenchao Xia
@ 2013-12-16  3:12       ` Wenchao Xia
  2014-01-02  2:30         ` Wenchao Xia
  0 siblings, 1 reply; 30+ messages in thread
From: Wenchao Xia @ 2013-12-16  3:12 UTC (permalink / raw)
  To: Eric Blake, qemu-devel; +Cc: kwolf, pbonzini, lcapitulino, mdroth, armbru

于 2013/12/16 10:55, Wenchao Xia 写道:
> 于 2013/12/13 21:36, Eric Blake 写道:
>> On 11/12/2013 06:44 PM, Wenchao Xia wrote:
>>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>>> ---
>>
>>> +++ b/tests/qapi-schema/qapi-schema-test.json
>>> @@ -93,3 +93,15 @@
>>>       '*u16' : [ 'uint16' ],
>>>       '*i64x':   'int'     ,
>>>       '*u64x':   'uint64'  } }
>>> +
>>> +# testing event
>>> +{ 'type': 'EventStructOne',
>>> +  'data': { 'struct1': 'UserDefOne', 'string': 'str', '*enum2':
>>> 'EnumOne' } }
>>> +
>>> +{ 'event': 'EVENT_A' }
>>> +{ 'event': 'EVENT_B',
>>> +  'data': { } }
>>
>> Do we really have events with no associated information (the mere name
>> of the event carrying the full data?)
>>
>
>    I guess EVENT_SHUDDOWN is one example, that have no associated info,
> except time stamp.
>

   I think you questioned why EVENT_B is needed? I added it before to
allow that, both define can work.

>
>>> +{ 'event': 'EVENT_C',
>>> +  'data': { '*a': 'int', '*b': 'UserDefOne', 'c': 'str' } }
>>> +{ 'event': 'EVENT_D',
>>> +  'data': { 'a' : 'EventStructOne', 'b' : 'str', '*c': 'str',
>>> '*enum3': 'EnumOne' } }
>>
>> Looks like reasonable support for encoding existing events.
>>
>>> + OrderedDict([('event', 'EVENT_A')]),
>>> + OrderedDict([('event', 'EVENT_B'), ('data', OrderedDict())]),
>>
>> Shouldn't the omission of 'data' be the same as 'data':{}?
>>
>
>    I didn't touch the "{ }" parsing part, maybe an existing parsing
> style, will check it.
>
>>> +++ b/tests/test-qmp-event.c
>>> @@ -0,0 +1,250 @@
>>> +/*
>>> + * QMP Input Visitor unit-tests.
>>> + *
>>> + * Copyright (C) 2011 Red Hat Inc.
>>
>> Unusual choice of copyright year and ownership.
>>
>   yep, will fix.
>
>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
  2013-12-16  2:50       ` Wenchao Xia
@ 2013-12-16  9:04         ` Markus Armbruster
  0 siblings, 0 replies; 30+ messages in thread
From: Markus Armbruster @ 2013-12-16  9:04 UTC (permalink / raw)
  To: Wenchao Xia; +Cc: Kevin Wolf, qemu-devel, mdroth, pbonzini, lcapitulino

Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes:

> 于 2013/12/13 21:43, Kevin Wolf 写道:
>> Am 13.12.2013 um 14:31 hat Eric Blake geschrieben:
>>> On 11/12/2013 06:44 PM, Wenchao Xia wrote:
>>>> +++ b/scripts/qapi-event.py
>>>> @@ -0,0 +1,355 @@
>>>> +#
>>>> +# QAPI event generator
>>>> +#
>>>> +# Copyright IBM, Corp. 2013
>>>> +#
>>>> +# Authors:
>>>> +#  Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>>>> +#
>>>> +# This work is licensed under the terms of the GNU GPLv2.
>>>
>>> Can you please use GPLv2+ (that is, add the "or later" clause)?  We
>>> already have GPLv2-only code, but I don't want to increase the size of
>>> that unfortunate license choice.
>>
>> In fact, it's even worse:
>>
>> +# This work is licensed under the terms of the GNU GPLv2.
>> +# See the COPYING.LIB file in the top-level directory.
>>
>> These two lines contradict each other, COPYING.LIB contains the
>> LGPL 2.1. The same bad license header is in the other QAPI generator
>> scripts, so it's only copy&paste here.
>>
>> This doesn't make things easier, because if things are copied, the
>> license of the source must be respected. And it seems rather dubious to
>> me what this license actually is. If it's GPLv2-only, we can't just
>> change it in the new copy.
>>
>> Kevin
>>
>   ah..I am bad in license problem, will use the doc as LGPL from other
> file.

Please use GPLv2+ unless you have a specific reason for another license.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
  2013-12-13 13:43     ` Kevin Wolf
  2013-12-16  2:50       ` Wenchao Xia
@ 2013-12-16  9:13       ` Markus Armbruster
  2014-01-07 12:14         ` Markus Armbruster
  2014-01-16  2:50         ` Michael Roth
  1 sibling, 2 replies; 30+ messages in thread
From: Markus Armbruster @ 2013-12-16  9:13 UTC (permalink / raw)
  To: Kevin Wolf
  Cc: mdroth, qemu-devel, lcapitulino, Anthony Liguori, pbonzini, Wenchao Xia

[Licensing problem, cc: Anthony]

Kevin Wolf <kwolf@redhat.com> writes:

> Am 13.12.2013 um 14:31 hat Eric Blake geschrieben:
>> On 11/12/2013 06:44 PM, Wenchao Xia wrote:
>> > +++ b/scripts/qapi-event.py
>> > @@ -0,0 +1,355 @@
>> > +#
>> > +# QAPI event generator
>> > +#
>> > +# Copyright IBM, Corp. 2013
>> > +#
>> > +# Authors:
>> > +#  Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> > +#
>> > +# This work is licensed under the terms of the GNU GPLv2.
>> 
>> Can you please use GPLv2+ (that is, add the "or later" clause)?  We
>> already have GPLv2-only code, but I don't want to increase the size of
>> that unfortunate license choice.
>
> In fact, it's even worse:
>
> +# This work is licensed under the terms of the GNU GPLv2.
> +# See the COPYING.LIB file in the top-level directory.
>
> These two lines contradict each other, COPYING.LIB contains the
> LGPL 2.1. The same bad license header is in the other QAPI generator
> scripts, so it's only copy&paste here.

Specifically:

    File                        Commit
    scripts/qapi-commands.py    c17d9908
    scripts/qapi-visit.py       fb3182ce
    scripts/qapi-types.py       06d64c62
    scripts/qapi.py             0f923be2

All four from Michael Roth via Luiz.

> This doesn't make things easier, because if things are copied, the
> license of the source must be respected. And it seems rather dubious to
> me what this license actually is. If it's GPLv2-only, we can't just
> change it in the new copy.

IANAL, and I wouldn't dare to judge which of the two conflicting license
claims takes precedence.  Possibly neither, and then the files might
technically not be distributable.

Anyway, this mess needs to be addressed.  Michael, what was your
*intended* license?

If it wasn't GPLv2+, then why?

Do we need formal ACKs from all contributors to fix the licensing
comment in these four files?

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 3/3] tests: add test cases for qapi event support
  2013-12-16  3:12       ` Wenchao Xia
@ 2014-01-02  2:30         ` Wenchao Xia
  0 siblings, 0 replies; 30+ messages in thread
From: Wenchao Xia @ 2014-01-02  2:30 UTC (permalink / raw)
  To: Eric Blake, qemu-devel; +Cc: kwolf, pbonzini, mdroth, armbru, lcapitulino

>>>
>>>> + OrderedDict([('event', 'EVENT_A')]),
>>>> + OrderedDict([('event', 'EVENT_B'), ('data', OrderedDict())]),
>>>
>>> Shouldn't the omission of 'data' be the same as 'data':{}?
>>>
>>
>>    I didn't touch the "{ }" parsing part, maybe an existing parsing
>> style, will check it.
>>

   Checked the code, it is generated by tests/qapi-schema/test-qapi.py:

try:
     exprs = parse_schema(sys.stdin)
except SystemExit:
     raise
except:
     print >>sys.stderr, "Crashed:", sys.exc_info()[0]
     exit(1)

pprint(exprs)

   I think it worked as expectation for exprs.

>>>> +++ b/tests/test-qmp-event.c
>>>> @@ -0,0 +1,250 @@
>>>> +/*
>>>> + * QMP Input Visitor unit-tests.
>>>> + *
>>>> + * Copyright (C) 2011 Red Hat Inc.
>>>
>>> Unusual choice of copyright year and ownership.
>>>
>>   yep, will fix.
>>
>>
>
>

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
  2013-12-16  9:13       ` Markus Armbruster
@ 2014-01-07 12:14         ` Markus Armbruster
  2014-01-13 10:08           ` Markus Armbruster
  2014-01-16  2:50         ` Michael Roth
  1 sibling, 1 reply; 30+ messages in thread
From: Markus Armbruster @ 2014-01-07 12:14 UTC (permalink / raw)
  To: Kevin Wolf
  Cc: mdroth, qemu-devel, lcapitulino, Anthony Liguori, pbonzini, Wenchao Xia

Ping?

Markus Armbruster <armbru@redhat.com> writes:

> [Licensing problem, cc: Anthony]
>
> Kevin Wolf <kwolf@redhat.com> writes:
>
>> Am 13.12.2013 um 14:31 hat Eric Blake geschrieben:
>>> On 11/12/2013 06:44 PM, Wenchao Xia wrote:
>>> > +++ b/scripts/qapi-event.py
>>> > @@ -0,0 +1,355 @@
>>> > +#
>>> > +# QAPI event generator
>>> > +#
>>> > +# Copyright IBM, Corp. 2013
>>> > +#
>>> > +# Authors:
>>> > +#  Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>>> > +#
>>> > +# This work is licensed under the terms of the GNU GPLv2.
>>> 
>>> Can you please use GPLv2+ (that is, add the "or later" clause)?  We
>>> already have GPLv2-only code, but I don't want to increase the size of
>>> that unfortunate license choice.
>>
>> In fact, it's even worse:
>>
>> +# This work is licensed under the terms of the GNU GPLv2.
>> +# See the COPYING.LIB file in the top-level directory.
>>
>> These two lines contradict each other, COPYING.LIB contains the
>> LGPL 2.1. The same bad license header is in the other QAPI generator
>> scripts, so it's only copy&paste here.
>
> Specifically:
>
>     File                        Commit
>     scripts/qapi-commands.py    c17d9908
>     scripts/qapi-visit.py       fb3182ce
>     scripts/qapi-types.py       06d64c62
>     scripts/qapi.py             0f923be2
>
> All four from Michael Roth via Luiz.
>
>> This doesn't make things easier, because if things are copied, the
>> license of the source must be respected. And it seems rather dubious to
>> me what this license actually is. If it's GPLv2-only, we can't just
>> change it in the new copy.
>
> IANAL, and I wouldn't dare to judge which of the two conflicting license
> claims takes precedence.  Possibly neither, and then the files might
> technically not be distributable.
>
> Anyway, this mess needs to be addressed.  Michael, what was your
> *intended* license?
>
> If it wasn't GPLv2+, then why?
>
> Do we need formal ACKs from all contributors to fix the licensing
> comment in these four files?

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
  2014-01-07 12:14         ` Markus Armbruster
@ 2014-01-13 10:08           ` Markus Armbruster
  2014-01-15  1:47             ` Wenchao Xia
  0 siblings, 1 reply; 30+ messages in thread
From: Markus Armbruster @ 2014-01-13 10:08 UTC (permalink / raw)
  To: Kevin Wolf
  Cc: mdroth, qemu-devel, lcapitulino, Anthony Liguori, pbonzini, Wenchao Xia

Ping^2!

Markus Armbruster <armbru@redhat.com> writes:

> Ping?
>
> Markus Armbruster <armbru@redhat.com> writes:
>
>> [Licensing problem, cc: Anthony]
>>
>> Kevin Wolf <kwolf@redhat.com> writes:
>>
>>> Am 13.12.2013 um 14:31 hat Eric Blake geschrieben:
>>>> On 11/12/2013 06:44 PM, Wenchao Xia wrote:
>>>> > +++ b/scripts/qapi-event.py
>>>> > @@ -0,0 +1,355 @@
>>>> > +#
>>>> > +# QAPI event generator
>>>> > +#
>>>> > +# Copyright IBM, Corp. 2013
>>>> > +#
>>>> > +# Authors:
>>>> > +#  Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>>>> > +#
>>>> > +# This work is licensed under the terms of the GNU GPLv2.
>>>> 
>>>> Can you please use GPLv2+ (that is, add the "or later" clause)?  We
>>>> already have GPLv2-only code, but I don't want to increase the size of
>>>> that unfortunate license choice.
>>>
>>> In fact, it's even worse:
>>>
>>> +# This work is licensed under the terms of the GNU GPLv2.
>>> +# See the COPYING.LIB file in the top-level directory.
>>>
>>> These two lines contradict each other, COPYING.LIB contains the
>>> LGPL 2.1. The same bad license header is in the other QAPI generator
>>> scripts, so it's only copy&paste here.
>>
>> Specifically:
>>
>>     File                        Commit
>>     scripts/qapi-commands.py    c17d9908
>>     scripts/qapi-visit.py       fb3182ce
>>     scripts/qapi-types.py       06d64c62
>>     scripts/qapi.py             0f923be2
>>
>> All four from Michael Roth via Luiz.
>>
>>> This doesn't make things easier, because if things are copied, the
>>> license of the source must be respected. And it seems rather dubious to
>>> me what this license actually is. If it's GPLv2-only, we can't just
>>> change it in the new copy.
>>
>> IANAL, and I wouldn't dare to judge which of the two conflicting license
>> claims takes precedence.  Possibly neither, and then the files might
>> technically not be distributable.
>>
>> Anyway, this mess needs to be addressed.  Michael, what was your
>> *intended* license?
>>
>> If it wasn't GPLv2+, then why?
>>
>> Do we need formal ACKs from all contributors to fix the licensing
>> comment in these four files?

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
  2014-01-13 10:08           ` Markus Armbruster
@ 2014-01-15  1:47             ` Wenchao Xia
  0 siblings, 0 replies; 30+ messages in thread
From: Wenchao Xia @ 2014-01-15  1:47 UTC (permalink / raw)
  To: Markus Armbruster, Kevin Wolf
  Cc: pbonzini, lcapitulino, mdroth, Anthony Liguori, qemu-devel

于 2014/1/13 18:08, Markus Armbruster 写道:
> Ping^2!
> 
> Markus Armbruster <armbru@redhat.com> writes:
> 
>> Ping?
>>
>> Markus Armbruster <armbru@redhat.com> writes:
>>
>>> [Licensing problem, cc: Anthony]
>>>
>>> Kevin Wolf <kwolf@redhat.com> writes:
>>>
>>>> Am 13.12.2013 um 14:31 hat Eric Blake geschrieben:
>>>>> On 11/12/2013 06:44 PM, Wenchao Xia wrote:
>>>>>> +++ b/scripts/qapi-event.py
>>>>>> @@ -0,0 +1,355 @@
>>>>>> +#
>>>>>> +# QAPI event generator
>>>>>> +#
>>>>>> +# Copyright IBM, Corp. 2013
>>>>>> +#
>>>>>> +# Authors:
>>>>>> +#  Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>>>>>> +#
>>>>>> +# This work is licensed under the terms of the GNU GPLv2.
>>>>>
>>>>> Can you please use GPLv2+ (that is, add the "or later" clause)?  We
>>>>> already have GPLv2-only code, but I don't want to increase the size of
>>>>> that unfortunate license choice.
>>>>
>>>> In fact, it's even worse:
>>>>
>>>> +# This work is licensed under the terms of the GNU GPLv2.
>>>> +# See the COPYING.LIB file in the top-level directory.
>>>>
>>>> These two lines contradict each other, COPYING.LIB contains the
>>>> LGPL 2.1. The same bad license header is in the other QAPI generator
>>>> scripts, so it's only copy&paste here.
>>>
>>> Specifically:
>>>
>>>      File                        Commit
>>>      scripts/qapi-commands.py    c17d9908
>>>      scripts/qapi-visit.py       fb3182ce
>>>      scripts/qapi-types.py       06d64c62
>>>      scripts/qapi.py             0f923be2
>>>
>>> All four from Michael Roth via Luiz.
>>>
>>>> This doesn't make things easier, because if things are copied, the
>>>> license of the source must be respected. And it seems rather dubious to
>>>> me what this license actually is. If it's GPLv2-only, we can't just
>>>> change it in the new copy.
>>>
>>> IANAL, and I wouldn't dare to judge which of the two conflicting license
>>> claims takes precedence.  Possibly neither, and then the files might
>>> technically not be distributable.
>>>
>>> Anyway, this mess needs to be addressed.  Michael, what was your
>>> *intended* license?
>>>
>>> If it wasn't GPLv2+, then why?
>>>
>>> Do we need formal ACKs from all contributors to fix the licensing
>>> comment in these four files?
> 
  I used GPLv2+ in my new files of v2, but not sure about other files.
Michael, do you think other scripts should be changed either?

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
  2013-12-16  9:13       ` Markus Armbruster
  2014-01-07 12:14         ` Markus Armbruster
@ 2014-01-16  2:50         ` Michael Roth
  2014-01-16 11:05           ` Paolo Bonzini
  1 sibling, 1 reply; 30+ messages in thread
From: Michael Roth @ 2014-01-16  2:50 UTC (permalink / raw)
  To: Markus Armbruster, Kevin Wolf
  Cc: qemu-devel, lcapitulino, Anthony Liguori, pbonzini, Wenchao Xia

Quoting Markus Armbruster (2013-12-16 03:13:08)
> [Licensing problem, cc: Anthony]
> 
> Kevin Wolf <kwolf@redhat.com> writes:
> 
> > Am 13.12.2013 um 14:31 hat Eric Blake geschrieben:
> >> On 11/12/2013 06:44 PM, Wenchao Xia wrote:
> >> > +++ b/scripts/qapi-event.py
> >> > @@ -0,0 +1,355 @@
> >> > +#
> >> > +# QAPI event generator
> >> > +#
> >> > +# Copyright IBM, Corp. 2013
> >> > +#
> >> > +# Authors:
> >> > +#  Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> >> > +#
> >> > +# This work is licensed under the terms of the GNU GPLv2.
> >> 
> >> Can you please use GPLv2+ (that is, add the "or later" clause)?  We
> >> already have GPLv2-only code, but I don't want to increase the size of
> >> that unfortunate license choice.
> >
> > In fact, it's even worse:
> >
> > +# This work is licensed under the terms of the GNU GPLv2.
> > +# See the COPYING.LIB file in the top-level directory.
> >
> > These two lines contradict each other, COPYING.LIB contains the
> > LGPL 2.1. The same bad license header is in the other QAPI generator
> > scripts, so it's only copy&paste here.
> 
> Specifically:
> 
>     File                        Commit
>     scripts/qapi-commands.py    c17d9908
>     scripts/qapi-visit.py       fb3182ce
>     scripts/qapi-types.py       06d64c62
>     scripts/qapi.py             0f923be2
> 
> All four from Michael Roth via Luiz.
> 
> > This doesn't make things easier, because if things are copied, the
> > license of the source must be respected. And it seems rather dubious to
> > me what this license actually is. If it's GPLv2-only, we can't just
> > change it in the new copy.
> 
> IANAL, and I wouldn't dare to judge which of the two conflicting license
> claims takes precedence.  Possibly neither, and then the files might
> technically not be distributable.

IAAlsoNAL, but GPLv2 is explicit, whereas the "COPYING.LIB" simply
references a document with no information relevant to GPLv2, so I think a
strong case can be made that the intended license was GPLv2 and the
"clarification" is effectively a no-op.

> Anyway, this mess needs to be addressed.  Michael, what was your
> *intended* license?

GPLv2 was my intention at least (I meant to reference COPYING). But
I think we need Anthony's ack to be certain, since he was the original
author, and I added the screwed up license header after-the-fact under
the assumption that the code was to be GPLv2.

Here's the original:
http://repo.or.cz/w/qemu/aliguori.git/blob_plain/glib:/scripts/qapi-types.py

> 
> If it wasn't GPLv2+, then why?

This was committed prior to the push to switch to GPLv2+, but I'm fine
with relicensing my contributions as GPLv2+ should we opt to do so, but
I think that's a separate issue.

> 
> Do we need formal ACKs from all contributors to fix the licensing
> comment in these four files?

If we were actually re-licensing then yes (at least, that's what we've done
in the past). To clarify the existing license maybe not, but we should
probably err on the side of caution.

Current list seems to be:

mdroth@loki:~/w/qemu.git$ git log --format="%an: %ae" scripts/qapi* | sort | uniq
Amos Kong: akong@redhat.com
Anthony Liguori: aliguori@us.ibm.com
Anthony Liguori: anthony@codemonkey.ws
Avi Kivity: avi@redhat.com
Blue Swirl: blauwirbel@gmail.com
Cole Robinson: crobinso@redhat.com
Federico Simoncelli: fsimonce@redhat.com
Igor Mammedov: imammedo@redhat.com
Kevin Wolf: kwolf@redhat.com
Laszlo Ersek: lersek@redhat.com
Luiz Capitulino: lcapitulino@redhat.com
Markus Armbruster: armbru@redhat.com
Michael Roth: mdroth@linux.vnet.ibm.com
Paolo Bonzini: pbonzini@redhat.com
Peter Maydell: peter.maydell@linaro.org
Richard Henderson: rth@twiddle.net
Stefan Weil: sw@weilnetz.de
Tomoki Sekiyama: tomoki.sekiyama@hds.com

If we go to that effort, it may make sense to try to re-license to GPLv2+
while we're at it, but either way I think this should be done as a separate
patchset, and shouldn't hold up Wenchao's series. I can send that out, since
it's my screw-up.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
  2014-01-16  2:50         ` Michael Roth
@ 2014-01-16 11:05           ` Paolo Bonzini
  2014-01-30 10:35             ` Markus Armbruster
  0 siblings, 1 reply; 30+ messages in thread
From: Paolo Bonzini @ 2014-01-16 11:05 UTC (permalink / raw)
  To: Michael Roth
  Cc: Kevin Wolf, qemu-devel, Markus Armbruster, lcapitulino,
	Anthony Liguori, Wenchao Xia

Il 16/01/2014 03:50, Michael Roth ha scritto:
> If we go to that effort, it may make sense to try to re-license to GPLv2+
> while we're at it, but either way I think this should be done as a separate
> patchset, and shouldn't hold up Wenchao's series. I can send that out, since
> it's my screw-up.
> 

Removing Red Hat, Blue Swirl and Stefan Weil, who have agreed on a
blanket relicensing of GPLv2-only code to GPLv2+ leaves only half a
dozen people:

Anthony Liguori: anthony@codemonkey.ws (IBM?)
Michael Roth: mdroth@linux.vnet.ibm.com (IBM?)
Peter Maydell: peter.maydell@linaro.org
Richard Henderson: rth@twiddle.net
Tomoki Sekiyama: tomoki.sekiyama@hds.com

Paolo

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event
  2014-01-16 11:05           ` Paolo Bonzini
@ 2014-01-30 10:35             ` Markus Armbruster
  0 siblings, 0 replies; 30+ messages in thread
From: Markus Armbruster @ 2014-01-30 10:35 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Kevin Wolf, qemu-devel, Michael Roth, lcapitulino,
	Anthony Liguori, Wenchao Xia

Paolo Bonzini <pbonzini@redhat.com> writes:

> Il 16/01/2014 03:50, Michael Roth ha scritto:
>> If we go to that effort, it may make sense to try to re-license to GPLv2+
>> while we're at it, but either way I think this should be done as a separate
>> patchset, and shouldn't hold up Wenchao's series. I can send that out, since
>> it's my screw-up.
>> 
>
> Removing Red Hat, Blue Swirl and Stefan Weil, who have agreed on a
> blanket relicensing of GPLv2-only code to GPLv2+ leaves only half a
> dozen people:
>
> Anthony Liguori: anthony@codemonkey.ws (IBM?)
> Michael Roth: mdroth@linux.vnet.ibm.com (IBM?)
> Peter Maydell: peter.maydell@linaro.org
> Richard Henderson: rth@twiddle.net
> Tomoki Sekiyama: tomoki.sekiyama@hds.com

I sent a patch.  Now we hunt for the required ACKs.

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2014-01-30 10:35 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-13  1:44 [Qemu-devel] [PATCH RFC 0/3] add direct support of event in qapi schema Wenchao Xia
2013-11-13  1:44 ` [Qemu-devel] [PATCH RFC 1/3] os-posix: include sys/time.h Wenchao Xia
2013-12-13 13:26   ` Eric Blake
2013-11-13  1:44 ` [Qemu-devel] [PATCH RFC 2/3] qapi script: add support of event Wenchao Xia
2013-11-28  0:48   ` Luiz Capitulino
2013-11-28  7:16     ` Wenchao Xia
2013-11-28 14:31       ` Luiz Capitulino
2013-12-02  6:48         ` Wenchao Xia
2013-12-13  3:00           ` Wenchao Xia
2013-12-13 13:31   ` Eric Blake
2013-12-13 13:43     ` Kevin Wolf
2013-12-16  2:50       ` Wenchao Xia
2013-12-16  9:04         ` Markus Armbruster
2013-12-16  9:13       ` Markus Armbruster
2014-01-07 12:14         ` Markus Armbruster
2014-01-13 10:08           ` Markus Armbruster
2014-01-15  1:47             ` Wenchao Xia
2014-01-16  2:50         ` Michael Roth
2014-01-16 11:05           ` Paolo Bonzini
2014-01-30 10:35             ` Markus Armbruster
2013-12-16  2:51     ` Wenchao Xia
2013-11-13  1:44 ` [Qemu-devel] [PATCH 2/2] try Wenchao Xia
2013-11-13  1:47   ` Wenchao Xia
2013-11-13  1:44 ` [Qemu-devel] [PATCH RFC 3/3] tests: add test cases for qapi event support Wenchao Xia
2013-12-13 13:36   ` Eric Blake
2013-12-16  2:55     ` Wenchao Xia
2013-12-16  3:12       ` Wenchao Xia
2014-01-02  2:30         ` Wenchao Xia
2013-11-22  2:03 ` [Qemu-devel] [PATCH RFC 0/3] add direct support of event in qapi schema Wenchao Xia
2013-11-22 14:33   ` Luiz Capitulino

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.