* [Qemu-devel] [PATCH V6 01/29] os-posix: include sys/time.h
2014-06-05 12:21 [Qemu-devel] [PATCH V6 00/29] add direct support of event in qapi schema Wenchao Xia
@ 2014-06-05 12:21 ` Wenchao Xia
2014-06-05 12:21 ` [Qemu-devel] [PATCH V6 02/29] qapi: add event helper functions Wenchao Xia
` (28 subsequent siblings)
29 siblings, 0 replies; 77+ messages in thread
From: Wenchao Xia @ 2014-06-05 12:21 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, armbru, Wenchao Xia, lcapitulino
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 <wenchaoqemu@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.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] 77+ messages in thread* [Qemu-devel] [PATCH V6 02/29] qapi: add event helper functions
2014-06-05 12:21 [Qemu-devel] [PATCH V6 00/29] add direct support of event in qapi schema Wenchao Xia
2014-06-05 12:21 ` [Qemu-devel] [PATCH V6 01/29] os-posix: include sys/time.h Wenchao Xia
@ 2014-06-05 12:21 ` Wenchao Xia
2014-06-05 12:21 ` [Qemu-devel] [PATCH V6 03/29] qapi script: add event support Wenchao Xia
` (27 subsequent siblings)
29 siblings, 0 replies; 77+ messages in thread
From: Wenchao Xia @ 2014-06-05 12:21 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, armbru, Wenchao Xia, lcapitulino
This file holds some functions that do not need to be generated.
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
include/qapi/qmp-event.h | 27 +++++++++++++++++
qapi/Makefile.objs | 1 +
qapi/qmp-event.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 102 insertions(+), 0 deletions(-)
create mode 100644 include/qapi/qmp-event.h
create mode 100644 qapi/qmp-event.c
diff --git a/include/qapi/qmp-event.h b/include/qapi/qmp-event.h
new file mode 100644
index 0000000..02b6ce5
--- /dev/null
+++ b/include/qapi/qmp-event.h
@@ -0,0 +1,27 @@
+/*
+ * QMP Event related
+ *
+ * Copyright (c) 2014 Wenchao Xia
+ *
+ * Authors:
+ * Wenchao Xia <wenchaoqemu@gmail.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/error.h"
+#include "qapi/qmp/qdict.h"
+
+typedef void (*QMPEventFuncEmit)(int event_kind, QDict *dict, Error **errp);
+
+void qmp_event_set_func_emit(QMPEventFuncEmit emit);
+
+QMPEventFuncEmit qmp_event_get_func_emit(void);
+
+QDict *qmp_event_build_dict(const char *event_name);
+#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..0d1ce0b
--- /dev/null
+++ b/qapi/qmp-event.c
@@ -0,0 +1,74 @@
+/*
+ * QMP Event related
+ *
+ * Copyright (c) 2014 Wenchao Xia
+ *
+ * Authors:
+ * Wenchao Xia <wenchaoqemu@gmail.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 <inttypes.h>
+
+#include "qemu-common.h"
+#include "qapi/qmp-event.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
+
+static QMPEventFuncEmit qmp_emit;
+
+void qmp_event_set_func_emit(QMPEventFuncEmit emit)
+{
+ qmp_emit = emit;
+}
+
+QMPEventFuncEmit qmp_event_get_func_emit(void)
+{
+ return qmp_emit;
+}
+
+static void timestamp_put(QDict *qdict)
+{
+ int err;
+ QObject *obj;
+ qemu_timeval tv;
+ int64_t sec, usec;
+
+ err = qemu_gettimeofday(&tv);
+ if (err < 0) {
+ /* Put -1 to indicate failure of getting host time */
+ sec = -1;
+ usec = -1;
+ } else {
+ sec = tv.tv_sec;
+ usec = tv.tv_usec;
+ }
+
+ obj = qobject_from_jsonf("{ 'seconds': %" PRId64 ", "
+ "'microseconds': %" PRId64 " }",
+ sec, usec);
+ qdict_put_obj(qdict, "timestamp", obj);
+}
+
+/*
+ * Build a QDict, then fill event name and time stamp, caller should free the
+ * QDict after usage.
+ */
+QDict *qmp_event_build_dict(const char *event_name)
+{
+ QDict *dict = qdict_new();
+ qdict_put(dict, "event", qstring_from_str(event_name));
+ timestamp_put(dict);
+ return dict;
+}
--
1.7.1
^ permalink raw reply related [flat|nested] 77+ messages in thread* [Qemu-devel] [PATCH V6 03/29] qapi script: add event support
2014-06-05 12:21 [Qemu-devel] [PATCH V6 00/29] add direct support of event in qapi schema Wenchao Xia
2014-06-05 12:21 ` [Qemu-devel] [PATCH V6 01/29] os-posix: include sys/time.h Wenchao Xia
2014-06-05 12:21 ` [Qemu-devel] [PATCH V6 02/29] qapi: add event helper functions Wenchao Xia
@ 2014-06-05 12:21 ` Wenchao Xia
2014-06-13 16:47 ` Eric Blake
` (3 more replies)
2014-06-05 12:21 ` [Qemu-devel] [PATCH V6 04/29] test: add test cases for qapi event Wenchao Xia
` (26 subsequent siblings)
29 siblings, 4 replies; 77+ messages in thread
From: Wenchao Xia @ 2014-06-05 12:21 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, armbru, Wenchao Xia, lcapitulino
qapi-event.py will parse the schema and generate qapi-event.c, then
the API in qapi-event.c can be used to handle event in qemu code.
All API have prefix "qapi_event".
The script mainly includes two parts: generate API for each event
define, generate an enum type for all defined events.
Since in some cases the real emit behavior may change, for example,
qemu-img would not send a event, a callback layer is used to
control the behavior. As a result, the stubs at compile time
can be saved, the binding of block layer code and monitor code
will become looser.
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
---
Makefile | 11 +-
Makefile.objs | 2 +-
docs/qapi-code-gen.txt | 18 ++
scripts/qapi-event.py | 369 ++++++++++++++++++++++++++++++
scripts/qapi.py | 12 +
tests/Makefile | 2 +-
tests/qapi-schema/event-nest-struct.err | 1 +
tests/qapi-schema/event-nest-struct.exit | 1 +
tests/qapi-schema/event-nest-struct.json | 2 +
9 files changed, 413 insertions(+), 5 deletions(-)
create mode 100644 scripts/qapi-event.py
create mode 100644 tests/qapi-schema/event-nest-struct.err
create mode 100644 tests/qapi-schema/event-nest-struct.exit
create mode 100644 tests/qapi-schema/event-nest-struct.json
create mode 100644 tests/qapi-schema/event-nest-struct.out
diff --git a/Makefile b/Makefile
index d830483..237657e 100644
--- a/Makefile
+++ b/Makefile
@@ -45,8 +45,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
@@ -202,7 +202,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
block-modules = $(foreach o,$(block-obj-m),"$(basename $(subst /,-,$o))",) NULL
util/module.o-cflags = -D'CONFIG_BLOCK_MODULES=$(block-modules)'
@@ -256,6 +256,11 @@ $(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 -i $<, \
" 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 -i $<, \
+ " 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 \
diff --git a/Makefile.objs b/Makefile.objs
index b897e1d..1f76cea 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/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index dea0d50..c3d315f 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -215,6 +215,24 @@ An example command is:
'data': { 'arg1': 'str', '*arg2': 'str' },
'returns': 'str' }
+=== Events ===
+
+Events are defined with key word 'event'. When 'data' is also specified,
+additional info will be carried on. Finally there will be C API generated
+in qapi-event.h, and when called by QEMU code, a message with timestamp will
+be emitted on the wire. If timestamp is -1, it means failure to retrieve host
+time.
+
+An example event is:
+
+{ 'event': 'EVENT_C',
+ 'data': { '*a': 'int', 'b': 'str' } }
+
+Resulting in this JSON object:
+
+{ "event": "EVENT_C",
+ "data": { "b": "test string" },
+ "timestamp": { "seconds": 1267020223, "microseconds": 435656 } }
== Code generation ==
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
new file mode 100644
index 0000000..e1dcc43
--- /dev/null
+++ b/scripts/qapi-event.py
@@ -0,0 +1,369 @@
+#
+# QAPI event generator
+#
+# Copyright (c) 2014 Wenchao Xia
+#
+# Authors:
+# Wenchao Xia <wenchaoqemu@gmail.com>
+#
+# This work is licensed under the terms of the GNU GPL, version 2.
+# See the COPYING 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(event_name, params):
+ api_name = "void qapi_event_send_%s(" % c_fun(event_name).lower();
+ l = len(api_name)
+
+ if params:
+ for argname, argentry, optional, structured in parse_args(params):
+ 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;
+
+
+# Following are the core functions that generate C APIs to emit event.
+
+def generate_event_declaration(api_name):
+ return mcgen('''
+
+%(api_name)s;
+''',
+ api_name = api_name)
+
+def generate_event_implement(api_name, event_name, params):
+ # step 1: declare and variables
+ ret = mcgen("""
+
+%(api_name)s
+{
+ QDict *qmp;
+ Error *local_err = NULL;
+ QMPEventFuncEmit emit;
+""",
+ api_name = api_name)
+
+ if params:
+ ret += mcgen("""
+ QmpOutputVisitor *qov;
+ Visitor *v;
+ QObject *obj;
+
+""")
+
+ # step 2: check emit function, create a dict
+ ret += mcgen("""
+ emit = qmp_event_get_func_emit();
+ if (!emit) {
+ return;
+ }
+
+ qmp = qmp_event_build_dict("%(event_name)s");
+
+""",
+ event_name = event_name)
+
+ # step 3: visit the params if params != None
+ if params:
+ ret += mcgen("""
+ 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, &local_err);
+ if (local_err) {
+ goto clean;
+ }
+
+""",
+ event_name = event_name)
+
+ for argname, argentry, optional, structured in parse_args(params):
+ 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", &local_err);
+ if (local_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, &local_err);
+ if (local_err) {
+ goto clean;
+ }
+
+ obj = qmp_output_get_qobject(qov);
+ g_assert(obj != NULL);
+
+ qdict_put_obj(qmp, "data", obj);
+""")
+
+ # step 4: call qmp event api
+ ret += mcgen("""
+ emit(%(event_enum_value)s, qmp, &local_err);
+
+""",
+ event_enum_value = event_enum_value)
+
+ # step 5: clean up
+ if params:
+ ret += mcgen("""
+ clean:
+ qmp_output_visitor_cleanup(qov);
+""")
+ ret += mcgen("""
+ error_propagate(errp, local_err);
+ QDECREF(qmp);
+}
+""")
+
+ return ret
+
+
+# Following are the functions that generate an enum type for all defined
+# events, similar to qapi-types.py. Here we already have enum name and
+# values which were generated before and recorded in event_enum_*. It also
+# works around the issue that "import qapi-types" can't work.
+
+def generate_event_enum_decl(event_enum_name, event_enum_values):
+ lookup_decl = mcgen('''
+
+extern const char *%(event_enum_name)s_lookup[];
+''',
+ event_enum_name = event_enum_name)
+
+ enum_decl = mcgen('''
+typedef enum %(event_enum_name)s
+{
+''',
+ event_enum_name = event_enum_name)
+
+ # append automatically generated _MAX value
+ enum_max_value = generate_enum_full_value(event_enum_name, "MAX")
+ enum_values = event_enum_values + [ enum_max_value ]
+
+ i = 0
+ for value in enum_values:
+ enum_decl += mcgen('''
+ %(value)s = %(i)d,
+''',
+ value = value,
+ i = i)
+ i += 1
+
+ enum_decl += mcgen('''
+} %(event_enum_name)s;
+''',
+ event_enum_name = event_enum_name)
+
+ return lookup_decl + enum_decl
+
+def generate_event_enum_lookup(event_enum_name, event_enum_strings):
+ ret = mcgen('''
+
+const char *%(event_enum_name)s_lookup[] = {
+''',
+ event_enum_name = event_enum_name)
+
+ i = 0
+ for string in event_enum_strings:
+ ret += mcgen('''
+ "%(string)s",
+''',
+ string = string)
+
+ ret += mcgen('''
+ NULL,
+};
+''')
+ return ret
+
+
+# Start the real job
+
+try:
+ opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:i:o:",
+ ["source", "header", "builtins", "prefix=",
+ "input-file=", "output-dir="])
+except getopt.GetoptError, err:
+ print str(err)
+ sys.exit(1)
+
+input_file = ""
+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 ("-i", "--input-file"):
+ input_file = 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 (c) 2014 Wenchao Xia
+ *
+ * Authors:
+ * Wenchao Xia <wenchaoqemu@gmail.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-event.h"
+
+''',
+ prefix=prefix, header=basename(h_file)))
+
+fdecl.write(mcgen('''
+/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
+
+/*
+ * schema-defined QAPI event functions
+ *
+ * Copyright (c) 2014 Wenchao Xia
+ *
+ * Authors:
+ * Wenchao Xia <wenchaoqemu@gmail.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/error.h"
+#include "qapi/qmp/qdict.h"
+#include "%(prefix)sqapi-types.h"
+
+''',
+ prefix=prefix, guard=guardname(h_file)))
+
+exprs = parse_schema(input_file)
+
+event_enum_name = prefix.upper().replace('-', '_') + "QAPIEvent"
+event_enum_values = []
+event_enum_strings = []
+
+for expr in exprs:
+ if expr.has_key('event'):
+ event_name = expr['event']
+ params = expr.get('data')
+ if params and len(params) == 0:
+ params = None
+
+ api_name = _generate_event_api_name(event_name, params)
+ ret = generate_event_declaration(api_name)
+ fdecl.write(ret)
+
+ # We need an enum value per event
+ event_enum_value = generate_enum_full_value(event_enum_name,
+ event_name)
+ ret = generate_event_implement(api_name, event_name, params)
+ fdef.write(ret)
+
+ # Record it, and generate enum later
+ event_enum_values.append(event_enum_value)
+ event_enum_strings.append(event_name)
+
+ret = generate_event_enum_decl(event_enum_name, event_enum_values)
+fdecl.write(ret)
+ret = generate_event_enum_lookup(event_enum_name, event_enum_strings)
+fdef.write(ret)
+
+fdecl.write('''
+#endif
+''')
+
+fdecl.flush()
+fdecl.close()
+
+fdef.flush()
+fdef.close()
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 86e9608..9b488f2 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -248,6 +248,16 @@ def discriminator_find_enum_define(expr):
return find_enum(discriminator_type)
+def check_event(expr, expr_info):
+ params = expr.get('data')
+ if params:
+ for argname, argentry, optional, structured in parse_args(params):
+ if structured:
+ raise QAPIExprError(expr_info,
+ "Nested structure define in event is not "
+ "supported now, event '%s', argname '%s'"
+ % (expr['event'], argname))
+
def check_union(expr, expr_info):
name = expr['union']
base = expr.get('base')
@@ -311,6 +321,8 @@ def check_exprs(schema):
expr = expr_elem['expr']
if expr.has_key('union'):
check_union(expr, expr_elem['info'])
+ if expr.has_key('event'):
+ check_event(expr, expr_elem['info'])
def parse_schema(input_file):
try:
diff --git a/tests/Makefile b/tests/Makefile
index 6b294a7..287455f 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -198,7 +198,7 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
include-simple.json include-relpath.json include-format-err.json \
include-non-file.json include-no-file.json include-before-err.json \
include-nested-err.json include-self-cycle.json include-cycle.json \
- include-repetition.json)
+ include-repetition.json event-nest-struct.json)
GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h tests/test-qmp-commands.h
diff --git a/tests/qapi-schema/event-nest-struct.err b/tests/qapi-schema/event-nest-struct.err
new file mode 100644
index 0000000..e4a0faa
--- /dev/null
+++ b/tests/qapi-schema/event-nest-struct.err
@@ -0,0 +1 @@
+tests/qapi-schema/event-nest-struct.json:1: Nested structure define in event is not supported now, event 'EVENT_A', argname 'a'
diff --git a/tests/qapi-schema/event-nest-struct.exit b/tests/qapi-schema/event-nest-struct.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/event-nest-struct.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/event-nest-struct.json b/tests/qapi-schema/event-nest-struct.json
new file mode 100644
index 0000000..ee6f3ec
--- /dev/null
+++ b/tests/qapi-schema/event-nest-struct.json
@@ -0,0 +1,2 @@
+{ 'event': 'EVENT_A',
+ 'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } }
diff --git a/tests/qapi-schema/event-nest-struct.out b/tests/qapi-schema/event-nest-struct.out
new file mode 100644
index 0000000..e69de29
--
1.7.1
^ permalink raw reply related [flat|nested] 77+ messages in thread* Re: [Qemu-devel] [PATCH V6 03/29] qapi script: add event support
2014-06-05 12:21 ` [Qemu-devel] [PATCH V6 03/29] qapi script: add event support Wenchao Xia
@ 2014-06-13 16:47 ` Eric Blake
2014-06-13 21:28 ` Eric Blake
` (2 subsequent siblings)
3 siblings, 0 replies; 77+ messages in thread
From: Eric Blake @ 2014-06-13 16:47 UTC (permalink / raw)
To: Wenchao Xia, qemu-devel; +Cc: mdroth, armbru, lcapitulino
[-- Attachment #1: Type: text/plain, Size: 4372 bytes --]
On 06/05/2014 06:21 AM, Wenchao Xia wrote:
> qapi-event.py will parse the schema and generate qapi-event.c, then
> the API in qapi-event.c can be used to handle event in qemu code.
s/event in/events in/
> All API have prefix "qapi_event".
>
> The script mainly includes two parts: generate API for each event
> define, generate an enum type for all defined events.
>
> Since in some cases the real emit behavior may change, for example,
> qemu-img would not send a event, a callback layer is used to
> control the behavior. As a result, the stubs at compile time
> can be saved, the binding of block layer code and monitor code
> will become looser.
>
> Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
> ---
> Makefile | 11 +-
> Makefile.objs | 2 +-
> docs/qapi-code-gen.txt | 18 ++
> scripts/qapi-event.py | 369 ++++++++++++++++++++++++++++++
> scripts/qapi.py | 12 +
> tests/Makefile | 2 +-
> tests/qapi-schema/event-nest-struct.err | 1 +
> tests/qapi-schema/event-nest-struct.exit | 1 +
> tests/qapi-schema/event-nest-struct.json | 2 +
> 9 files changed, 413 insertions(+), 5 deletions(-)
> create mode 100644 scripts/qapi-event.py
> create mode 100644 tests/qapi-schema/event-nest-struct.err
> create mode 100644 tests/qapi-schema/event-nest-struct.exit
> create mode 100644 tests/qapi-schema/event-nest-struct.json
> create mode 100644 tests/qapi-schema/event-nest-struct.out
My python is weak, but I did apply this patch (or rather, used Paolo's
rebase of this patch) to test the generated files.
At this point, I'm interested in seeing the series go in sooner rather
than later, and my findings below are minor enough that I'm okay fixing
them in a followup patch rather than waiting for a respin of the whole
series.
Your patch fails to update .gitignore for the new generated files:
Untracked files:
(use "git add <file>..." to include in what will be committed)
qapi-event.c
qapi-event.h
>
> +=== Events ===
> +
> +Events are defined with key word 'event'. When 'data' is also specified,
> +additional info will be carried on. Finally there will be C API generated
s/carried on/included in the event/
> +++ b/scripts/qapi-event.py
> +
> +def _generate_event_api_name(event_name, params):
> + api_name = "void qapi_event_send_%s(" % c_fun(event_name).lower();
As of this commit, no events using qapi_event_send are generated yet.
I'm trusting that this code works, but I reserve the right to revisit
this patch later once I look at later patches and see what the generated
code is doing there. But for now, the generated code has merely:
const char *QAPIEvent_lookup[] = {
NULL,
};
which looks correct, even if a bit sparse :)
> +++ b/scripts/qapi.py
> @@ -248,6 +248,16 @@ def discriminator_find_enum_define(expr):
>
> return find_enum(discriminator_type)
>
> +def check_event(expr, expr_info):
> + params = expr.get('data')
> + if params:
> + for argname, argentry, optional, structured in parse_args(params):
> + if structured:
> + raise QAPIExprError(expr_info,
> + "Nested structure define in event is not "
> + "supported now, event '%s', argname '%s'"
s/ now// - we don't EVER want to support nested structures in events
(for that matter, I've been arguing in other threads that we want to
completely ditch support for nested structures to simplify the generator
code-base and make it possible to cleanly support argument defaults).
> +++ b/tests/qapi-schema/event-nest-struct.err
> @@ -0,0 +1 @@
> +tests/qapi-schema/event-nest-struct.json:1: Nested structure define in event is not supported now, event 'EVENT_A', argname 'a'
Of course, if you tweak the message above, you'll also have to tweak
this test. But thanks for adding a test!
Since I'm okay saving the cleanups mentioned above for a followup-patch,
and assuming I don't revisit this file:
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: 604 bytes --]
^ permalink raw reply [flat|nested] 77+ messages in thread* Re: [Qemu-devel] [PATCH V6 03/29] qapi script: add event support
2014-06-05 12:21 ` [Qemu-devel] [PATCH V6 03/29] qapi script: add event support Wenchao Xia
2014-06-13 16:47 ` Eric Blake
@ 2014-06-13 21:28 ` Eric Blake
2014-06-18 3:33 ` Eric Blake
2014-06-18 3:50 ` Eric Blake
3 siblings, 0 replies; 77+ messages in thread
From: Eric Blake @ 2014-06-13 21:28 UTC (permalink / raw)
To: Wenchao Xia, qemu-devel; +Cc: mdroth, armbru, lcapitulino
[-- Attachment #1: Type: text/plain, Size: 1426 bytes --]
On 06/05/2014 06:21 AM, Wenchao Xia wrote:
> qapi-event.py will parse the schema and generate qapi-event.c, then
> the API in qapi-event.c can be used to handle event in qemu code.
> All API have prefix "qapi_event".
>
As promised, a revisit of the generator code now that I've looked at
more of it in use. And since I've identified enough elsewhere that
warrants a series respin to v7, you should squash in fixes for these issues:
> +++ b/scripts/qapi-event.py
> +
> +def generate_event_implement(api_name, event_name, params):
> + # step 1: declare and variables
s/and/any/
> +
> + /* Fake visit, as if all member are under a structure */
s/member/members/
See also my comments on 16/29 - what if you change the generator to use
&error_abort instead of futzing around with local_error? After all, the
qmp_output_visitor doesn't ever set errp; and all of your callers
silently ignore errp even if it were to be set. Which makes sense,
since events are best-effort anyways (it's not like you are building up
a reply to a synchronous command, so you really don't have anyone to
inform about the failed event short of sending yet another event - but
if the first event failed, then your attempt to send another event to
inform about the failure is probably doomed).
--
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: 604 bytes --]
^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [Qemu-devel] [PATCH V6 03/29] qapi script: add event support
2014-06-05 12:21 ` [Qemu-devel] [PATCH V6 03/29] qapi script: add event support Wenchao Xia
2014-06-13 16:47 ` Eric Blake
2014-06-13 21:28 ` Eric Blake
@ 2014-06-18 3:33 ` Eric Blake
2014-06-18 6:06 ` Paolo Bonzini
2014-06-18 3:50 ` Eric Blake
3 siblings, 1 reply; 77+ messages in thread
From: Eric Blake @ 2014-06-18 3:33 UTC (permalink / raw)
To: Wenchao Xia, qemu-devel; +Cc: mdroth, armbru, lcapitulino
[-- Attachment #1: Type: text/plain, Size: 1142 bytes --]
On 06/05/2014 06:21 AM, Wenchao Xia wrote:
> qapi-event.py will parse the schema and generate qapi-event.c, then
> the API in qapi-event.c can be used to handle event in qemu code.
> All API have prefix "qapi_event".
>
> The script mainly includes two parts: generate API for each event
> define, generate an enum type for all defined events.
>
> Since in some cases the real emit behavior may change, for example,
> qemu-img would not send a event, a callback layer is used to
> control the behavior. As a result, the stubs at compile time
> can be saved, the binding of block layer code and monitor code
> will become looser.
>
> +++ b/scripts/qapi-event.py
> @@ -0,0 +1,369 @@
> +#
> +# QAPI event generator
> +#
> +# Copyright (c) 2014 Wenchao Xia
> +#
> +# Authors:
> +# Wenchao Xia <wenchaoqemu@gmail.com>
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2.
> +# See the COPYING file in the top-level directory.
Any reason this can't be GPLv2+ instead of GPLv2-only?
--
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: 604 bytes --]
^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [Qemu-devel] [PATCH V6 03/29] qapi script: add event support
2014-06-18 3:33 ` Eric Blake
@ 2014-06-18 6:06 ` Paolo Bonzini
2014-06-18 22:45 ` Wenchao Xia
0 siblings, 1 reply; 77+ messages in thread
From: Paolo Bonzini @ 2014-06-18 6:06 UTC (permalink / raw)
To: Eric Blake, Wenchao Xia, qemu-devel; +Cc: lcapitulino, mdroth, armbru
Il 18/06/2014 05:33, Eric Blake ha scritto:
>> > +# This work is licensed under the terms of the GNU GPL, version 2.
>> > +# See the COPYING file in the top-level directory.
> Any reason this can't be GPLv2+ instead of GPLv2-only?
I suppose because it copies parts of other qapi-* scripts. :(
Paolo
^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [Qemu-devel] [PATCH V6 03/29] qapi script: add event support
2014-06-18 6:06 ` Paolo Bonzini
@ 2014-06-18 22:45 ` Wenchao Xia
0 siblings, 0 replies; 77+ messages in thread
From: Wenchao Xia @ 2014-06-18 22:45 UTC (permalink / raw)
To: Paolo Bonzini, Eric Blake, qemu-devel; +Cc: lcapitulino, mdroth, armbru
于 2014/6/18 14:06, Paolo Bonzini 写道:
> Il 18/06/2014 05:33, Eric Blake ha scritto:
>>> > +# This work is licensed under the terms of the GNU GPL, version 2.
>>> > +# See the COPYING file in the top-level directory.
>> Any reason this can't be GPLv2+ instead of GPLv2-only?
>
> I suppose because it copies parts of other qapi-* scripts. :(
>
> Paolo
Yes, feel free to change the license.
^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [Qemu-devel] [PATCH V6 03/29] qapi script: add event support
2014-06-05 12:21 ` [Qemu-devel] [PATCH V6 03/29] qapi script: add event support Wenchao Xia
` (2 preceding siblings ...)
2014-06-18 3:33 ` Eric Blake
@ 2014-06-18 3:50 ` Eric Blake
3 siblings, 0 replies; 77+ messages in thread
From: Eric Blake @ 2014-06-18 3:50 UTC (permalink / raw)
To: Wenchao Xia, qemu-devel; +Cc: mdroth, armbru, lcapitulino
[-- Attachment #1: Type: text/plain, Size: 1171 bytes --]
On 06/05/2014 06:21 AM, Wenchao Xia wrote:
> qapi-event.py will parse the schema and generate qapi-event.c, then
> the API in qapi-event.c can be used to handle event in qemu code.
> All API have prefix "qapi_event".
>
> The script mainly includes two parts: generate API for each event
> define, generate an enum type for all defined events.
>
> +def _generate_event_api_name(event_name, params):
> + api_name = "void qapi_event_send_%s(" % c_fun(event_name).lower();
> + l = len(api_name)
> +
> + if params:
> + for argname, argentry, optional, structured in parse_args(params):
> + 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))
This may need to be rebased or have a followup patch based on Amos' work:
https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg02387.html
--
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: 604 bytes --]
^ permalink raw reply [flat|nested] 77+ messages in thread
* [Qemu-devel] [PATCH V6 04/29] test: add test cases for qapi event
2014-06-05 12:21 [Qemu-devel] [PATCH V6 00/29] add direct support of event in qapi schema Wenchao Xia
` (2 preceding siblings ...)
2014-06-05 12:21 ` [Qemu-devel] [PATCH V6 03/29] qapi script: add event support Wenchao Xia
@ 2014-06-05 12:21 ` Wenchao Xia
2014-06-13 17:05 ` Eric Blake
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 05/29] qapi: adjust existing defines Wenchao Xia
` (25 subsequent siblings)
29 siblings, 1 reply; 77+ messages in thread
From: Wenchao Xia @ 2014-06-05 12:21 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, armbru, Wenchao Xia, lcapitulino
These cases will verify whether the expected qdict is built.
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
---
tests/Makefile | 16 ++-
tests/qapi-schema/qapi-schema-test.json | 12 ++
tests/qapi-schema/qapi-schema-test.out | 10 +-
tests/test-qmp-event.c | 265 +++++++++++++++++++++++++++++++
4 files changed, 298 insertions(+), 5 deletions(-)
create mode 100644 tests/test-qmp-event.c
diff --git a/tests/Makefile b/tests/Makefile
index 287455f..f07a916 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -27,6 +27,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)
@@ -200,7 +202,8 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
include-nested-err.json include-self-cycle.json include-cycle.json \
include-repetition.json event-nest-struct.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-qmp-commands.h tests/test-qapi-event.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 \
@@ -209,9 +212,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
@@ -263,9 +267,15 @@ $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-com
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \
$(gen-out-type) -o tests -p "test-" -i $<, \
" 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-" -i $<, \
+ " 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 818c06d..ab4d3d9 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -89,3 +89,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 6cd03f3..95e9899 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -15,7 +15,12 @@
OrderedDict([('command', 'user_def_cmd1'), ('data', OrderedDict([('ud1a', 'UserDefOne')]))]),
OrderedDict([('command', 'user_def_cmd2'), ('data', OrderedDict([('ud1a', 'UserDefOne'), ('*ud1b', 'UserDefOne')])), ('returns', 'UserDefTwo')]),
OrderedDict([('command', 'user_def_cmd3'), ('data', OrderedDict([('a', 'int'), ('*b', 'int')])), ('returns', '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')]))]),
+ 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': 'UserDefAnonUnionKind', 'enum_values': None},
@@ -28,4 +33,5 @@
OrderedDict([('type', 'UserDefA'), ('data', OrderedDict([('boolean', 'bool')]))]),
OrderedDict([('type', 'UserDefB'), ('data', OrderedDict([('integer', 'int')]))]),
OrderedDict([('type', 'UserDefUnionBase'), ('data', OrderedDict([('string', 'str'), ('enum1', 'EnumOne')]))]),
- 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..c967d35
--- /dev/null
+++ b/tests/test-qmp-event.c
@@ -0,0 +1,265 @@
+/*
+ * qapi event unit-tests.
+ *
+ * Copyright (c) 2014 Wenchao Xia
+ *
+ * Authors:
+ * Wenchao Xia <wenchaoqemu@gmail.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 <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/qint.h"
+#include "qapi/qmp/qobject.h"
+#include "qapi/qmp-event.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:
+ d->result = g_strcmp0(qstring_get_str(qobject_to_qstring(obj1)),
+ qstring_get_str(qobject_to_qstring(obj2))) == 0;
+ 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;
+}
+
+/* This function is hooked as final emit function, which can verify the
+ correctness. */
+static void event_test_emit(int event_kind, QDict *d, Error **errp)
+{
+ QObject *obj;
+ QDict *t;
+ int64_t s, ms;
+
+ /* 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);
+ s = qint_get_int(qobject_to_qint(obj));
+ obj = qdict_get(t, "microseconds");
+ g_assert(obj && qobject_type(obj) == QTYPE_QINT);
+ ms = qint_get_int(qobject_to_qint(obj));
+ if (s == -1) {
+ g_assert(ms == -1);
+ } else {
+ g_assert(ms >= 0 && ms <= 999999);
+ }
+ g_assert(qdict_size(t) == 2);
+
+ qdict_del(d, "timestamp");
+
+ g_assert(qdict_cmp_simple(d, test_event_data->expect));
+
+}
+
+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 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);
+}
+
+
+/* Test cases */
+
+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_send_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_send_event_b(NULL);
+}
+
+static void test_event_c(TestEventData *data,
+ const void *unused)
+{
+ QDict *d, *d_data, *d_b;
+
+ UserDefOne b;
+ UserDefZero z;
+ z.integer = 2;
+ b.base = &z;
+ 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_send_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;
+ UserDefZero z;
+ QDict *d, *d_data, *d_a, *d_struct1;
+
+ z.integer = 2;
+ struct1.base = &z;
+ 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_send_event_d(&a, "test3", false, NULL, true, ENUM_ONE_VALUE3,
+ NULL);
+
+ g_free(struct1.string);
+ g_free(a.string);
+}
+
+int main(int argc, char **argv)
+{
+ qmp_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] 77+ messages in thread* Re: [Qemu-devel] [PATCH V6 04/29] test: add test cases for qapi event
2014-06-05 12:21 ` [Qemu-devel] [PATCH V6 04/29] test: add test cases for qapi event Wenchao Xia
@ 2014-06-13 17:05 ` Eric Blake
0 siblings, 0 replies; 77+ messages in thread
From: Eric Blake @ 2014-06-13 17:05 UTC (permalink / raw)
To: Wenchao Xia, qemu-devel; +Cc: mdroth, armbru, lcapitulino
[-- Attachment #1: Type: text/plain, Size: 878 bytes --]
On 06/05/2014 06:21 AM, Wenchao Xia wrote:
> These cases will verify whether the expected qdict is built.
>
> Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
> ---
> tests/Makefile | 16 ++-
> tests/qapi-schema/qapi-schema-test.json | 12 ++
> tests/qapi-schema/qapi-schema-test.out | 10 +-
> tests/test-qmp-event.c | 265 +++++++++++++++++++++++++++++++
> 4 files changed, 298 insertions(+), 5 deletions(-)
> create mode 100644 tests/test-qmp-event.c
>
> +
> + /* Verify that we have timestamp, then remove it to compare other field */
s/field/fields/
Minor enough to include that as part of your followup patch to the whole
series.
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: 604 bytes --]
^ permalink raw reply [flat|nested] 77+ messages in thread
* [Qemu-devel] [PATCH V6 05/29] qapi: adjust existing defines
2014-06-05 12:21 [Qemu-devel] [PATCH V6 00/29] add direct support of event in qapi schema Wenchao Xia
` (3 preceding siblings ...)
2014-06-05 12:21 ` [Qemu-devel] [PATCH V6 04/29] test: add test cases for qapi event Wenchao Xia
@ 2014-06-05 12:22 ` Wenchao Xia
2014-06-13 17:32 ` Eric Blake
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 06/29] monitor: add an implemention as qapi event emit method Wenchao Xia
` (24 subsequent siblings)
29 siblings, 1 reply; 77+ messages in thread
From: Wenchao Xia @ 2014-06-05 12:22 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, armbru, Wenchao Xia, lcapitulino
In order to let event defines use existing types later, instead of
redefine new ones, some old type defines for spice and vnc are changed,
and BlockErrorAction is moved from block.h to qapi schema. Note that
BlockErrorAction is not merged with BlockdevOnError.
One thing not perfect is that, VncInfo should be foldered but may break
API stability.
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
---
block.c | 17 ++++---
block/backup.c | 2 +-
block/mirror.c | 7 ++-
block/stream.c | 4 +-
blockjob.c | 11 ++--
hmp.c | 5 +-
hw/block/virtio-blk.c | 6 +-
hw/ide/core.c | 6 +-
hw/scsi/scsi-disk.c | 6 +-
include/block/block.h | 4 --
include/qemu/sockets.h | 2 +
qapi-schema.json | 126 ++++++++++++++++++++++++++++++++++++++----------
ui/spice-core.c | 7 ++-
ui/vnc.c | 9 ++--
util/qemu-sockets.c | 10 ++++
15 files changed, 156 insertions(+), 66 deletions(-)
diff --git a/block.c b/block.c
index 310ea89..84ad945 100644
--- a/block.c
+++ b/block.c
@@ -2140,13 +2140,13 @@ void bdrv_emit_qmp_error_event(const BlockDriverState *bdrv,
const char *action_str;
switch (action) {
- case BDRV_ACTION_REPORT:
+ case BLOCK_ERROR_ACTION_REPORT:
action_str = "report";
break;
- case BDRV_ACTION_IGNORE:
+ case BLOCK_ERROR_ACTION_IGNORE:
action_str = "ignore";
break;
- case BDRV_ACTION_STOP:
+ case BLOCK_ERROR_ACTION_STOP:
action_str = "stop";
break;
default:
@@ -3599,13 +3599,14 @@ BlockErrorAction bdrv_get_error_action(BlockDriverState *bs, bool is_read, int e
switch (on_err) {
case BLOCKDEV_ON_ERROR_ENOSPC:
- return (error == ENOSPC) ? BDRV_ACTION_STOP : BDRV_ACTION_REPORT;
+ return (error == ENOSPC) ?
+ BLOCK_ERROR_ACTION_STOP : BLOCK_ERROR_ACTION_REPORT;
case BLOCKDEV_ON_ERROR_STOP:
- return BDRV_ACTION_STOP;
+ return BLOCK_ERROR_ACTION_STOP;
case BLOCKDEV_ON_ERROR_REPORT:
- return BDRV_ACTION_REPORT;
+ return BLOCK_ERROR_ACTION_REPORT;
case BLOCKDEV_ON_ERROR_IGNORE:
- return BDRV_ACTION_IGNORE;
+ return BLOCK_ERROR_ACTION_IGNORE;
default:
abort();
}
@@ -3620,7 +3621,7 @@ void bdrv_error_action(BlockDriverState *bs, BlockErrorAction action,
{
assert(error >= 0);
bdrv_emit_qmp_error_event(bs, QEVENT_BLOCK_IO_ERROR, action, is_read);
- if (action == BDRV_ACTION_STOP) {
+ if (action == BLOCK_ERROR_ACTION_STOP) {
vm_stop(RUN_STATE_IO_ERROR);
bdrv_iostatus_set_err(bs, error);
}
diff --git a/block/backup.c b/block/backup.c
index 15a2e55..7978ae2 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -325,7 +325,7 @@ static void coroutine_fn backup_run(void *opaque)
/* Depending on error action, fail now or retry cluster */
BlockErrorAction action =
backup_error_action(job, error_is_read, -ret);
- if (action == BDRV_ACTION_REPORT) {
+ if (action == BLOCK_ERROR_ACTION_REPORT) {
break;
} else {
start--;
diff --git a/block/mirror.c b/block/mirror.c
index 94c8661..df58aea 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -118,7 +118,7 @@ static void mirror_write_complete(void *opaque, int ret)
bdrv_set_dirty(source, op->sector_num, op->nb_sectors);
action = mirror_error_action(s, false, -ret);
- if (action == BDRV_ACTION_REPORT && s->ret >= 0) {
+ if (action == BLOCK_ERROR_ACTION_REPORT && s->ret >= 0) {
s->ret = ret;
}
}
@@ -135,7 +135,7 @@ static void mirror_read_complete(void *opaque, int ret)
bdrv_set_dirty(source, op->sector_num, op->nb_sectors);
action = mirror_error_action(s, true, -ret);
- if (action == BDRV_ACTION_REPORT && s->ret >= 0) {
+ if (action == BLOCK_ERROR_ACTION_REPORT && s->ret >= 0) {
s->ret = ret;
}
@@ -415,7 +415,8 @@ static void coroutine_fn mirror_run(void *opaque)
trace_mirror_before_flush(s);
ret = bdrv_flush(s->target);
if (ret < 0) {
- if (mirror_error_action(s, false, -ret) == BDRV_ACTION_REPORT) {
+ if (mirror_error_action(s, false, -ret) ==
+ BLOCK_ERROR_ACTION_REPORT) {
goto immediate_exit;
}
} else {
diff --git a/block/stream.c b/block/stream.c
index 91d18a2..0433409 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -159,14 +159,14 @@ wait:
BlockErrorAction action =
block_job_error_action(&s->common, s->common.bs, s->on_error,
true, -ret);
- if (action == BDRV_ACTION_STOP) {
+ if (action == BLOCK_ERROR_ACTION_STOP) {
n = 0;
continue;
}
if (error == 0) {
error = ret;
}
- if (action == BDRV_ACTION_REPORT) {
+ if (action == BLOCK_ERROR_ACTION_REPORT) {
break;
}
}
diff --git a/blockjob.c b/blockjob.c
index 7d84ca1..bc63d42 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -262,22 +262,23 @@ BlockErrorAction block_job_error_action(BlockJob *job, BlockDriverState *bs,
switch (on_err) {
case BLOCKDEV_ON_ERROR_ENOSPC:
- action = (error == ENOSPC) ? BDRV_ACTION_STOP : BDRV_ACTION_REPORT;
+ action = (error == ENOSPC) ?
+ BLOCK_ERROR_ACTION_STOP : BLOCK_ERROR_ACTION_REPORT;
break;
case BLOCKDEV_ON_ERROR_STOP:
- action = BDRV_ACTION_STOP;
+ action = BLOCK_ERROR_ACTION_STOP;
break;
case BLOCKDEV_ON_ERROR_REPORT:
- action = BDRV_ACTION_REPORT;
+ action = BLOCK_ERROR_ACTION_REPORT;
break;
case BLOCKDEV_ON_ERROR_IGNORE:
- action = BDRV_ACTION_IGNORE;
+ action = BLOCK_ERROR_ACTION_IGNORE;
break;
default:
abort();
}
bdrv_emit_qmp_error_event(job->bs, QEVENT_BLOCK_JOB_ERROR, action, is_read);
- if (action == BDRV_ACTION_STOP) {
+ if (action == BLOCK_ERROR_ACTION_STOP) {
block_job_pause(job);
block_job_iostatus_set_err(job, error);
if (bs != job->bs) {
diff --git a/hmp.c b/hmp.c
index ccc35d4..1174bab 100644
--- a/hmp.c
+++ b/hmp.c
@@ -463,7 +463,8 @@ void hmp_info_vnc(Monitor *mon, const QDict *qdict)
for (client = info->clients; client; client = client->next) {
monitor_printf(mon, "Client:\n");
monitor_printf(mon, " address: %s:%s\n",
- client->value->host, client->value->service);
+ client->value->base->host,
+ client->value->base->service);
monitor_printf(mon, " x509_dname: %s\n",
client->value->x509_dname ?
client->value->x509_dname : "none");
@@ -511,7 +512,7 @@ void hmp_info_spice(Monitor *mon, const QDict *qdict)
for (chan = info->channels; chan; chan = chan->next) {
monitor_printf(mon, "Channel:\n");
monitor_printf(mon, " address: %s:%s%s\n",
- chan->value->host, chan->value->port,
+ chan->value->base->host, chan->value->base->port,
chan->value->tls ? " [tls]" : "");
monitor_printf(mon, " session: %" PRId64 "\n",
chan->value->connection_id);
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 8a568e5..62ac610 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -57,17 +57,17 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error,
BlockErrorAction action = bdrv_get_error_action(req->dev->bs, is_read, error);
VirtIOBlock *s = req->dev;
- if (action == BDRV_ACTION_STOP) {
+ if (action == BLOCK_ERROR_ACTION_STOP) {
req->next = s->rq;
s->rq = req;
- } else if (action == BDRV_ACTION_REPORT) {
+ } else if (action == BLOCK_ERROR_ACTION_REPORT) {
virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR);
bdrv_acct_done(s->bs, &req->acct);
g_free(req);
}
bdrv_error_action(s->bs, action, is_read, error);
- return action != BDRV_ACTION_IGNORE;
+ return action != BLOCK_ERROR_ACTION_IGNORE;
}
static void virtio_blk_rw_complete(void *opaque, int ret)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 1cac5f5..3a38f1e 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -596,10 +596,10 @@ static int ide_handle_rw_error(IDEState *s, int error, int op)
bool is_read = (op & BM_STATUS_RETRY_READ) != 0;
BlockErrorAction action = bdrv_get_error_action(s->bs, is_read, error);
- if (action == BDRV_ACTION_STOP) {
+ if (action == BLOCK_ERROR_ACTION_STOP) {
s->bus->dma->ops->set_unit(s->bus->dma, s->unit);
s->bus->error_status = op;
- } else if (action == BDRV_ACTION_REPORT) {
+ } else if (action == BLOCK_ERROR_ACTION_REPORT) {
if (op & BM_STATUS_DMA_RETRY) {
dma_buf_commit(s);
ide_dma_error(s);
@@ -608,7 +608,7 @@ static int ide_handle_rw_error(IDEState *s, int error, int op)
}
}
bdrv_error_action(s->bs, action, is_read, error);
- return action != BDRV_ACTION_IGNORE;
+ return action != BLOCK_ERROR_ACTION_IGNORE;
}
void ide_dma_cb(void *opaque, int ret)
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 4bcef55..f35229b 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -419,7 +419,7 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error)
SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
BlockErrorAction action = bdrv_get_error_action(s->qdev.conf.bs, is_read, error);
- if (action == BDRV_ACTION_REPORT) {
+ if (action == BLOCK_ERROR_ACTION_REPORT) {
switch (error) {
case ENOMEDIUM:
scsi_check_condition(r, SENSE_CODE(NO_MEDIUM));
@@ -439,10 +439,10 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error)
}
}
bdrv_error_action(s->qdev.conf.bs, action, is_read, error);
- if (action == BDRV_ACTION_STOP) {
+ if (action == BLOCK_ERROR_ACTION_STOP) {
scsi_req_retry(&r->req);
}
- return action != BDRV_ACTION_IGNORE;
+ return action != BLOCK_ERROR_ACTION_IGNORE;
}
static void scsi_write_complete(void * opaque, int ret)
diff --git a/include/block/block.h b/include/block/block.h
index faee3aa..a677baf 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -150,10 +150,6 @@ typedef enum {
#define BDRV_BLOCK_ALLOCATED 0x10
#define BDRV_BLOCK_OFFSET_MASK BDRV_SECTOR_MASK
-typedef enum {
- BDRV_ACTION_REPORT, BDRV_ACTION_IGNORE, BDRV_ACTION_STOP
-} BlockErrorAction;
-
typedef QSIMPLEQ_HEAD(BlockReopenQueue, BlockReopenQueueEntry) BlockReopenQueue;
typedef struct BDRVReopenState {
diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 45588d7..af24669 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -29,6 +29,7 @@ int inet_aton(const char *cp, struct in_addr *ia);
#include "qemu/option.h"
#include "qapi/error.h"
#include "qapi/qmp/qerror.h"
+#include "qapi-types.h"
extern QemuOptsList socket_optslist;
@@ -61,6 +62,7 @@ int inet_nonblocking_connect(const char *str,
int inet_dgram_opts(QemuOpts *opts, Error **errp);
const char *inet_strfamily(int family);
+NetworkAddressFamily inet_netfamily(int family);
int unix_listen_opts(QemuOpts *opts, Error **errp);
int unix_listen(const char *path, char *ostr, int olen, Error **errp);
diff --git a/qapi-schema.json b/qapi-schema.json
index 7bc33ea..115d8d0 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1163,21 +1163,59 @@
{ 'command': 'query-blockstats', 'returns': ['BlockStats'] }
##
-# @VncClientInfo:
+# @NetworkAddressFamily
#
-# Information about a connected VNC client.
+# The network address family
+#
+# @ipv4: IPV4 family
+#
+# @ipv6: IPV6 family
+#
+# @unix: unix socket
+#
+# @unknown: otherwise
+#
+# Since: 2.1
+##
+{ 'enum': 'NetworkAddressFamily',
+ 'data': [ 'ipv4', 'ipv6', 'unix', 'unknown' ] }
+
+##
+# @VncBasicInfo
#
-# @host: The host name of the client. QEMU tries to resolve this to a DNS name
-# when possible.
+# The basic information for vnc network connection
#
-# @family: 'ipv6' if the client is connected via IPv6 and TCP
-# 'ipv4' if the client is connected via IPv4 and TCP
-# 'unix' if the client is connected via a unix domain socket
-# 'unknown' otherwise
+# @host: IP address
#
-# @service: The service name of the client's port. This may depends on the
-# host system's service database so symbolic names should not be
-# relied on.
+# @service: The service name of vnc port. This may depend on the host system's
+# service database so symbolic names should not be relied on.
+#
+# @family: address family
+#
+# Since: 2.1
+##
+{ 'type': 'VncBasicInfo',
+ 'data': { 'host': 'str',
+ 'service': 'str',
+ 'family': 'NetworkAddressFamily' } }
+
+##
+# @VncServerInfo
+#
+# The network connection information for server
+#
+# @auth: #optional, authentication method
+#
+# Since: 2.1
+##
+{ 'type': 'VncServerInfo',
+ 'base': 'VncBasicInfo',
+ 'data': { '*auth': 'str' } }
+
+##
+# @VncClientInfo:
+#
+# Information about a connected VNC client.
#
# @x509_dname: #optional If x509 authentication is in use, the Distinguished
# Name of the client.
@@ -1188,8 +1226,8 @@
# Since: 0.14.0
##
{ 'type': 'VncClientInfo',
- 'data': {'host': 'str', 'family': 'str', 'service': 'str',
- '*x509_dname': 'str', '*sasl_username': 'str'} }
+ 'base': 'VncBasicInfo',
+ 'data': { '*x509_dname' : 'str', '*sasl_username': 'str' } }
##
# @VncInfo:
@@ -1228,7 +1266,8 @@
# Since: 0.14.0
##
{ 'type': 'VncInfo',
- 'data': {'enabled': 'bool', '*host': 'str', '*family': 'str',
+ 'data': {'enabled': 'bool', '*host': 'str',
+ '*family': 'NetworkAddressFamily',
'*service': 'str', '*auth': 'str', '*clients': ['VncClientInfo']} }
##
@@ -1243,19 +1282,40 @@
{ 'command': 'query-vnc', 'returns': 'VncInfo' }
##
-# @SpiceChannel
+# @SpiceBasicInfo
#
-# Information about a SPICE client channel.
+# The basic information for SPICE network connection
+#
+# @host: IP address
+#
+# @port: port number
#
-# @host: The host name of the client. QEMU tries to resolve this to a DNS name
-# when possible.
+# @family: address family
#
-# @family: 'ipv6' if the client is connected via IPv6 and TCP
-# 'ipv4' if the client is connected via IPv4 and TCP
-# 'unix' if the client is connected via a unix domain socket
-# 'unknown' otherwise
+# Since: 2.1
+##
+{ 'type': 'SpiceBasicInfo',
+ 'data': { 'host': 'str',
+ 'port': 'str',
+ 'family': 'NetworkAddressFamily' } }
+
+##
+# @SpiceServerInfo
+#
+# Information about a SPICE server
+#
+# @auth: #optional, authentication method
+#
+# Since: 2.1
+##
+{ 'type': 'SpiceServerInfo',
+ 'base': 'SpiceBasicInfo',
+ 'data': { '*auth': 'str' } }
+
+##
+# @SpiceChannel
#
-# @port: The client's port number.
+# Information about a SPICE client channel.
#
# @connection-id: SPICE connection id number. All channels with the same id
# belong to the same SPICE session.
@@ -1273,8 +1333,8 @@
# Since: 0.14.0
##
{ 'type': 'SpiceChannel',
- 'data': {'host': 'str', 'family': 'str', 'port': 'str',
- 'connection-id': 'int', 'channel-type': 'int', 'channel-id': 'int',
+ 'base': 'SpiceBasicInfo',
+ 'data': {'connection-id': 'int', 'channel-type': 'int', 'channel-id': 'int',
'tls': 'bool'} }
##
@@ -4722,3 +4782,19 @@
'btn' : 'InputBtnEvent',
'rel' : 'InputMoveEvent',
'abs' : 'InputMoveEvent' } }
+
+##
+# @BlockErrorAction
+#
+# An enumeration of action that has been taken when a DISK I/O occurs
+#
+# @ignore: error has been ignored
+#
+# @report: error has been reported to the device
+#
+# @stop: error caused VM to be stopped
+#
+# Since: 2.1
+##
+{ 'enum': 'BlockErrorAction',
+ 'data': [ 'ignore', 'report', 'stop' ] }
diff --git a/ui/spice-core.c b/ui/spice-core.c
index d10818a..8d54fb3 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -391,15 +391,16 @@ static SpiceChannelList *qmp_query_spice_channels(void)
chan = g_malloc0(sizeof(*chan));
chan->value = g_malloc0(sizeof(*chan->value));
+ chan->value->base = g_malloc0(sizeof(*chan->value->base));
paddr = (struct sockaddr *)&item->info->paddr_ext;
plen = item->info->plen_ext;
getnameinfo(paddr, plen,
host, sizeof(host), port, sizeof(port),
NI_NUMERICHOST | NI_NUMERICSERV);
- chan->value->host = g_strdup(host);
- chan->value->port = g_strdup(port);
- chan->value->family = g_strdup(inet_strfamily(paddr->sa_family));
+ chan->value->base->host = g_strdup(host);
+ chan->value->base->port = g_strdup(port);
+ chan->value->base->family = inet_netfamily(paddr->sa_family);
chan->value->connection_id = item->info->connection_id;
chan->value->channel_type = item->info->type;
diff --git a/ui/vnc.c b/ui/vnc.c
index 61b1f93..469852a 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -321,9 +321,10 @@ static VncClientInfo *qmp_query_vnc_client(const VncState *client)
}
info = g_malloc0(sizeof(*info));
- info->host = g_strdup(host);
- info->service = g_strdup(serv);
- info->family = g_strdup(inet_strfamily(sa.ss_family));
+ info->base = g_malloc0(sizeof(*info->base));
+ info->base->host = g_strdup(host);
+ info->base->service = g_strdup(serv);
+ info->base->family = inet_netfamily(sa.ss_family);
#ifdef CONFIG_VNC_TLS
if (client->tls.session && client->tls.dname) {
@@ -398,7 +399,7 @@ VncInfo *qmp_query_vnc(Error **errp)
info->service = g_strdup(serv);
info->has_family = true;
- info->family = g_strdup(inet_strfamily(sa.ss_family));
+ info->family = inet_netfamily(sa.ss_family);
info->has_auth = true;
info->auth = g_strdup(vnc_auth_name(vnc_display));
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 627e609..7f558bb 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -104,6 +104,16 @@ const char *inet_strfamily(int family)
return "unknown";
}
+NetworkAddressFamily inet_netfamily(int family)
+{
+ switch (family) {
+ case PF_INET6: return NETWORK_ADDRESS_FAMILY_IPV6;
+ case PF_INET: return NETWORK_ADDRESS_FAMILY_IPV4;
+ case PF_UNIX: return NETWORK_ADDRESS_FAMILY_UNIX;
+ }
+ return NETWORK_ADDRESS_FAMILY_UNKNOWN;
+}
+
int inet_listen_opts(QemuOpts *opts, int port_offset, Error **errp)
{
struct addrinfo ai,*res,*e;
--
1.7.1
^ permalink raw reply related [flat|nested] 77+ messages in thread* Re: [Qemu-devel] [PATCH V6 05/29] qapi: adjust existing defines
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 05/29] qapi: adjust existing defines Wenchao Xia
@ 2014-06-13 17:32 ` Eric Blake
0 siblings, 0 replies; 77+ messages in thread
From: Eric Blake @ 2014-06-13 17:32 UTC (permalink / raw)
To: Wenchao Xia, qemu-devel; +Cc: mdroth, armbru, lcapitulino
[-- Attachment #1: Type: text/plain, Size: 6067 bytes --]
On 06/05/2014 06:22 AM, Wenchao Xia wrote:
> In order to let event defines use existing types later, instead of
> redefine new ones, some old type defines for spice and vnc are changed,
> and BlockErrorAction is moved from block.h to qapi schema. Note that
> BlockErrorAction is not merged with BlockdevOnError.
If you were to respin this, I'd split it into two - one part moving
BlockErrorAction from block.h into the schema, and the other tweaking
spice and vnc schema members. But at this point, I'm more interested in
getting it into the tree than worrying about another round of delays for
a respin.
>
> One thing not perfect is that, VncInfo should be foldered but may break
s/that,/that/
"foldered" is not a word, but off-hand I have no idea what you meant. [1]
> API stability.
>
> Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
> ---
> block.c | 17 ++++---
> block/backup.c | 2 +-
> block/mirror.c | 7 ++-
> block/stream.c | 4 +-
> blockjob.c | 11 ++--
> hmp.c | 5 +-
> hw/block/virtio-blk.c | 6 +-
> hw/ide/core.c | 6 +-
> hw/scsi/scsi-disk.c | 6 +-
> include/block/block.h | 4 --
> include/qemu/sockets.h | 2 +
> qapi-schema.json | 126 ++++++++++++++++++++++++++++++++++++++----------
Of course, some of this is in qapi/block-core.json now; but Paolo's
rebased series picked up on that.
> ui/spice-core.c | 7 ++-
> ui/vnc.c | 9 ++--
> util/qemu-sockets.c | 10 ++++
> 15 files changed, 156 insertions(+), 66 deletions(-)
>
> +++ b/qapi-schema.json
> @@ -1163,21 +1163,59 @@
> { 'command': 'query-blockstats', 'returns': ['BlockStats'] }
>
> ##
> -# @VncClientInfo:
> +# @NetworkAddressFamily
> #
> -# Information about a connected VNC client.
> +# The network address family
> +#
> +# @ipv4: IPV4 family
> +#
> +# @ipv6: IPV6 family
> +#
> +# @unix: unix socket
> +#
> +# @unknown: otherwise
> +#
> +# Since: 2.1
> +##
> +{ 'enum': 'NetworkAddressFamily',
> + 'data': [ 'ipv4', 'ipv6', 'unix', 'unknown' ] }
Nice. Conversion of 'str' to 'NetworkAddressFamily' has no impact to the
on-the-wire format, but makes us have better type safety.
> +
> +##
> +# @VncBasicInfo
> #
> -# @host: The host name of the client. QEMU tries to resolve this to a DNS name
> -# when possible.
> +# The basic information for vnc network connection
> #
> -# @family: 'ipv6' if the client is connected via IPv6 and TCP
> -# 'ipv4' if the client is connected via IPv4 and TCP
> -# 'unix' if the client is connected via a unix domain socket
> -# 'unknown' otherwise
> +# @host: IP address
> #
> -# @service: The service name of the client's port. This may depends on the
> -# host system's service database so symbolic names should not be
> -# relied on.
> +# @service: The service name of vnc port. This may depend on the host system's
s/of/of the/
> +# service database so symbolic names should not be relied on.
> +#
> +# @family: address family
> +#
> +# Since: 2.1
> +##
> +{ 'type': 'VncBasicInfo',
> + 'data': { 'host': 'str',
> + 'service': 'str',
> + 'family': 'NetworkAddressFamily' } }
> +
> +##
> +# @VncServerInfo
> +#
> +# The network connection information for server
> +#
> +# @auth: #optional, authentication method
I wonder if this parameter should eventually be converted to an enum
rather than open-coded str, but that doesn't need to happen right away.
> +#
> +# Since: 2.1
> +##
> +{ 'type': 'VncServerInfo',
> + 'base': 'VncBasicInfo',
> + 'data': { '*auth': 'str' } }
> +
> +##
> +# @VncClientInfo:
> +#
> +# Information about a connected VNC client.
> #
> # @x509_dname: #optional If x509 authentication is in use, the Distinguished
> # Name of the client.
> @@ -1188,8 +1226,8 @@
> # Since: 0.14.0
> ##
> { 'type': 'VncClientInfo',
> - 'data': {'host': 'str', 'family': 'str', 'service': 'str',
> - '*x509_dname': 'str', '*sasl_username': 'str'} }
> + 'base': 'VncBasicInfo',
> + 'data': { '*x509_dname' : 'str', '*sasl_username': 'str' } }
Spurious addition of spaces; s/ :/:/ in your cleanup patch.
>
> ##
> # @VncInfo:
> @@ -1228,7 +1266,8 @@
> # Since: 0.14.0
> ##
> { 'type': 'VncInfo',
> - 'data': {'enabled': 'bool', '*host': 'str', '*family': 'str',
> + 'data': {'enabled': 'bool', '*host': 'str',
> + '*family': 'NetworkAddressFamily',
> '*service': 'str', '*auth': 'str', '*clients': ['VncClientInfo']} }
[1] Okay, I think I see what you meant in your commit message. You did
NOT convert 'VncInfo' to use 'VncBasicInfo' as a base class, because you
were worried about whether it would break things. And your worry is
justified - VncBasicInfo marks 'host', 'family', and 'service' as
manadatory, while 'VncInfo' marks them as optional.
If VncInfo is only used on the output side of commands (which indeed
appears to be the case - it is only used as the return of 'query-vnc'),
then converting from optional to mandatory is fine from the
backwards-compatibility aspect; the only question is whether the
existing code for query-vnc has sane defaults for those three fields in
the case where it was previously omitting them. If you choose to make
that change, do it as a followup patch. This patch is fine. And now
that I understand your concern, I'd rewrite that paragraph in the commit
message to this (and either Paolo can rebase it back into his github
tree, or whoever applies the patches can make the tweak when adding my R-b):
At this point, VncInfo is not made a child of VncBasicInfo, due to the
difference in mandatory vs. optional parameters.
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: 604 bytes --]
^ permalink raw reply [flat|nested] 77+ messages in thread
* [Qemu-devel] [PATCH V6 06/29] monitor: add an implemention as qapi event emit method
2014-06-05 12:21 [Qemu-devel] [PATCH V6 00/29] add direct support of event in qapi schema Wenchao Xia
` (4 preceding siblings ...)
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 05/29] qapi: adjust existing defines Wenchao Xia
@ 2014-06-05 12:22 ` Wenchao Xia
2014-06-13 19:04 ` Eric Blake
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 07/29] qapi: add new schema file qapi-event.json Wenchao Xia
` (23 subsequent siblings)
29 siblings, 1 reply; 77+ messages in thread
From: Wenchao Xia @ 2014-06-05 12:22 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, armbru, Wenchao Xia, lcapitulino
Now monitor has been hooked on the new event mechanism, so the patches
later can convert event callers one by one. Most code are copied from
old monitor_protocol_* functions with some modification.
Note that two build time warnings will be raised after this patch. One is
caused by no caller of monitor_qapi_event_throttle(), the other one is
caused by QAPI_EVENT_MAX = 0. They will be fixed automatically after
full event conversion later.
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
---
monitor.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
trace-events | 4 ++
2 files changed, 131 insertions(+), 1 deletions(-)
diff --git a/monitor.c b/monitor.c
index 593679a..e122381 100644
--- a/monitor.c
+++ b/monitor.c
@@ -69,6 +69,8 @@
#include "qmp-commands.h"
#include "hmp.h"
#include "qemu/thread.h"
+#include "qapi/qmp-event.h"
+#include "qapi-event.h"
/* for pic/irq_info */
#if defined(TARGET_SPARC)
@@ -185,6 +187,14 @@ typedef struct MonitorEventState {
QObject *data; /* Event pending delayed dispatch */
} MonitorEventState;
+typedef struct MonitorQAPIEventState {
+ QAPIEvent event; /* Event being tracked */
+ int64_t rate; /* Period over which to throttle. 0 to disable */
+ int64_t last; /* Time at which event was last emitted */
+ QEMUTimer *timer; /* Timer for handling delayed events */
+ QObject *data; /* Event pending delayed dispatch */
+} MonitorQAPIEventState;
+
struct Monitor {
CharDriverState *chr;
int mux_out;
@@ -489,6 +499,122 @@ static const char *monitor_event_names[] = {
QEMU_BUILD_BUG_ON(ARRAY_SIZE(monitor_event_names) != QEVENT_MAX)
static MonitorEventState monitor_event_state[QEVENT_MAX];
+static MonitorQAPIEventState monitor_qapi_event_state[QAPI_EVENT_MAX];
+
+/*
+ * Emits the event to every monitor instance, @event is only used for trace
+ */
+static void monitor_qapi_event_emit(QAPIEvent event, QObject *data)
+{
+ Monitor *mon;
+
+ trace_monitor_qapi_event_emit(event, data);
+ QLIST_FOREACH(mon, &mon_list, entry) {
+ if (monitor_ctrl_mode(mon) && qmp_cmd_mode(mon)) {
+ monitor_json_emitter(mon, data);
+ }
+ }
+}
+
+/*
+ * Queue a new event for emission to Monitor instances,
+ * applying any rate limiting if required.
+ */
+static void
+monitor_qapi_event_queue(int event_kind, QDict *data, Error **errp)
+{
+ MonitorQAPIEventState *evstate;
+ assert(event_kind < QAPI_EVENT_MAX);
+ QAPIEvent event = (QAPIEvent)event_kind;
+ int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
+
+ evstate = &(monitor_qapi_event_state[event]);
+ trace_monitor_qapi_event_queue(event,
+ data,
+ evstate->rate,
+ evstate->last,
+ now);
+
+ /* Rate limit of 0 indicates no throttling */
+ if (!evstate->rate) {
+ monitor_qapi_event_emit(event, QOBJECT(data));
+ evstate->last = now;
+ } else {
+ int64_t delta = now - evstate->last;
+ if (evstate->data ||
+ delta < evstate->rate) {
+ /* If there's an existing event pending, replace
+ * it with the new event, otherwise schedule a
+ * timer for delayed emission
+ */
+ if (evstate->data) {
+ qobject_decref(evstate->data);
+ } else {
+ int64_t then = evstate->last + evstate->rate;
+ timer_mod_ns(evstate->timer, then);
+ }
+ evstate->data = QOBJECT(data);
+ qobject_incref(evstate->data);
+ } else {
+ monitor_qapi_event_emit(event, QOBJECT(data));
+ evstate->last = now;
+ }
+ }
+}
+
+/*
+ * The callback invoked by QemuTimer when a delayed
+ * event is ready to be emitted
+ */
+static void monitor_qapi_event_handler(void *opaque)
+{
+ MonitorQAPIEventState *evstate = opaque;
+ int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
+
+
+ trace_monitor_qapi_event_handler(evstate->event,
+ evstate->data,
+ evstate->last,
+ now);
+ if (evstate->data) {
+ monitor_qapi_event_emit(evstate->event, evstate->data);
+ qobject_decref(evstate->data);
+ evstate->data = NULL;
+ }
+ evstate->last = now;
+}
+
+/*
+ * @event: the event ID to be limited
+ * @rate: the rate limit in milliseconds
+ *
+ * Sets a rate limit on a particular event, so no
+ * more than 1 event will be emitted within @rate
+ * milliseconds
+ */
+static void monitor_qapi_event_throttle(QAPIEvent event, int64_t rate)
+{
+ MonitorQAPIEventState *evstate;
+ assert(event < QAPI_EVENT_MAX);
+
+ evstate = &(monitor_qapi_event_state[event]);
+
+ trace_monitor_qapi_event_throttle(event, rate);
+ evstate->event = event;
+ evstate->rate = rate * SCALE_MS;
+ evstate->last = 0;
+ evstate->data = NULL;
+ evstate->timer = timer_new(QEMU_CLOCK_REALTIME,
+ SCALE_MS,
+ monitor_qapi_event_handler,
+ evstate);
+}
+
+static void monitor_qapi_event_init(void)
+{
+ qmp_event_set_func_emit(monitor_qapi_event_queue);
+}
+
/*
* Emits the event to every monitor instance
@@ -507,7 +633,6 @@ monitor_protocol_event_emit(MonitorEvent event,
}
}
-
/*
* Queue a new event for emission to Monitor instances,
* applying any rate limiting if required.
@@ -5169,6 +5294,7 @@ void monitor_init(CharDriverState *chr, int flags)
if (is_first_init) {
monitor_protocol_event_init();
+ monitor_qapi_event_init();
sortcmdlist();
is_first_init = 0;
}
diff --git a/trace-events b/trace-events
index ffe6e62..f4b8dbb 100644
--- a/trace-events
+++ b/trace-events
@@ -932,6 +932,10 @@ monitor_protocol_event_handler(uint32_t event, void *data, uint64_t last, uint64
monitor_protocol_event_emit(uint32_t event, void *data) "event=%d data=%p"
monitor_protocol_event_queue(uint32_t event, void *data, uint64_t rate, uint64_t last, uint64_t now) "event=%d data=%p rate=%" PRId64 " last=%" PRId64 " now=%" PRId64
monitor_protocol_event_throttle(uint32_t event, uint64_t rate) "event=%d rate=%" PRId64
+monitor_qapi_event_emit(uint32_t event, void *data) "event=%d data=%p"
+monitor_qapi_event_queue(uint32_t event, void *data, uint64_t rate, uint64_t last, uint64_t now) "event=%d data=%p rate=%" PRId64 " last=%" PRId64 " now=%" PRId64
+monitor_qapi_event_handler(uint32_t event, void *data, uint64_t last, uint64_t now) "event=%d data=%p last=%" PRId64 " now=%" PRId64
+monitor_qapi_event_throttle(uint32_t event, uint64_t rate) "event=%d rate=%" PRId64
# hw/net/opencores_eth.c
open_eth_mii_write(unsigned idx, uint16_t v) "MII[%02x] <- %04x"
--
1.7.1
^ permalink raw reply related [flat|nested] 77+ messages in thread* Re: [Qemu-devel] [PATCH V6 06/29] monitor: add an implemention as qapi event emit method
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 06/29] monitor: add an implemention as qapi event emit method Wenchao Xia
@ 2014-06-13 19:04 ` Eric Blake
2014-06-15 0:27 ` Wenchao Xia
0 siblings, 1 reply; 77+ messages in thread
From: Eric Blake @ 2014-06-13 19:04 UTC (permalink / raw)
To: Wenchao Xia, qemu-devel; +Cc: mdroth, armbru, lcapitulino
[-- Attachment #1: Type: text/plain, Size: 6774 bytes --]
On 06/05/2014 06:22 AM, Wenchao Xia wrote:
In the subject: s/as/of/
> Now monitor has been hooked on the new event mechanism, so the patches
s/Now/The/
> later can convert event callers one by one. Most code are copied from
s/the patches later/that later patches/
s/are/is/
> old monitor_protocol_* functions with some modification.
>
> Note that two build time warnings will be raised after this patch. One is
> caused by no caller of monitor_qapi_event_throttle(), the other one is
> caused by QAPI_EVENT_MAX = 0. They will be fixed automatically after
> full event conversion later.
I only got one of those two warnings, about the unused function. What
compiler and options are you using to get a warning about
QAPI_EVENT_MAX?. Furthermore, since the default 'configure' turns
warnings into errors, this would be a build-breaker that hurts 'git
bisect'. I think it's easy enough to avoid, if you would please squash
this in:
diff --git i/monitor.c w/monitor.c
index 952e1cb..a593d17 100644
--- i/monitor.c
+++ w/monitor.c
@@ -594,6 +594,7 @@ static void monitor_qapi_event_handler(void *opaque)
* more than 1 event will be emitted within @rate
* milliseconds
*/
+__attribute__((unused)) /* FIXME remove once in use */
static void monitor_qapi_event_throttle(QAPIEvent event, int64_t rate)
{
MonitorQAPIEventState *evstate;
You can then remove that attribute later in 29/29 when you delete
everything else about the older implementation.
At this point, I think we're racking up enough fixes to be worth a v7
respin (particularly since avoiding 'git bisect' breakage cannot be done
as a followup patch, but has to be done in-place in the series).
>
> Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
> ---
> monitor.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> trace-events | 4 ++
> 2 files changed, 131 insertions(+), 1 deletions(-)
>
>
> +typedef struct MonitorQAPIEventState {
> + QAPIEvent event; /* Event being tracked */
> + int64_t rate; /* Period over which to throttle. 0 to disable */
> + int64_t last; /* Time at which event was last emitted */
in what unit? [1]...
> + QEMUTimer *timer; /* Timer for handling delayed events */
> + QObject *data; /* Event pending delayed dispatch */
Any reason the comments aren't aligned?
> +} MonitorQAPIEventState;
> +
> struct Monitor {
> CharDriverState *chr;
> int mux_out;
> @@ -489,6 +499,122 @@ static const char *monitor_event_names[] = {
> QEMU_BUILD_BUG_ON(ARRAY_SIZE(monitor_event_names) != QEVENT_MAX)
>
> static MonitorEventState monitor_event_state[QEVENT_MAX];
> +static MonitorQAPIEventState monitor_qapi_event_state[QAPI_EVENT_MAX];
If you are still seeing a compiler warning about allocating an array of
size 0, you could likewise try this hack:
/* FIXME Hack a minimum array size until we have events */
static MonitorQAPIEventState monitor_qapi_event_state[QAPI_EVENT_MAX +
!QAPI_EVENT_MAX];
and likewise clean that up in 29/29
> +static void
> +monitor_qapi_event_queue(int event_kind, QDict *data, Error **errp)
This is not the usual formatting in qemu.
> +{
> + MonitorQAPIEventState *evstate;
> + assert(event_kind < QAPI_EVENT_MAX);
This doesn't filter out negative values for event_kind. Is it worth the
extra paranoia to either make event_kind unsigned, or to assert that
event_kind >= 0?
> + QAPIEvent event = (QAPIEvent)event_kind;
Why are we casting here? Would it not make more sense to change the
signature in patch 2/29 to use the enum type from the get-go?
typedef void (*QMPEventFuncEmit)(QAPIEvent event_kind, QDict *dict,
Error **errp);
and adjust the signature of this function to match
> + int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
...[1] okay, it looks like 'rate' and 'last' are specified in
nanoseconds. Worth documenting in their declaration above.
Particularly since [1]...
> +static void monitor_qapi_event_handler(void *opaque)
> +{
> + MonitorQAPIEventState *evstate = opaque;
> + int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
> +
> +
> + trace_monitor_qapi_event_handler(evstate->event,
Why the double blank line?
> +
> +/*
> + * @event: the event ID to be limited
> + * @rate: the rate limit in milliseconds
> + *
> + * Sets a rate limit on a particular event, so no
> + * more than 1 event will be emitted within @rate
> + * milliseconds
> + */
> +static void monitor_qapi_event_throttle(QAPIEvent event, int64_t rate)
...[1] this function is documented in milliseconds, not nanoseconds, and
you are scaling internally.
> +{
> + MonitorQAPIEventState *evstate;
> + assert(event < QAPI_EVENT_MAX);
> +
> + evstate = &(monitor_qapi_event_state[event]);
> +
> + trace_monitor_qapi_event_throttle(event, rate);
> + evstate->event = event;
> + evstate->rate = rate * SCALE_MS;
This value can overflow if a user passes in an insanely large rate. Do
we care?
> + evstate->last = 0;
> + evstate->data = NULL;
> + evstate->timer = timer_new(QEMU_CLOCK_REALTIME,
> + SCALE_MS,
> + monitor_qapi_event_handler,
> + evstate);
> @@ -507,7 +633,6 @@ monitor_protocol_event_emit(MonitorEvent event,
> }
> }
>
> -
> /*
Why the spurious line deletion?
> +++ b/trace-events
> @@ -932,6 +932,10 @@ monitor_protocol_event_handler(uint32_t event, void *data, uint64_t last, uint64
> monitor_protocol_event_emit(uint32_t event, void *data) "event=%d data=%p"
> monitor_protocol_event_queue(uint32_t event, void *data, uint64_t rate, uint64_t last, uint64_t now) "event=%d data=%p rate=%" PRId64 " last=%" PRId64 " now=%" PRId64
> monitor_protocol_event_throttle(uint32_t event, uint64_t rate) "event=%d rate=%" PRId64
> +monitor_qapi_event_emit(uint32_t event, void *data) "event=%d data=%p"
> +monitor_qapi_event_queue(uint32_t event, void *data, uint64_t rate, uint64_t last, uint64_t now) "event=%d data=%p rate=%" PRId64 " last=%" PRId64 " now=%" PRId64
> +monitor_qapi_event_handler(uint32_t event, void *data, uint64_t last, uint64_t now) "event=%d data=%p last=%" PRId64 " now=%" PRId64
> +monitor_qapi_event_throttle(uint32_t event, uint64_t rate) "event=%d rate=%" PRId64
Any reason you wanted to invent four new trace names, even though the
existing 4 traces have the same signatures? I'm wondering whether it
would have just been better to use the old trace names.
--
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: 604 bytes --]
^ permalink raw reply related [flat|nested] 77+ messages in thread* Re: [Qemu-devel] [PATCH V6 06/29] monitor: add an implemention as qapi event emit method
2014-06-13 19:04 ` Eric Blake
@ 2014-06-15 0:27 ` Wenchao Xia
0 siblings, 0 replies; 77+ messages in thread
From: Wenchao Xia @ 2014-06-15 0:27 UTC (permalink / raw)
To: Eric Blake, qemu-devel; +Cc: mdroth, armbru, lcapitulino
于 2014/6/14 3:04, Eric Blake 写道:
> On 06/05/2014 06:22 AM, Wenchao Xia wrote:
>
> In the subject: s/as/of/
>
>> Now monitor has been hooked on the new event mechanism, so the patches
>
> s/Now/The/
>
>> later can convert event callers one by one. Most code are copied from
>
> s/the patches later/that later patches/
>
> s/are/is/
>
>> old monitor_protocol_* functions with some modification.
>>
>> Note that two build time warnings will be raised after this patch. One is
>> caused by no caller of monitor_qapi_event_throttle(), the other one is
>> caused by QAPI_EVENT_MAX = 0. They will be fixed automatically after
>> full event conversion later.
>
> I only got one of those two warnings, about the unused function. What
> compiler and options are you using to get a warning about
> QAPI_EVENT_MAX?. Furthermore, since the default 'configure' turns
> warnings into errors, this would be a build-breaker that hurts 'git
> bisect'. I think it's easy enough to avoid, if you would please squash
> this in:
>
The QAPI_EVENT_MAX = 0 cause a warning for code:
monitor_qapi_event_throttle(QAPIEvent event, int64_t rate)
{
....
assert(event < QAPI_EVENT_MAX);
}
which is always false now. Perhaps change it as
assert((int)event < QAPI_EVENT_MAX);
?
> diff --git i/monitor.c w/monitor.c
> index 952e1cb..a593d17 100644
> --- i/monitor.c
> +++ w/monitor.c
> @@ -594,6 +594,7 @@ static void monitor_qapi_event_handler(void *opaque)
> * more than 1 event will be emitted within @rate
> * milliseconds
> */
> +__attribute__((unused)) /* FIXME remove once in use */
> static void monitor_qapi_event_throttle(QAPIEvent event, int64_t rate)
> {
> MonitorQAPIEventState *evstate;
>
>
> You can then remove that attribute later in 29/29 when you delete
> everything else about the older implementation.
>
> At this point, I think we're racking up enough fixes to be worth a v7
> respin (particularly since avoiding 'git bisect' breakage cannot be done
> as a followup patch, but has to be done in-place in the series).
>
Thanks for tipping, it is a good way to go.
>>
>> Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
>> ---
>> monitor.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>> trace-events | 4 ++
>> 2 files changed, 131 insertions(+), 1 deletions(-)
>>
>
>>
>> +typedef struct MonitorQAPIEventState {
>> + QAPIEvent event; /* Event being tracked */
>> + int64_t rate; /* Period over which to throttle. 0 to disable */
>> + int64_t last; /* Time at which event was last emitted */
>
> in what unit? [1]...
>
>> + QEMUTimer *timer; /* Timer for handling delayed events */
>> + QObject *data; /* Event pending delayed dispatch */
>
> Any reason the comments aren't aligned?
Will fix.
>
>> +} MonitorQAPIEventState;
>> +
>> struct Monitor {
>> CharDriverState *chr;
>> int mux_out;
>> @@ -489,6 +499,122 @@ static const char *monitor_event_names[] = {
>> QEMU_BUILD_BUG_ON(ARRAY_SIZE(monitor_event_names) != QEVENT_MAX)
>>
>> static MonitorEventState monitor_event_state[QEVENT_MAX];
>> +static MonitorQAPIEventState monitor_qapi_event_state[QAPI_EVENT_MAX];
>
> If you are still seeing a compiler warning about allocating an array of
> size 0, you could likewise try this hack:
>
> /* FIXME Hack a minimum array size until we have events */
> static MonitorQAPIEventState monitor_qapi_event_state[QAPI_EVENT_MAX +
> !QAPI_EVENT_MAX];
>
> and likewise clean that up in 29/29
>
>> +static void
>> +monitor_qapi_event_queue(int event_kind, QDict *data, Error **errp)
>
> This is not the usual formatting in qemu.
Fixing it.
>
>> +{
>> + MonitorQAPIEventState *evstate;
>> + assert(event_kind < QAPI_EVENT_MAX);
>
> This doesn't filter out negative values for event_kind. Is it worth the
> extra paranoia to either make event_kind unsigned, or to assert that
> event_kind >= 0?
>
Build waring will be raised for QAPI_EVENT_MAX = 0, same as above,
any better way to solve?
>> + QAPIEvent event = (QAPIEvent)event_kind;
>
> Why are we casting here? Would it not make more sense to change the
> signature in patch 2/29 to use the enum type from the get-go?
>
> typedef void (*QMPEventFuncEmit)(QAPIEvent event_kind, QDict *dict,
> Error **errp);
>
> and adjust the signature of this function to match
>
Since QAPIEvent is generated by qapi-event.py, and they are
different in qemu code and test code, so there will be conflict
in test code which include qmp-event.h:
in qemu:
qmp-event.h include qapi-event.h
in test:
qmp-event.h include test-qapi-event.h
For simple I just used int type instead of QAPIEvent type.
To use QAPIEvent in declaration, I think there would be some
tricks in test, and doc that using qapi-event.h define in
qmp-event.c may break test code, the encapsulation is not very goood.
>> + int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
>
> ...[1] okay, it looks like 'rate' and 'last' are specified in
> nanoseconds. Worth documenting in their declaration above.
> Particularly since [1]...
>
>> +static void monitor_qapi_event_handler(void *opaque)
>> +{
>> + MonitorQAPIEventState *evstate = opaque;
>> + int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
>> +
>> +
>> + trace_monitor_qapi_event_handler(evstate->event,
>
> Why the double blank line?
>
Copied code, will fix.
>> +
>> +/*
>> + * @event: the event ID to be limited
>> + * @rate: the rate limit in milliseconds
>> + *
>> + * Sets a rate limit on a particular event, so no
>> + * more than 1 event will be emitted within @rate
>> + * milliseconds
>> + */
>> +static void monitor_qapi_event_throttle(QAPIEvent event, int64_t rate)
>
> ...[1] this function is documented in milliseconds, not nanoseconds, and
> you are scaling internally.
>
>> +{
>> + MonitorQAPIEventState *evstate;
>> + assert(event < QAPI_EVENT_MAX);
>> +
>> + evstate = &(monitor_qapi_event_state[event]);
>> +
>> + trace_monitor_qapi_event_throttle(event, rate);
>> + evstate->event = event;
>> + evstate->rate = rate * SCALE_MS;
>
> This value can overflow if a user passes in an insanely large rate. Do
> we care?
>
There is not much existing caller, maybe we can change it as
nonoseconds.
>> + evstate->last = 0;
>> + evstate->data = NULL;
>> + evstate->timer = timer_new(QEMU_CLOCK_REALTIME,
>> + SCALE_MS,
>> + monitor_qapi_event_handler,
>> + evstate);
>
>> @@ -507,7 +633,6 @@ monitor_protocol_event_emit(MonitorEvent event,
>> }
>> }
>>
>> -
>> /*
>
> Why the spurious line deletion?
>
Will remove.
>> +++ b/trace-events
>> @@ -932,6 +932,10 @@ monitor_protocol_event_handler(uint32_t event, void *data, uint64_t last, uint64
>> monitor_protocol_event_emit(uint32_t event, void *data) "event=%d data=%p"
>> monitor_protocol_event_queue(uint32_t event, void *data, uint64_t rate, uint64_t last, uint64_t now) "event=%d data=%p rate=%" PRId64 " last=%" PRId64 " now=%" PRId64
>> monitor_protocol_event_throttle(uint32_t event, uint64_t rate) "event=%d rate=%" PRId64
>> +monitor_qapi_event_emit(uint32_t event, void *data) "event=%d data=%p"
>> +monitor_qapi_event_queue(uint32_t event, void *data, uint64_t rate, uint64_t last, uint64_t now) "event=%d data=%p rate=%" PRId64 " last=%" PRId64 " now=%" PRId64
>> +monitor_qapi_event_handler(uint32_t event, void *data, uint64_t last, uint64_t now) "event=%d data=%p last=%" PRId64 " now=%" PRId64
>> +monitor_qapi_event_throttle(uint32_t event, uint64_t rate) "event=%d rate=%" PRId64
>
> Any reason you wanted to invent four new trace names, even though the
> existing 4 traces have the same signatures? I'm wondering whether it
> would have just been better to use the old trace names.
>
The old trace functions are still in use of old event code,
so I added fource new ones. The old ones should be removed in
cleanup patch.
^ permalink raw reply [flat|nested] 77+ messages in thread
* [Qemu-devel] [PATCH V6 07/29] qapi: add new schema file qapi-event.json
2014-06-05 12:21 [Qemu-devel] [PATCH V6 00/29] add direct support of event in qapi schema Wenchao Xia
` (5 preceding siblings ...)
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 06/29] monitor: add an implemention as qapi event emit method Wenchao Xia
@ 2014-06-05 12:22 ` Wenchao Xia
2014-06-13 19:25 ` Eric Blake
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 08/29] qapi event: convert SHUTDOWN Wenchao Xia
` (22 subsequent siblings)
29 siblings, 1 reply; 77+ messages in thread
From: Wenchao Xia @ 2014-06-05 12:22 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, armbru, Wenchao Xia, lcapitulino
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
---
Makefile | 12 ++++++++----
qapi-schema.json | 2 ++
2 files changed, 10 insertions(+), 4 deletions(-)
create mode 100644 qapi-event.json
diff --git a/Makefile b/Makefile
index 237657e..554fb2d 100644
--- a/Makefile
+++ b/Makefile
@@ -247,22 +247,26 @@ $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
" GEN $@")
qapi-types.c qapi-types.h :\
-$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
+$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/qapi-event.json \
+$(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \
$(gen-out-type) -o "." -b -i $<, \
" GEN $@")
qapi-visit.c qapi-visit.h :\
-$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
+$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/qapi-event.json \
+$(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \
$(gen-out-type) -o "." -b -i $<, \
" GEN $@")
qapi-event.c qapi-event.h :\
-$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-event.py $(qapi-py)
+$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/qapi-event.json \
+$(SRC_PATH)/scripts/qapi-event.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-event.py \
$(gen-out-type) -o "." -b -i $<, \
" GEN $@")
qmp-commands.h qmp-marshal.c :\
-$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
+$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/qapi-event.json \
+$(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \
$(gen-out-type) -o "." -m -i $<, \
" GEN $@")
diff --git a/qapi-event.json b/qapi-event.json
new file mode 100644
index 0000000..e69de29
diff --git a/qapi-schema.json b/qapi-schema.json
index 115d8d0..d04514a 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4798,3 +4798,5 @@
##
{ 'enum': 'BlockErrorAction',
'data': [ 'ignore', 'report', 'stop' ] }
+
+{ 'include': 'qapi-event.json' }
--
1.7.1
^ permalink raw reply related [flat|nested] 77+ messages in thread* Re: [Qemu-devel] [PATCH V6 07/29] qapi: add new schema file qapi-event.json
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 07/29] qapi: add new schema file qapi-event.json Wenchao Xia
@ 2014-06-13 19:25 ` Eric Blake
2014-06-13 19:45 ` Eric Blake
0 siblings, 1 reply; 77+ messages in thread
From: Eric Blake @ 2014-06-13 19:25 UTC (permalink / raw)
To: Wenchao Xia, qemu-devel; +Cc: mdroth, Max Reitz, armbru, lcapitulino
[-- Attachment #1: Type: text/plain, Size: 1628 bytes --]
On 06/05/2014 06:22 AM, Wenchao Xia wrote:
> Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
> ---
> Makefile | 12 ++++++++----
> qapi-schema.json | 2 ++
> 2 files changed, 10 insertions(+), 4 deletions(-)
> create mode 100644 qapi-event.json
>
> diff --git a/Makefile b/Makefile
> index 237657e..554fb2d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -247,22 +247,26 @@ $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
> " GEN $@")
>
> qapi-types.c qapi-types.h :\
> -$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
> +$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/qapi-event.json \
> +$(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
Please rebase this on top of Max's fix to add $(qapi-modules):
https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg01838.html
> diff --git a/qapi-event.json b/qapi-event.json
> new file mode 100644
> index 0000000..e69de29
Probably better to name this file qapi/event.json, to go alongside the
other sub-files we have already created.
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 115d8d0..d04514a 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -4798,3 +4798,5 @@
> ##
> { 'enum': 'BlockErrorAction',
> 'data': [ 'ignore', 'report', 'stop' ] }
> +
> +{ 'include': 'qapi-event.json' }
Rather than putting this at the bottom of the file, would it make sense
to put it at the top next to the other 'include' directives?
--
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: 604 bytes --]
^ permalink raw reply [flat|nested] 77+ messages in thread* Re: [Qemu-devel] [PATCH V6 07/29] qapi: add new schema file qapi-event.json
2014-06-13 19:25 ` Eric Blake
@ 2014-06-13 19:45 ` Eric Blake
0 siblings, 0 replies; 77+ messages in thread
From: Eric Blake @ 2014-06-13 19:45 UTC (permalink / raw)
To: Wenchao Xia, qemu-devel; +Cc: armbru, lcapitulino, mdroth, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 1126 bytes --]
On 06/13/2014 01:25 PM, Eric Blake wrote:
> On 06/05/2014 06:22 AM, Wenchao Xia wrote:
>> Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
>> ---
>> Makefile | 12 ++++++++----
>> qapi-schema.json | 2 ++
>> 2 files changed, 10 insertions(+), 4 deletions(-)
>> create mode 100644 qapi-event.json
>>
>> diff --git a/qapi-event.json b/qapi-event.json
>> new file mode 100644
>> index 0000000..e69de29
>
> Probably better to name this file qapi/event.json, to go alongside the
> other sub-files we have already created.
Also, I forgot to mention - adding an empty file to qemu.git is unusual.
When you respin, make the file non-empty by at least giving it a decent
comment at the top stating the purpose of the file. You could even add
in a copyright/license comment, if we had any agreement on what to use
(but that's already a different thread, since I already pointed out on
Benoit's work to split out several .json files that we don't have any
example to choose from).
--
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: 604 bytes --]
^ permalink raw reply [flat|nested] 77+ messages in thread
* [Qemu-devel] [PATCH V6 08/29] qapi event: convert SHUTDOWN
2014-06-05 12:21 [Qemu-devel] [PATCH V6 00/29] add direct support of event in qapi schema Wenchao Xia
` (6 preceding siblings ...)
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 07/29] qapi: add new schema file qapi-event.json Wenchao Xia
@ 2014-06-05 12:22 ` Wenchao Xia
2014-06-13 19:57 ` Eric Blake
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 09/29] qapi event: convert POWERDOWN Wenchao Xia
` (21 subsequent siblings)
29 siblings, 1 reply; 77+ messages in thread
From: Wenchao Xia @ 2014-06-05 12:22 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, armbru, Wenchao Xia, lcapitulino
This patch also eliminates build time warning caused by
QAPI_EVENT_MAX = 0.
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
---
docs/qmp/qmp-events.txt | 15 ---------------
qapi-event.json | 12 ++++++++++++
vl.c | 3 ++-
3 files changed, 14 insertions(+), 16 deletions(-)
diff --git a/docs/qmp/qmp-events.txt b/docs/qmp/qmp-events.txt
index 145402e..ff2f30d 100644
--- a/docs/qmp/qmp-events.txt
+++ b/docs/qmp/qmp-events.txt
@@ -304,21 +304,6 @@ Example:
"data": { "offset": 78 },
"timestamp": { "seconds": 1267020223, "microseconds": 435656 } }
-SHUTDOWN
---------
-
-Emitted when the Virtual Machine is powered down.
-
-Data: None.
-
-Example:
-
-{ "event": "SHUTDOWN",
- "timestamp": { "seconds": 1267040730, "microseconds": 682951 } }
-
-Note: If the command-line option "-no-shutdown" has been specified, a STOP
-event will eventually follow the SHUTDOWN event.
-
SPICE_CONNECTED, SPICE_DISCONNECTED
-----------------------------------
diff --git a/qapi-event.json b/qapi-event.json
index e69de29..b2a943f 100644
--- a/qapi-event.json
+++ b/qapi-event.json
@@ -0,0 +1,12 @@
+##
+# @SHUTDOWN
+#
+# Emitted when the virtual machine shutdown, qemu terminated the emulation and
+# is about to exit.
+#
+# Note: If the command-line option "-no-shutdown" has been specified, qemu will
+# not exit, and a STOP event will eventually follow the SHUTDOWN event
+#
+# Since: 2.1
+##
+{ 'event': 'SHUTDOWN' }
diff --git a/vl.c b/vl.c
index 0c15608..273d237 100644
--- a/vl.c
+++ b/vl.c
@@ -117,6 +117,7 @@ int main(int argc, char **argv)
#include "ui/qemu-spice.h"
#include "qapi/string-input-visitor.h"
#include "qom/object_interfaces.h"
+#include "qapi-event.h"
#define DEFAULT_RAM_SIZE 128
@@ -2028,7 +2029,7 @@ static bool main_loop_should_exit(void)
}
if (qemu_shutdown_requested()) {
qemu_kill_report();
- monitor_protocol_event(QEVENT_SHUTDOWN, NULL);
+ qapi_event_send_shutdown(NULL);
if (no_shutdown) {
vm_stop(RUN_STATE_SHUTDOWN);
} else {
--
1.7.1
^ permalink raw reply related [flat|nested] 77+ messages in thread* Re: [Qemu-devel] [PATCH V6 08/29] qapi event: convert SHUTDOWN
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 08/29] qapi event: convert SHUTDOWN Wenchao Xia
@ 2014-06-13 19:57 ` Eric Blake
2014-06-15 0:32 ` Wenchao Xia
0 siblings, 1 reply; 77+ messages in thread
From: Eric Blake @ 2014-06-13 19:57 UTC (permalink / raw)
To: Wenchao Xia, qemu-devel; +Cc: mdroth, armbru, lcapitulino
[-- Attachment #1: Type: text/plain, Size: 3112 bytes --]
On 06/05/2014 06:22 AM, Wenchao Xia wrote:
> This patch also eliminates build time warning caused by
> QAPI_EVENT_MAX = 0.
I still don't know why I wasn't seeing a warning for that, but agree
this cleans it up (or whichever event gets converted first, as there
aren't really any dependency restrictions in the order in which you do
conversions). If you do follow my suggestion of adding a workaround in
6/29 to avoid build warnings, then don't undo it here, but save it until
29/29; that way, no one individual conversion is doing more work than
any other.
>
> Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
> ---
> docs/qmp/qmp-events.txt | 15 ---------------
> qapi-event.json | 12 ++++++++++++
> vl.c | 3 ++-
> 3 files changed, 14 insertions(+), 16 deletions(-)
Yay - I finally have enough to see what code gets generated in 3/29.
The qapi-event.h header now has:
void qapi_event_send_shutdown(Error **errp);
extern const char *QAPIEvent_lookup[];
typedef enum QAPIEvent
{
QAPI_EVENT_SHUTDOWN = 0,
QAPI_EVENT_MAX = 1,
} QAPIEvent;
and the .c file has:
void qapi_event_send_shutdown(Error **errp)
{
QDict *qmp;
Error *local_err = NULL;
QMPEventFuncEmit emit;
emit = qmp_event_get_func_emit();
if (!emit) {
return;
}
qmp = qmp_event_build_dict("SHUTDOWN");
emit(QAPI_EVENT_SHUTDOWN, qmp, &local_err);
error_propagate(errp, local_err);
QDECREF(qmp);
}
Looks good, for a data-free event (I guess I'll have to wait till later
in the series to see what gets generated for an event with data). Hmm,
I wonder if patch 3/29 should have also modified docs/qapi-code-gen.txt
to show a sample generated function.
>
> -
> -Emitted when the Virtual Machine is powered down.
> +++ b/qapi-event.json
> @@ -0,0 +1,12 @@
> +##
> +# @SHUTDOWN
> +#
> +# Emitted when the virtual machine shutdown, qemu terminated the emulation and
> +# is about to exit.
Different wording than the text it replaces, and the grammar is a bit
unusual. Maybe:
Emitted when the virtual machine has shut down, indicating that qemu is
about to exit.
> if (qemu_shutdown_requested()) {
> qemu_kill_report();
> - monitor_protocol_event(QEVENT_SHUTDOWN, NULL);
> + qapi_event_send_shutdown(NULL);
Note that the two NULLs serve different purposes. In the old code, NULL
meant there was no additional data. In the new code, NULL indicates
that we are ignoring the possibility for error. I wonder if it would be
better to pass &error_abort instead of NULL? For that matter, I just
re-read 6/29 - the one implementation we have of an emit function
(monitor_qapi_event_queue) never sets errp. Is it better to just
consider events as best-effort, and completely ditch the errp parameter
from the emit callback typedef in 2/29, since it looks like you intend
to pass NULL for all clients of the callback?
--
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: 604 bytes --]
^ permalink raw reply [flat|nested] 77+ messages in thread* Re: [Qemu-devel] [PATCH V6 08/29] qapi event: convert SHUTDOWN
2014-06-13 19:57 ` Eric Blake
@ 2014-06-15 0:32 ` Wenchao Xia
0 siblings, 0 replies; 77+ messages in thread
From: Wenchao Xia @ 2014-06-15 0:32 UTC (permalink / raw)
To: Eric Blake, qemu-devel; +Cc: mdroth, armbru, lcapitulino
于 2014/6/14 3:57, Eric Blake 写道:
> On 06/05/2014 06:22 AM, Wenchao Xia wrote:
>> This patch also eliminates build time warning caused by
>> QAPI_EVENT_MAX = 0.
>
> I still don't know why I wasn't seeing a warning for that, but agree
> this cleans it up (or whichever event gets converted first, as there
> aren't really any dependency restrictions in the order in which you do
> conversions). If you do follow my suggestion of adding a workaround in
> 6/29 to avoid build warnings, then don't undo it here, but save it until
> 29/29; that way, no one individual conversion is doing more work than
> any other.
>
>>
>> Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
>> ---
>> docs/qmp/qmp-events.txt | 15 ---------------
>> qapi-event.json | 12 ++++++++++++
>> vl.c | 3 ++-
>> 3 files changed, 14 insertions(+), 16 deletions(-)
>
> Yay - I finally have enough to see what code gets generated in 3/29.
> The qapi-event.h header now has:
>
> void qapi_event_send_shutdown(Error **errp);
>
> extern const char *QAPIEvent_lookup[];
> typedef enum QAPIEvent
> {
> QAPI_EVENT_SHUTDOWN = 0,
> QAPI_EVENT_MAX = 1,
> } QAPIEvent;
>
>
> and the .c file has:
>
> void qapi_event_send_shutdown(Error **errp)
> {
> QDict *qmp;
> Error *local_err = NULL;
> QMPEventFuncEmit emit;
> emit = qmp_event_get_func_emit();
> if (!emit) {
> return;
> }
>
> qmp = qmp_event_build_dict("SHUTDOWN");
>
> emit(QAPI_EVENT_SHUTDOWN, qmp, &local_err);
>
> error_propagate(errp, local_err);
> QDECREF(qmp);
> }
>
>
> Looks good, for a data-free event (I guess I'll have to wait till later
> in the series to see what gets generated for an event with data). Hmm,
> I wonder if patch 3/29 should have also modified docs/qapi-code-gen.txt
> to show a sample generated function.
>
Will add doc part.
>>
>> -
>> -Emitted when the Virtual Machine is powered down.
>
>> +++ b/qapi-event.json
>> @@ -0,0 +1,12 @@
>> +##
>> +# @SHUTDOWN
>> +#
>> +# Emitted when the virtual machine shutdown, qemu terminated the emulation and
>> +# is about to exit.
>
> Different wording than the text it replaces, and the grammar is a bit
> unusual. Maybe:
>
> Emitted when the virtual machine has shut down, indicating that qemu is
> about to exit.
>
>> if (qemu_shutdown_requested()) {
>> qemu_kill_report();
>> - monitor_protocol_event(QEVENT_SHUTDOWN, NULL);
>> + qapi_event_send_shutdown(NULL);
>
> Note that the two NULLs serve different purposes. In the old code, NULL
> meant there was no additional data. In the new code, NULL indicates
> that we are ignoring the possibility for error. I wonder if it would be
> better to pass &error_abort instead of NULL? For that matter, I just
> re-read 6/29 - the one implementation we have of an emit function
> (monitor_qapi_event_queue) never sets errp. Is it better to just
> consider events as best-effort, and completely ditch the errp parameter
> from the emit callback typedef in 2/29, since it looks like you intend
> to pass NULL for all clients of the callback?
>
It make things simple, I will remove &errp in next version.
^ permalink raw reply [flat|nested] 77+ messages in thread
* [Qemu-devel] [PATCH V6 09/29] qapi event: convert POWERDOWN
2014-06-05 12:21 [Qemu-devel] [PATCH V6 00/29] add direct support of event in qapi schema Wenchao Xia
` (7 preceding siblings ...)
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 08/29] qapi event: convert SHUTDOWN Wenchao Xia
@ 2014-06-05 12:22 ` Wenchao Xia
2014-06-13 20:02 ` Eric Blake
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 10/29] qapi event: convert RESET Wenchao Xia
` (20 subsequent siblings)
29 siblings, 1 reply; 77+ messages in thread
From: Wenchao Xia @ 2014-06-05 12:22 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, armbru, Wenchao Xia, lcapitulino
There is no existing comments for POWERDOWN in doc/qmp/qmp-events.txt,
so no change on it like other conversion patch.
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
---
qapi-event.json | 10 ++++++++++
vl.c | 2 +-
2 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/qapi-event.json b/qapi-event.json
index b2a943f..fbdbc7c 100644
--- a/qapi-event.json
+++ b/qapi-event.json
@@ -10,3 +10,13 @@
# Since: 2.1
##
{ 'event': 'SHUTDOWN' }
+
+##
+# @POWERDOWN
+#
+# Emitted when the virtual machine is powered down through the power control
+# system, such as via ACPI.
+#
+# Since: 2.1
+##
+{ 'event': 'POWERDOWN' }
diff --git a/vl.c b/vl.c
index 273d237..69ad0e9 100644
--- a/vl.c
+++ b/vl.c
@@ -1991,7 +1991,7 @@ void qemu_system_shutdown_request(void)
static void qemu_system_powerdown(void)
{
- monitor_protocol_event(QEVENT_POWERDOWN, NULL);
+ qapi_event_send_powerdown(NULL);
notifier_list_notify(&powerdown_notifiers, NULL);
}
--
1.7.1
^ permalink raw reply related [flat|nested] 77+ messages in thread* Re: [Qemu-devel] [PATCH V6 09/29] qapi event: convert POWERDOWN
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 09/29] qapi event: convert POWERDOWN Wenchao Xia
@ 2014-06-13 20:02 ` Eric Blake
0 siblings, 0 replies; 77+ messages in thread
From: Eric Blake @ 2014-06-13 20:02 UTC (permalink / raw)
To: Wenchao Xia, qemu-devel; +Cc: mdroth, armbru, lcapitulino
[-- Attachment #1: Type: text/plain, Size: 953 bytes --]
On 06/05/2014 06:22 AM, Wenchao Xia wrote:
> There is no existing comments for POWERDOWN in doc/qmp/qmp-events.txt,
> so no change on it like other conversion patch.
Oddly enough, POWERDOWN _is_ documented in docs/qmp/qmp-spec.txt, but
that reference doesn't need updating with this conversion.
>
> Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
> ---
> qapi-event.json | 10 ++++++++++
> vl.c | 2 +-
> 2 files changed, 11 insertions(+), 1 deletions(-)
> +++ b/vl.c
> @@ -1991,7 +1991,7 @@ void qemu_system_shutdown_request(void)
>
> static void qemu_system_powerdown(void)
> {
> - monitor_protocol_event(QEVENT_POWERDOWN, NULL);
> + qapi_event_send_powerdown(NULL);
Modulo my comment in 8/29 on whether we even need an errp argument,
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: 604 bytes --]
^ permalink raw reply [flat|nested] 77+ messages in thread
* [Qemu-devel] [PATCH V6 10/29] qapi event: convert RESET
2014-06-05 12:21 [Qemu-devel] [PATCH V6 00/29] add direct support of event in qapi schema Wenchao Xia
` (8 preceding siblings ...)
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 09/29] qapi event: convert POWERDOWN Wenchao Xia
@ 2014-06-05 12:22 ` Wenchao Xia
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 11/29] qapi event: convert STOP Wenchao Xia
` (19 subsequent siblings)
29 siblings, 0 replies; 77+ messages in thread
From: Wenchao Xia @ 2014-06-05 12:22 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, armbru, Wenchao Xia, lcapitulino
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
---
docs/qmp/qmp-events.txt | 12 ------------
qapi-event.json | 9 +++++++++
vl.c | 2 +-
3 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/docs/qmp/qmp-events.txt b/docs/qmp/qmp-events.txt
index ff2f30d..20e3151 100644
--- a/docs/qmp/qmp-events.txt
+++ b/docs/qmp/qmp-events.txt
@@ -264,18 +264,6 @@ Example:
"data": { "node-name": "1.raw", "sector-num": 345435, "sector-count": 5 },
"timestamp": { "seconds": 1344522075, "microseconds": 745528 } }
-RESET
------
-
-Emitted when the Virtual Machine is reseted.
-
-Data: None.
-
-Example:
-
-{ "event": "RESET",
- "timestamp": { "seconds": 1267041653, "microseconds": 9518 } }
-
RESUME
------
diff --git a/qapi-event.json b/qapi-event.json
index fbdbc7c..6f27555 100644
--- a/qapi-event.json
+++ b/qapi-event.json
@@ -20,3 +20,12 @@
# Since: 2.1
##
{ 'event': 'POWERDOWN' }
+
+##
+# @RESET
+#
+# Emitted when the virtual machine is reset
+#
+# Since: 2.1
+##
+{ 'event': 'RESET' }
diff --git a/vl.c b/vl.c
index 69ad0e9..b95fc5e 100644
--- a/vl.c
+++ b/vl.c
@@ -1907,7 +1907,7 @@ void qemu_system_reset(bool report)
qemu_devices_reset();
}
if (report) {
- monitor_protocol_event(QEVENT_RESET, NULL);
+ qapi_event_send_reset(NULL);
}
cpu_synchronize_all_post_reset();
}
--
1.7.1
^ permalink raw reply related [flat|nested] 77+ messages in thread* [Qemu-devel] [PATCH V6 11/29] qapi event: convert STOP
2014-06-05 12:21 [Qemu-devel] [PATCH V6 00/29] add direct support of event in qapi schema Wenchao Xia
` (9 preceding siblings ...)
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 10/29] qapi event: convert RESET Wenchao Xia
@ 2014-06-05 12:22 ` Wenchao Xia
2014-06-13 20:29 ` Eric Blake
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 12/29] qapi event: convert RESUME Wenchao Xia
` (18 subsequent siblings)
29 siblings, 1 reply; 77+ messages in thread
From: Wenchao Xia @ 2014-06-05 12:22 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, armbru, Wenchao Xia, lcapitulino
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
---
cpus.c | 5 +++--
docs/qmp/qmp-events.txt | 12 ------------
qapi-event.json | 9 +++++++++
3 files changed, 12 insertions(+), 14 deletions(-)
diff --git a/cpus.c b/cpus.c
index dd7ac13..28abb11 100644
--- a/cpus.c
+++ b/cpus.c
@@ -25,7 +25,7 @@
/* Needed early for CONFIG_BSD etc. */
#include "config-host.h"
-#include "monitor/monitor.h"
+#include "qapi/qmp/qerror.h"
#include "sysemu/sysemu.h"
#include "exec/gdbstub.h"
#include "sysemu/dma.h"
@@ -38,6 +38,7 @@
#include "qemu/main-loop.h"
#include "qemu/bitmap.h"
#include "qemu/seqlock.h"
+#include "qapi-event.h"
#ifndef _WIN32
#include "qemu/compatfd.h"
@@ -530,7 +531,7 @@ static int do_vm_stop(RunState state)
pause_all_vcpus();
runstate_set(state);
vm_state_notify(0, state);
- monitor_protocol_event(QEVENT_STOP, NULL);
+ qapi_event_send_stop(NULL);
}
bdrv_drain_all();
diff --git a/docs/qmp/qmp-events.txt b/docs/qmp/qmp-events.txt
index 20e3151..c241a07 100644
--- a/docs/qmp/qmp-events.txt
+++ b/docs/qmp/qmp-events.txt
@@ -354,18 +354,6 @@ Example:
"channel-id": 0, "tls": true}
}}
-STOP
-----
-
-Emitted when the Virtual Machine is stopped.
-
-Data: None.
-
-Example:
-
-{ "event": "STOP",
- "timestamp": { "seconds": 1267041730, "microseconds": 281295 } }
-
SUSPEND
-------
diff --git a/qapi-event.json b/qapi-event.json
index 6f27555..af5a18c 100644
--- a/qapi-event.json
+++ b/qapi-event.json
@@ -29,3 +29,12 @@
# Since: 2.1
##
{ 'event': 'RESET' }
+
+##
+# @STOP
+#
+# Emitted when the virtual machine is stopped
+#
+# Since: 2.1
+##
+{ 'event': 'STOP' }
--
1.7.1
^ permalink raw reply related [flat|nested] 77+ messages in thread* Re: [Qemu-devel] [PATCH V6 11/29] qapi event: convert STOP
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 11/29] qapi event: convert STOP Wenchao Xia
@ 2014-06-13 20:29 ` Eric Blake
2014-06-17 9:17 ` Paolo Bonzini
0 siblings, 1 reply; 77+ messages in thread
From: Eric Blake @ 2014-06-13 20:29 UTC (permalink / raw)
To: Wenchao Xia, qemu-devel; +Cc: mdroth, armbru, lcapitulino
[-- Attachment #1: Type: text/plain, Size: 943 bytes --]
On 06/05/2014 06:22 AM, Wenchao Xia wrote:
> Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
> ---
> cpus.c | 5 +++--
> docs/qmp/qmp-events.txt | 12 ------------
> qapi-event.json | 9 +++++++++
> 3 files changed, 12 insertions(+), 14 deletions(-)
>
> @@ -530,7 +531,7 @@ static int do_vm_stop(RunState state)
> pause_all_vcpus();
> runstate_set(state);
> vm_state_notify(0, state);
> - monitor_protocol_event(QEVENT_STOP, NULL);
> + qapi_event_send_stop(NULL);
Same comments from earlier about not needing errp argument
> +++ b/qapi-event.json
> @@ -29,3 +29,12 @@
> # Since: 2.1
> ##
> { 'event': 'RESET' }
> +
> +##
> +# @STOP
> +#
> +# Emitted when the virtual machine is stopped
> +#
> +# Since: 2.1
0.14.0
--
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: 604 bytes --]
^ permalink raw reply [flat|nested] 77+ messages in thread* Re: [Qemu-devel] [PATCH V6 11/29] qapi event: convert STOP
2014-06-13 20:29 ` Eric Blake
@ 2014-06-17 9:17 ` Paolo Bonzini
2014-06-17 13:18 ` Eric Blake
0 siblings, 1 reply; 77+ messages in thread
From: Paolo Bonzini @ 2014-06-17 9:17 UTC (permalink / raw)
To: Eric Blake, Wenchao Xia, qemu-devel; +Cc: lcapitulino, mdroth, armbru
Il 13/06/2014 22:29, Eric Blake ha scritto:
>> > +#
>> > +# Emitted when the virtual machine is stopped
>> > +#
>> > +# Since: 2.1
> 0.14.0
0.12.0, actually:
$ git describe --contains b1a15e7eaafba8f26e2263b1a9b6e6d40e585e72
v0.12.0-rc0~187
Paolo
^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [Qemu-devel] [PATCH V6 11/29] qapi event: convert STOP
2014-06-17 9:17 ` Paolo Bonzini
@ 2014-06-17 13:18 ` Eric Blake
0 siblings, 0 replies; 77+ messages in thread
From: Eric Blake @ 2014-06-17 13:18 UTC (permalink / raw)
To: Paolo Bonzini, Wenchao Xia, qemu-devel; +Cc: lcapitulino, mdroth, armbru
[-- Attachment #1: Type: text/plain, Size: 788 bytes --]
On 06/17/2014 03:17 AM, Paolo Bonzini wrote:
> Il 13/06/2014 22:29, Eric Blake ha scritto:
>>> > +#
>>> > +# Emitted when the virtual machine is stopped
>>> > +#
>>> > +# Since: 2.1
>> 0.14.0
>
> 0.12.0, actually:
>
> $ git describe --contains b1a15e7eaafba8f26e2263b1a9b6e6d40e585e72
> v0.12.0-rc0~187
But nothing else in qapi-schema.json mentions anything earlier than
0.14. True, QMP was first started in the 0.12 timeframe, but upstream
libvirt refuses to use QMP for anything earlier than qemu 0.15, unless
libvirt was able to probe that enough additional functionality of QMP
was backported to make it useful (thus it supports RHEL 6's qemu 0.12+).
--
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: 604 bytes --]
^ permalink raw reply [flat|nested] 77+ messages in thread
* [Qemu-devel] [PATCH V6 12/29] qapi event: convert RESUME
2014-06-05 12:21 [Qemu-devel] [PATCH V6 00/29] add direct support of event in qapi schema Wenchao Xia
` (10 preceding siblings ...)
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 11/29] qapi event: convert STOP Wenchao Xia
@ 2014-06-05 12:22 ` Wenchao Xia
2014-06-13 20:33 ` Eric Blake
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 13/29] qapi event: convert SUSPEND Wenchao Xia
` (17 subsequent siblings)
29 siblings, 1 reply; 77+ messages in thread
From: Wenchao Xia @ 2014-06-05 12:22 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, armbru, Wenchao Xia, lcapitulino
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
---
docs/qmp/qmp-events.txt | 12 ------------
qapi-event.json | 9 +++++++++
vl.c | 2 +-
3 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/docs/qmp/qmp-events.txt b/docs/qmp/qmp-events.txt
index c241a07..cda67d4 100644
--- a/docs/qmp/qmp-events.txt
+++ b/docs/qmp/qmp-events.txt
@@ -264,18 +264,6 @@ Example:
"data": { "node-name": "1.raw", "sector-num": 345435, "sector-count": 5 },
"timestamp": { "seconds": 1344522075, "microseconds": 745528 } }
-RESUME
-------
-
-Emitted when the Virtual Machine resumes execution.
-
-Data: None.
-
-Example:
-
-{ "event": "RESUME",
- "timestamp": { "seconds": 1271770767, "microseconds": 582542 } }
-
RTC_CHANGE
----------
diff --git a/qapi-event.json b/qapi-event.json
index af5a18c..3485985 100644
--- a/qapi-event.json
+++ b/qapi-event.json
@@ -38,3 +38,12 @@
# Since: 2.1
##
{ 'event': 'STOP' }
+
+##
+# @RESUME
+#
+# Emitted when the virtual machine resumes execution
+#
+# Since: 2.1
+##
+{ 'event': 'RESUME' }
diff --git a/vl.c b/vl.c
index b95fc5e..d195e50 100644
--- a/vl.c
+++ b/vl.c
@@ -1755,7 +1755,7 @@ void vm_start(void)
runstate_set(RUN_STATE_RUNNING);
vm_state_notify(1, RUN_STATE_RUNNING);
resume_all_vcpus();
- monitor_protocol_event(QEVENT_RESUME, NULL);
+ qapi_event_send_resume(NULL);
}
}
--
1.7.1
^ permalink raw reply related [flat|nested] 77+ messages in thread* [Qemu-devel] [PATCH V6 13/29] qapi event: convert SUSPEND
2014-06-05 12:21 [Qemu-devel] [PATCH V6 00/29] add direct support of event in qapi schema Wenchao Xia
` (11 preceding siblings ...)
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 12/29] qapi event: convert RESUME Wenchao Xia
@ 2014-06-05 12:22 ` Wenchao Xia
2014-06-13 20:40 ` Eric Blake
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 14/29] qapi event: convert SUSPEND_DISK Wenchao Xia
` (16 subsequent siblings)
29 siblings, 1 reply; 77+ messages in thread
From: Wenchao Xia @ 2014-06-05 12:22 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, armbru, Wenchao Xia, lcapitulino
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
---
docs/qmp/qmp-events.txt | 12 ------------
qapi-event.json | 10 ++++++++++
vl.c | 2 +-
3 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/docs/qmp/qmp-events.txt b/docs/qmp/qmp-events.txt
index cda67d4..d86a077 100644
--- a/docs/qmp/qmp-events.txt
+++ b/docs/qmp/qmp-events.txt
@@ -342,18 +342,6 @@ Example:
"channel-id": 0, "tls": true}
}}
-SUSPEND
--------
-
-Emitted when guest enters S3 state.
-
-Data: None.
-
-Example:
-
-{ "event": "SUSPEND",
- "timestamp": { "seconds": 1344456160, "microseconds": 309119 } }
-
SUSPEND_DISK
------------
diff --git a/qapi-event.json b/qapi-event.json
index 3485985..83c4ea2 100644
--- a/qapi-event.json
+++ b/qapi-event.json
@@ -47,3 +47,13 @@
# Since: 2.1
##
{ 'event': 'RESUME' }
+
+##
+# @SUSPEND
+#
+# Emitted when guest enters a hardware suspension state, for example, S3 state,
+# which is sometimes called standby state
+#
+# Since: 2.1
+##
+{ 'event': 'SUSPEND' }
diff --git a/vl.c b/vl.c
index d195e50..c6cb2c1 100644
--- a/vl.c
+++ b/vl.c
@@ -1928,7 +1928,7 @@ static void qemu_system_suspend(void)
pause_all_vcpus();
notifier_list_notify(&suspend_notifiers, NULL);
runstate_set(RUN_STATE_SUSPENDED);
- monitor_protocol_event(QEVENT_SUSPEND, NULL);
+ qapi_event_send_suspend(NULL);
}
void qemu_system_suspend_request(void)
--
1.7.1
^ permalink raw reply related [flat|nested] 77+ messages in thread* Re: [Qemu-devel] [PATCH V6 13/29] qapi event: convert SUSPEND
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 13/29] qapi event: convert SUSPEND Wenchao Xia
@ 2014-06-13 20:40 ` Eric Blake
0 siblings, 0 replies; 77+ messages in thread
From: Eric Blake @ 2014-06-13 20:40 UTC (permalink / raw)
To: Wenchao Xia, qemu-devel; +Cc: mdroth, armbru, lcapitulino
[-- Attachment #1: Type: text/plain, Size: 597 bytes --]
On 06/05/2014 06:22 AM, Wenchao Xia wrote:
> Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
> ---
> docs/qmp/qmp-events.txt | 12 ------------
> qapi-event.json | 10 ++++++++++
> vl.c | 2 +-
> 3 files changed, 11 insertions(+), 13 deletions(-)
>
> +##
> +# @SUSPEND
> +#
> +# Emitted when guest enters a hardware suspension state, for example, S3 state,
> +# which is sometimes called standby state
> +#
> +# Since: 2.1
1.1
--
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: 604 bytes --]
^ permalink raw reply [flat|nested] 77+ messages in thread
* [Qemu-devel] [PATCH V6 14/29] qapi event: convert SUSPEND_DISK
2014-06-05 12:21 [Qemu-devel] [PATCH V6 00/29] add direct support of event in qapi schema Wenchao Xia
` (12 preceding siblings ...)
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 13/29] qapi event: convert SUSPEND Wenchao Xia
@ 2014-06-05 12:22 ` Wenchao Xia
2014-06-13 20:42 ` Eric Blake
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 15/29] qapi event: convert WAKEUP Wenchao Xia
` (15 subsequent siblings)
29 siblings, 1 reply; 77+ messages in thread
From: Wenchao Xia @ 2014-06-05 12:22 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, armbru, Wenchao Xia, lcapitulino
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
---
docs/qmp/qmp-events.txt | 14 --------------
hw/acpi/core.c | 4 ++--
qapi-event.json | 12 ++++++++++++
3 files changed, 14 insertions(+), 16 deletions(-)
diff --git a/docs/qmp/qmp-events.txt b/docs/qmp/qmp-events.txt
index d86a077..c2f23ef 100644
--- a/docs/qmp/qmp-events.txt
+++ b/docs/qmp/qmp-events.txt
@@ -342,20 +342,6 @@ Example:
"channel-id": 0, "tls": true}
}}
-SUSPEND_DISK
-------------
-
-Emitted when the guest makes a request to enter S4 state.
-
-Data: None.
-
-Example:
-
-{ "event": "SUSPEND_DISK",
- "timestamp": { "seconds": 1344456160, "microseconds": 309119 } }
-
-Note: QEMU shuts down when entering S4 state.
-
VNC_CONNECTED
-------------
diff --git a/hw/acpi/core.c b/hw/acpi/core.c
index 79414b4..ace6438 100644
--- a/hw/acpi/core.c
+++ b/hw/acpi/core.c
@@ -22,11 +22,11 @@
#include "hw/hw.h"
#include "hw/i386/pc.h"
#include "hw/acpi/acpi.h"
-#include "monitor/monitor.h"
#include "qemu/config-file.h"
#include "qapi/opts-visitor.h"
#include "qapi/dealloc-visitor.h"
#include "qapi-visit.h"
+#include "qapi-event.h"
struct acpi_table_header {
uint16_t _length; /* our length, not actual part of the hdr */
@@ -550,7 +550,7 @@ static void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val)
break;
default:
if (sus_typ == ar->pm1.cnt.s4_val) { /* S4 request */
- monitor_protocol_event(QEVENT_SUSPEND_DISK, NULL);
+ qapi_event_send_suspend_disk(NULL);
qemu_system_shutdown_request();
}
break;
diff --git a/qapi-event.json b/qapi-event.json
index 83c4ea2..c68edfd 100644
--- a/qapi-event.json
+++ b/qapi-event.json
@@ -57,3 +57,15 @@
# Since: 2.1
##
{ 'event': 'SUSPEND' }
+
+##
+# @SUSPEND_DISK
+#
+# Emitted when guest enters a hardware suspension state with data saved on
+# disk, for example, S4 state, which is sometimes called hibernate state
+#
+# Note: QEMU shuts down (similar to event @SHUTDOWN) when entering this state
+#
+# Since: 2.1
+##
+{ 'event': 'SUSPEND_DISK' }
--
1.7.1
^ permalink raw reply related [flat|nested] 77+ messages in thread* Re: [Qemu-devel] [PATCH V6 14/29] qapi event: convert SUSPEND_DISK
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 14/29] qapi event: convert SUSPEND_DISK Wenchao Xia
@ 2014-06-13 20:42 ` Eric Blake
0 siblings, 0 replies; 77+ messages in thread
From: Eric Blake @ 2014-06-13 20:42 UTC (permalink / raw)
To: Wenchao Xia, qemu-devel; +Cc: mdroth, armbru, lcapitulino
[-- Attachment #1: Type: text/plain, Size: 825 bytes --]
On 06/05/2014 06:22 AM, Wenchao Xia wrote:
> Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
> ---
> docs/qmp/qmp-events.txt | 14 --------------
> hw/acpi/core.c | 4 ++--
> qapi-event.json | 12 ++++++++++++
> 3 files changed, 14 insertions(+), 16 deletions(-)
>
> +++ b/qapi-event.json
> @@ -57,3 +57,15 @@
> # Since: 2.1
> ##
> { 'event': 'SUSPEND' }
> +
> +##
> +# @SUSPEND_DISK
> +#
> +# Emitted when guest enters a hardware suspension state with data saved on
> +# disk, for example, S4 state, which is sometimes called hibernate state
> +#
> +# Note: QEMU shuts down (similar to event @SHUTDOWN) when entering this state
> +#
> +# Since: 2.1
1.2
--
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: 604 bytes --]
^ permalink raw reply [flat|nested] 77+ messages in thread
* [Qemu-devel] [PATCH V6 15/29] qapi event: convert WAKEUP
2014-06-05 12:21 [Qemu-devel] [PATCH V6 00/29] add direct support of event in qapi schema Wenchao Xia
` (13 preceding siblings ...)
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 14/29] qapi event: convert SUSPEND_DISK Wenchao Xia
@ 2014-06-05 12:22 ` Wenchao Xia
2014-06-13 20:57 ` Eric Blake
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 16/29] qapi event: convert RTC_CHANGE Wenchao Xia
` (14 subsequent siblings)
29 siblings, 1 reply; 77+ messages in thread
From: Wenchao Xia @ 2014-06-05 12:22 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, armbru, Wenchao Xia, lcapitulino
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
---
docs/qmp/qmp-events.txt | 12 ------------
qapi-event.json | 9 +++++++++
vl.c | 2 +-
3 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/docs/qmp/qmp-events.txt b/docs/qmp/qmp-events.txt
index c2f23ef..3d82db4 100644
--- a/docs/qmp/qmp-events.txt
+++ b/docs/qmp/qmp-events.txt
@@ -432,18 +432,6 @@ Example:
"host": "127.0.0.1", "sasl_username": "luiz" } },
"timestamp": { "seconds": 1263475302, "microseconds": 150772 } }
-WAKEUP
-------
-
-Emitted when the guest has woken up from S3 and is running.
-
-Data: None.
-
-Example:
-
-{ "event": "WAKEUP",
- "timestamp": { "seconds": 1344522075, "microseconds": 745528 } }
-
WATCHDOG
--------
diff --git a/qapi-event.json b/qapi-event.json
index c68edfd..32ac571 100644
--- a/qapi-event.json
+++ b/qapi-event.json
@@ -69,3 +69,12 @@
# Since: 2.1
##
{ 'event': 'SUSPEND_DISK' }
+
+##
+# @WAKEUP
+#
+# Emitted when the guest has woken up from suspend state and is running
+#
+# Since: 2.1
+##
+{ 'event': 'WAKEUP' }
diff --git a/vl.c b/vl.c
index c6cb2c1..b8ad1df 100644
--- a/vl.c
+++ b/vl.c
@@ -2052,7 +2052,7 @@ static bool main_loop_should_exit(void)
notifier_list_notify(&wakeup_notifiers, &wakeup_reason);
wakeup_reason = QEMU_WAKEUP_REASON_NONE;
resume_all_vcpus();
- monitor_protocol_event(QEVENT_WAKEUP, NULL);
+ qapi_event_send_wakeup(NULL);
}
if (qemu_powerdown_requested()) {
qemu_system_powerdown();
--
1.7.1
^ permalink raw reply related [flat|nested] 77+ messages in thread* [Qemu-devel] [PATCH V6 16/29] qapi event: convert RTC_CHANGE
2014-06-05 12:21 [Qemu-devel] [PATCH V6 00/29] add direct support of event in qapi schema Wenchao Xia
` (14 preceding siblings ...)
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 15/29] qapi event: convert WAKEUP Wenchao Xia
@ 2014-06-05 12:22 ` Wenchao Xia
2014-06-13 21:27 ` Eric Blake
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 17/29] qapi event: convert WATCHDOG Wenchao Xia
` (13 subsequent siblings)
29 siblings, 1 reply; 77+ messages in thread
From: Wenchao Xia @ 2014-06-05 12:22 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, armbru, Wenchao Xia, lcapitulino
This patch also eliminates build time warning caused by no caller
of monitor_qapi_event_throttle().
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
---
docs/qmp/qmp-events.txt | 16 ----------------
hw/ppc/spapr_rtas.c | 3 ++-
hw/timer/mc146818rtc.c | 3 ++-
include/sysemu/sysemu.h | 2 --
monitor.c | 4 +++-
qapi-event.json | 13 +++++++++++++
vl.c | 9 ---------
7 files changed, 20 insertions(+), 30 deletions(-)
diff --git a/docs/qmp/qmp-events.txt b/docs/qmp/qmp-events.txt
index 3d82db4..8cad3e7 100644
--- a/docs/qmp/qmp-events.txt
+++ b/docs/qmp/qmp-events.txt
@@ -264,22 +264,6 @@ Example:
"data": { "node-name": "1.raw", "sector-num": 345435, "sector-count": 5 },
"timestamp": { "seconds": 1344522075, "microseconds": 745528 } }
-RTC_CHANGE
-----------
-
-Emitted when the guest changes the RTC time.
-
-Data:
-
-- "offset": Offset between base RTC clock (as specified by -rtc base), and
-new RTC clock value (json-number)
-
-Example:
-
-{ "event": "RTC_CHANGE",
- "data": { "offset": 78 },
- "timestamp": { "seconds": 1267020223, "microseconds": 435656 } }
-
SPICE_CONNECTED, SPICE_DISCONNECTED
-----------------------------------
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index ea4a2b2..7e4cffe 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -32,6 +32,7 @@
#include "hw/ppc/spapr.h"
#include "hw/ppc/spapr_vio.h"
+#include "qapi-event.h"
#include <libfdt.h>
@@ -93,7 +94,7 @@ static void rtas_set_time_of_day(PowerPCCPU *cpu, sPAPREnvironment *spapr,
tm.tm_sec = rtas_ld(args, 5);
/* Just generate a monitor event for the change */
- rtc_change_mon_event(&tm);
+ qapi_event_send_rtc_change(qemu_timedate_diff(&tm), NULL);
spapr->rtc_offset = qemu_timedate_diff(&tm);
rtas_st(rets, 0, RTAS_OUT_SUCCESS);
diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
index df54546..9bb3708 100644
--- a/hw/timer/mc146818rtc.c
+++ b/hw/timer/mc146818rtc.c
@@ -26,6 +26,7 @@
#include "sysemu/sysemu.h"
#include "hw/timer/mc146818rtc.h"
#include "qapi/visitor.h"
+#include "qapi-event.h"
#ifdef TARGET_I386
#include "hw/i386/apic.h"
@@ -530,7 +531,7 @@ static void rtc_set_time(RTCState *s)
s->base_rtc = mktimegm(&tm);
s->last_update = qemu_clock_get_ns(rtc_clock);
- rtc_change_mon_event(&tm);
+ qapi_event_send_rtc_change(qemu_timedate_diff(&tm), NULL);
}
static void rtc_set_cmos(RTCState *s, const struct tm *tm)
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index ba5c7f8..0046b27 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -189,8 +189,6 @@ void do_usb_add(Monitor *mon, const QDict *qdict);
void do_usb_del(Monitor *mon, const QDict *qdict);
void usb_info(Monitor *mon, const QDict *qdict);
-void rtc_change_mon_event(struct tm *tm);
-
void add_boot_device_path(int32_t bootindex, DeviceState *dev,
const char *suffix);
char *get_boot_devices_list(size_t *size, bool ignore_suffixes);
diff --git a/monitor.c b/monitor.c
index e122381..e6d32c2 100644
--- a/monitor.c
+++ b/monitor.c
@@ -612,6 +612,9 @@ static void monitor_qapi_event_throttle(QAPIEvent event, int64_t rate)
static void monitor_qapi_event_init(void)
{
+ /* Limit RTC & BALLOON events to 1 per second */
+ monitor_qapi_event_throttle(QAPI_EVENT_RTC_CHANGE, 1000);
+
qmp_event_set_func_emit(monitor_qapi_event_queue);
}
@@ -737,7 +740,6 @@ monitor_protocol_event_throttle(MonitorEvent event,
static void monitor_protocol_event_init(void)
{
/* Limit RTC & BALLOON events to 1 per second */
- monitor_protocol_event_throttle(QEVENT_RTC_CHANGE, 1000);
monitor_protocol_event_throttle(QEVENT_BALLOON_CHANGE, 1000);
monitor_protocol_event_throttle(QEVENT_WATCHDOG, 1000);
/* limit the rate of quorum events to avoid hammering the management */
diff --git a/qapi-event.json b/qapi-event.json
index 32ac571..dc20bb4 100644
--- a/qapi-event.json
+++ b/qapi-event.json
@@ -78,3 +78,16 @@
# Since: 2.1
##
{ 'event': 'WAKEUP' }
+
+##
+# @RTC_CHANGE
+#
+# Emitted when the guest changes the RTC time.
+#
+# @offset: offset between base RTC clock (as specified by -rtc base), and
+# new RTC clock value
+#
+# Since: 2.1
+##
+{ 'event': 'RTC_CHANGE',
+ 'data': { 'offset': 'int' } }
diff --git a/vl.c b/vl.c
index b8ad1df..cb56714 100644
--- a/vl.c
+++ b/vl.c
@@ -727,15 +727,6 @@ int qemu_timedate_diff(struct tm *tm)
return seconds - time(NULL);
}
-void rtc_change_mon_event(struct tm *tm)
-{
- QObject *data;
-
- data = qobject_from_jsonf("{ 'offset': %d }", qemu_timedate_diff(tm));
- monitor_protocol_event(QEVENT_RTC_CHANGE, data);
- qobject_decref(data);
-}
-
static void configure_rtc_date_offset(const char *startdate, int legacy)
{
time_t rtc_start_date;
--
1.7.1
^ permalink raw reply related [flat|nested] 77+ messages in thread* Re: [Qemu-devel] [PATCH V6 16/29] qapi event: convert RTC_CHANGE
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 16/29] qapi event: convert RTC_CHANGE Wenchao Xia
@ 2014-06-13 21:27 ` Eric Blake
2014-06-15 0:38 ` Wenchao Xia
` (2 more replies)
0 siblings, 3 replies; 77+ messages in thread
From: Eric Blake @ 2014-06-13 21:27 UTC (permalink / raw)
To: Wenchao Xia, qemu-devel; +Cc: mdroth, armbru, lcapitulino
[-- Attachment #1: Type: text/plain, Size: 5038 bytes --]
On 06/05/2014 06:22 AM, Wenchao Xia wrote:
> This patch also eliminates build time warning caused by no caller
> of monitor_qapi_event_throttle().
Again, my suggestion on 6/29 could avoid that warning; if you use that
workaround, don't clean it until 29/29, but you can drop this paragraph
of this commit.
>
> Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
> ---
> docs/qmp/qmp-events.txt | 16 ----------------
> hw/ppc/spapr_rtas.c | 3 ++-
> hw/timer/mc146818rtc.c | 3 ++-
> include/sysemu/sysemu.h | 2 --
> monitor.c | 4 +++-
> qapi-event.json | 13 +++++++++++++
> vl.c | 9 ---------
> 7 files changed, 20 insertions(+), 30 deletions(-)
>
Yay - the first event with data, so I get to see what the generator does.
The .h file shows this signature:
>> void qapi_event_send_rtc_change(int64_t offset,
>> Error **errp);
and the .c has this code:
>> void qapi_event_send_rtc_change(int64_t offset,
>> Error **errp)
>> {
>> QDict *qmp;
>> Error *local_err = NULL;
>> QMPEventFuncEmit emit;
>> QmpOutputVisitor *qov;
>> Visitor *v;
>> QObject *obj;
>>
>> emit = qmp_event_get_func_emit();
>> if (!emit) {
>> return;
>> }
>>
>> qmp = qmp_event_build_dict("RTC_CHANGE");
>>
>> 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 */
Grammar error; guess I have more comments on 3/29.
>> visit_start_struct(v, NULL, "", "RTC_CHANGE", 0, &local_err);
>> if (local_err) {
>> goto clean;
>> }
Hmm, qmp_output_start_struct() never sets errp.
>>
>> visit_type_int(v, &offset, "offset", &local_err);
>> if (local_err) {
>> goto clean;
>> }
Likewise, qmp_output_type_int never sets errp.
>>
>> visit_end_struct(v, &local_err);
>> if (local_err) {
>> goto clean;
>> }
And neither does qmp_end_struct.
>>
>> obj = qmp_output_get_qobject(qov);
>> g_assert(obj != NULL);
>>
>> qdict_put_obj(qmp, "data", obj);
>> emit(QAPI_EVENT_RTC_CHANGE, qmp, &local_err);
And I already mentioned earlier that the only implementation of the
emit() callback never sets local_err (and probably doesn't even need it
as a parameter).
>>
>> clean:
>> qmp_output_visitor_cleanup(qov);
>> error_propagate(errp, local_err);
>> QDECREF(qmp);
>> }
If you change the earlier 3 instances to just use visit_...(,
&error_abort), you can completely ditch the local_err variable, drop the
clean: label, and overall have a much shorter generated function.
> @@ -93,7 +94,7 @@ static void rtas_set_time_of_day(PowerPCCPU *cpu, sPAPREnvironment *spapr,
> tm.tm_sec = rtas_ld(args, 5);
>
> /* Just generate a monitor event for the change */
> - rtc_change_mon_event(&tm);
> + qapi_event_send_rtc_change(qemu_timedate_diff(&tm), NULL);
> spapr->rtc_offset = qemu_timedate_diff(&tm);
Eww. This makes me worry whether grabbing qemu_timedate_diff() two times
in a row may cause a different value to be reported than what is stored
locally. Please grab it only once into a local variable, then copy that
value into both locations.
Once again, all callers of qapi_event_send_rtc_change() are passing a
NULL errp to silently ignore errors; and I just audited that no errors
happen anyways.
> +++ b/monitor.c
> @@ -612,6 +612,9 @@ static void monitor_qapi_event_throttle(QAPIEvent event, int64_t rate)
>
> static void monitor_qapi_event_init(void)
> {
> + /* Limit RTC & BALLOON events to 1 per second */
Comment is a bit misleading until a later patch converts balloon events,...
> + monitor_qapi_event_throttle(QAPI_EVENT_RTC_CHANGE, 1000);
> +
> qmp_event_set_func_emit(monitor_qapi_event_queue);
> }
>
> @@ -737,7 +740,6 @@ monitor_protocol_event_throttle(MonitorEvent event,
> static void monitor_protocol_event_init(void)
> {
> /* Limit RTC & BALLOON events to 1 per second */
> - monitor_protocol_event_throttle(QEVENT_RTC_CHANGE, 1000);
> monitor_protocol_event_throttle(QEVENT_BALLOON_CHANGE, 1000);
> monitor_protocol_event_throttle(QEVENT_WATCHDOG, 1000);
...furthermore, the OLD comment was wrong (it forgot about the watchdog
event). You could get around that by rewording the comment to just say
'limit guest-triggerable events to 1 per second' without calling out
what those events are.
> +# @RTC_CHANGE
> +#
> +# Emitted when the guest changes the RTC time.
> +#
> +# @offset: offset between base RTC clock (as specified by -rtc base), and
> +# new RTC clock value
> +#
> +# Since: 2.1
0.14.0
--
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: 604 bytes --]
^ permalink raw reply [flat|nested] 77+ messages in thread* Re: [Qemu-devel] [PATCH V6 16/29] qapi event: convert RTC_CHANGE
2014-06-13 21:27 ` Eric Blake
@ 2014-06-15 0:38 ` Wenchao Xia
2014-06-15 14:01 ` Paolo Bonzini
2014-06-15 14:00 ` Paolo Bonzini
2014-06-17 9:21 ` Paolo Bonzini
2 siblings, 1 reply; 77+ messages in thread
From: Wenchao Xia @ 2014-06-15 0:38 UTC (permalink / raw)
To: Eric Blake, qemu-devel; +Cc: mdroth, armbru, lcapitulino
于 2014/6/14 5:27, Eric Blake 写道:
> On 06/05/2014 06:22 AM, Wenchao Xia wrote:
>> This patch also eliminates build time warning caused by no caller
>> of monitor_qapi_event_throttle().
>
> Again, my suggestion on 6/29 could avoid that warning; if you use that
> workaround, don't clean it until 29/29, but you can drop this paragraph
> of this commit.
>
>>
>> Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
>> ---
>> docs/qmp/qmp-events.txt | 16 ----------------
>> hw/ppc/spapr_rtas.c | 3 ++-
>> hw/timer/mc146818rtc.c | 3 ++-
>> include/sysemu/sysemu.h | 2 --
>> monitor.c | 4 +++-
>> qapi-event.json | 13 +++++++++++++
>> vl.c | 9 ---------
>> 7 files changed, 20 insertions(+), 30 deletions(-)
>>
>
> Yay - the first event with data, so I get to see what the generator does.
>
> The .h file shows this signature:
>
>>> void qapi_event_send_rtc_change(int64_t offset,
>>> Error **errp);
>
> and the .c has this code:
>
>>> void qapi_event_send_rtc_change(int64_t offset,
>>> Error **errp)
>>> {
>>> QDict *qmp;
>>> Error *local_err = NULL;
>>> QMPEventFuncEmit emit;
>>> QmpOutputVisitor *qov;
>>> Visitor *v;
>>> QObject *obj;
>>>
>>> emit = qmp_event_get_func_emit();
>>> if (!emit) {
>>> return;
>>> }
>>>
>>> qmp = qmp_event_build_dict("RTC_CHANGE");
>>>
>>> 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 */
>
> Grammar error; guess I have more comments on 3/29.
>
>>> visit_start_struct(v, NULL, "", "RTC_CHANGE", 0, &local_err);
>>> if (local_err) {
>>> goto clean;
>>> }
>
> Hmm, qmp_output_start_struct() never sets errp.
>
>>>
>>> visit_type_int(v, &offset, "offset", &local_err);
>>> if (local_err) {
>>> goto clean;
>>> }
>
> Likewise, qmp_output_type_int never sets errp.
>
>>>
>>> visit_end_struct(v, &local_err);
>>> if (local_err) {
>>> goto clean;
>>> }
>
> And neither does qmp_end_struct.
>
>>>
>>> obj = qmp_output_get_qobject(qov);
>>> g_assert(obj != NULL);
>>>
>>> qdict_put_obj(qmp, "data", obj);
>>> emit(QAPI_EVENT_RTC_CHANGE, qmp, &local_err);
>
> And I already mentioned earlier that the only implementation of the
> emit() callback never sets local_err (and probably doesn't even need it
> as a parameter).
>
>>>
>>> clean:
>>> qmp_output_visitor_cleanup(qov);
>>> error_propagate(errp, local_err);
>>> QDECREF(qmp);
>>> }
>
> If you change the earlier 3 instances to just use visit_...(,
> &error_abort), you can completely ditch the local_err variable, drop the
> clean: label, and overall have a much shorter generated function.
>
>
>> @@ -93,7 +94,7 @@ static void rtas_set_time_of_day(PowerPCCPU *cpu, sPAPREnvironment *spapr,
>> tm.tm_sec = rtas_ld(args, 5);
>>
>> /* Just generate a monitor event for the change */
>> - rtc_change_mon_event(&tm);
>> + qapi_event_send_rtc_change(qemu_timedate_diff(&tm), NULL);
>> spapr->rtc_offset = qemu_timedate_diff(&tm);
>
> Eww. This makes me worry whether grabbing qemu_timedate_diff() two times
> in a row may cause a different value to be reported than what is stored
> locally. Please grab it only once into a local variable, then copy that
> value into both locations.
>
> Once again, all callers of qapi_event_send_rtc_change() are passing a
> NULL errp to silently ignore errors; and I just audited that no errors
> happen anyways.
>
Fixing it.
>> +++ b/monitor.c
>> @@ -612,6 +612,9 @@ static void monitor_qapi_event_throttle(QAPIEvent event, int64_t rate)
>>
>> static void monitor_qapi_event_init(void)
>> {
>> + /* Limit RTC & BALLOON events to 1 per second */
>
> Comment is a bit misleading until a later patch converts balloon events,...
>
Oops, good catch, will fix it.
>> + monitor_qapi_event_throttle(QAPI_EVENT_RTC_CHANGE, 1000);
>> +
>> qmp_event_set_func_emit(monitor_qapi_event_queue);
>> }
>>
>> @@ -737,7 +740,6 @@ monitor_protocol_event_throttle(MonitorEvent event,
>> static void monitor_protocol_event_init(void)
>> {
>> /* Limit RTC & BALLOON events to 1 per second */
>> - monitor_protocol_event_throttle(QEVENT_RTC_CHANGE, 1000);
>> monitor_protocol_event_throttle(QEVENT_BALLOON_CHANGE, 1000);
>> monitor_protocol_event_throttle(QEVENT_WATCHDOG, 1000);
>
> ...furthermore, the OLD comment was wrong (it forgot about the watchdog
> event). You could get around that by rewording the comment to just say
> 'limit guest-triggerable events to 1 per second' without calling out
> what those events are.
>
>> +# @RTC_CHANGE
>> +#
>> +# Emitted when the guest changes the RTC time.
>> +#
>> +# @offset: offset between base RTC clock (as specified by -rtc base), and
>> +# new RTC clock value
>> +#
>> +# Since: 2.1
>
> 0.14.0
>
^ permalink raw reply [flat|nested] 77+ messages in thread* Re: [Qemu-devel] [PATCH V6 16/29] qapi event: convert RTC_CHANGE
2014-06-15 0:38 ` Wenchao Xia
@ 2014-06-15 14:01 ` Paolo Bonzini
0 siblings, 0 replies; 77+ messages in thread
From: Paolo Bonzini @ 2014-06-15 14:01 UTC (permalink / raw)
To: Wenchao Xia, Eric Blake, qemu-devel; +Cc: lcapitulino, mdroth, armbru
Il 15/06/2014 02:38, Wenchao Xia ha scritto:
>>
>> Once again, all callers of qapi_event_send_rtc_change() are passing a
>> NULL errp to silently ignore errors; and I just audited that no errors
>> happen anyways.
>>
>
> Fixing it.
No, please don't. I prefer the way you did it in v6.
Paolo
^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [Qemu-devel] [PATCH V6 16/29] qapi event: convert RTC_CHANGE
2014-06-13 21:27 ` Eric Blake
2014-06-15 0:38 ` Wenchao Xia
@ 2014-06-15 14:00 ` Paolo Bonzini
2014-06-17 9:21 ` Paolo Bonzini
2 siblings, 0 replies; 77+ messages in thread
From: Paolo Bonzini @ 2014-06-15 14:00 UTC (permalink / raw)
To: Eric Blake, Wenchao Xia, qemu-devel; +Cc: lcapitulino, mdroth, armbru
Il 13/06/2014 23:27, Eric Blake ha scritto:
>>> >> visit_start_struct(v, NULL, "", "RTC_CHANGE", 0, &local_err);
>>> >> if (local_err) {
>>> >> goto clean;
>>> >> }
> Hmm, qmp_output_start_struct() never sets errp.
>
>>> >>
>>> >> visit_type_int(v, &offset, "offset", &local_err);
>>> >> if (local_err) {
>>> >> goto clean;
>>> >> }
> Likewise, qmp_output_type_int never sets errp.
>
I think it is better to produce correct error propagation even if it is
unused. We could add range-checking of enums, for example.
I guess all the NULLs for errp could become &error_abort, but it can be
done after the merge.
Paolo
^ permalink raw reply [flat|nested] 77+ messages in thread* Re: [Qemu-devel] [PATCH V6 16/29] qapi event: convert RTC_CHANGE
2014-06-13 21:27 ` Eric Blake
2014-06-15 0:38 ` Wenchao Xia
2014-06-15 14:00 ` Paolo Bonzini
@ 2014-06-17 9:21 ` Paolo Bonzini
2014-06-17 13:19 ` Eric Blake
2 siblings, 1 reply; 77+ messages in thread
From: Paolo Bonzini @ 2014-06-17 9:21 UTC (permalink / raw)
To: Eric Blake, Wenchao Xia, qemu-devel; +Cc: lcapitulino, mdroth, armbru
Il 13/06/2014 23:27, Eric Blake ha scritto:
>> > +# @RTC_CHANGE
>> > +#
>> > +# Emitted when the guest changes the RTC time.
>> > +#
>> > +# @offset: offset between base RTC clock (as specified by -rtc base), and
>> > +# new RTC clock value
>> > +#
>> > +# Since: 2.1
> 0.14.0
0.13.0 :)
^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [Qemu-devel] [PATCH V6 16/29] qapi event: convert RTC_CHANGE
2014-06-17 9:21 ` Paolo Bonzini
@ 2014-06-17 13:19 ` Eric Blake
0 siblings, 0 replies; 77+ messages in thread
From: Eric Blake @ 2014-06-17 13:19 UTC (permalink / raw)
To: Paolo Bonzini, Wenchao Xia, qemu-devel; +Cc: lcapitulino, mdroth, armbru
[-- Attachment #1: Type: text/plain, Size: 643 bytes --]
On 06/17/2014 03:21 AM, Paolo Bonzini wrote:
> Il 13/06/2014 23:27, Eric Blake ha scritto:
>>> > +# @RTC_CHANGE
>>> > +#
>>> > +# Emitted when the guest changes the RTC time.
>>> > +#
>>> > +# @offset: offset between base RTC clock (as specified by -rtc
>>> base), and
>>> > +# new RTC clock value
>>> > +#
>>> > +# Since: 2.1
>> 0.14.0
>
> 0.13.0 :)
Again, since nothing else in QAPI documents anything earlier than 0.14,
I'm not sure it's worth caring about whether 0.13 supported a particular
event.
--
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: 604 bytes --]
^ permalink raw reply [flat|nested] 77+ messages in thread
* [Qemu-devel] [PATCH V6 17/29] qapi event: convert WATCHDOG
2014-06-05 12:21 [Qemu-devel] [PATCH V6 00/29] add direct support of event in qapi schema Wenchao Xia
` (15 preceding siblings ...)
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 16/29] qapi event: convert RTC_CHANGE Wenchao Xia
@ 2014-06-05 12:22 ` Wenchao Xia
2014-06-13 21:47 ` Eric Blake
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 18/29] qapi event: convert DEVICE_DELETED Wenchao Xia
` (12 subsequent siblings)
29 siblings, 1 reply; 77+ messages in thread
From: Wenchao Xia @ 2014-06-05 12:22 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, armbru, Wenchao Xia, lcapitulino
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
---
docs/qmp/qmp-events.txt | 19 -------------------
hw/watchdog/watchdog.c | 23 +++++++----------------
monitor.c | 2 +-
qapi-event.json | 15 +++++++++++++++
qapi-schema.json | 24 ++++++++++++++++++++++++
5 files changed, 47 insertions(+), 36 deletions(-)
diff --git a/docs/qmp/qmp-events.txt b/docs/qmp/qmp-events.txt
index 8cad3e7..df15dc8 100644
--- a/docs/qmp/qmp-events.txt
+++ b/docs/qmp/qmp-events.txt
@@ -415,22 +415,3 @@ Example:
"client": { "family": "ipv4", "service": "46089",
"host": "127.0.0.1", "sasl_username": "luiz" } },
"timestamp": { "seconds": 1263475302, "microseconds": 150772 } }
-
-WATCHDOG
---------
-
-Emitted when the watchdog device's timer is expired.
-
-Data:
-
-- "action": Action that has been taken, it's one of the following (json-string):
- "reset", "shutdown", "poweroff", "pause", "debug", or "none"
-
-Example:
-
-{ "event": "WATCHDOG",
- "data": { "action": "reset" },
- "timestamp": { "seconds": 1267061043, "microseconds": 959568 } }
-
-Note: If action is "reset", "shutdown", or "pause" the WATCHDOG event is
-followed respectively by the RESET, SHUTDOWN, or STOP events.
diff --git a/hw/watchdog/watchdog.c b/hw/watchdog/watchdog.c
index f28161b..9284d3f 100644
--- a/hw/watchdog/watchdog.c
+++ b/hw/watchdog/watchdog.c
@@ -24,9 +24,9 @@
#include "qemu/config-file.h"
#include "qemu/queue.h"
#include "qapi/qmp/types.h"
-#include "monitor/monitor.h"
#include "sysemu/sysemu.h"
#include "sysemu/watchdog.h"
+#include "qapi-event.h"
/* Possible values for action parameter. */
#define WDT_RESET 1 /* Hard reset. */
@@ -101,15 +101,6 @@ int select_watchdog_action(const char *p)
return 0;
}
-static void watchdog_mon_event(const char *action)
-{
- QObject *data;
-
- data = qobject_from_jsonf("{ 'action': %s }", action);
- monitor_protocol_event(QEVENT_WATCHDOG, data);
- qobject_decref(data);
-}
-
/* This actually performs the "action" once a watchdog has expired,
* ie. reboot, shutdown, exit, etc.
*/
@@ -117,31 +108,31 @@ void watchdog_perform_action(void)
{
switch(watchdog_action) {
case WDT_RESET: /* same as 'system_reset' in monitor */
- watchdog_mon_event("reset");
+ qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_RESET, NULL);
qemu_system_reset_request();
break;
case WDT_SHUTDOWN: /* same as 'system_powerdown' in monitor */
- watchdog_mon_event("shutdown");
+ qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_SHUTDOWN, NULL);
qemu_system_powerdown_request();
break;
case WDT_POWEROFF: /* same as 'quit' command in monitor */
- watchdog_mon_event("poweroff");
+ qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_POWEROFF, NULL);
exit(0);
case WDT_PAUSE: /* same as 'stop' command in monitor */
- watchdog_mon_event("pause");
+ qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_PAUSE, NULL);
vm_stop(RUN_STATE_WATCHDOG);
break;
case WDT_DEBUG:
- watchdog_mon_event("debug");
+ qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_DEBUG, NULL);
fprintf(stderr, "watchdog: timer fired\n");
break;
case WDT_NONE:
- watchdog_mon_event("none");
+ qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_NONE, NULL);
break;
}
}
diff --git a/monitor.c b/monitor.c
index e6d32c2..cae60ab 100644
--- a/monitor.c
+++ b/monitor.c
@@ -614,6 +614,7 @@ static void monitor_qapi_event_init(void)
{
/* Limit RTC & BALLOON events to 1 per second */
monitor_qapi_event_throttle(QAPI_EVENT_RTC_CHANGE, 1000);
+ monitor_qapi_event_throttle(QAPI_EVENT_WATCHDOG, 1000);
qmp_event_set_func_emit(monitor_qapi_event_queue);
}
@@ -741,7 +742,6 @@ static void monitor_protocol_event_init(void)
{
/* Limit RTC & BALLOON events to 1 per second */
monitor_protocol_event_throttle(QEVENT_BALLOON_CHANGE, 1000);
- monitor_protocol_event_throttle(QEVENT_WATCHDOG, 1000);
/* limit the rate of quorum events to avoid hammering the management */
monitor_protocol_event_throttle(QEVENT_QUORUM_REPORT_BAD, 1000);
monitor_protocol_event_throttle(QEVENT_QUORUM_FAILURE, 1000);
diff --git a/qapi-event.json b/qapi-event.json
index dc20bb4..640f841 100644
--- a/qapi-event.json
+++ b/qapi-event.json
@@ -91,3 +91,18 @@
##
{ 'event': 'RTC_CHANGE',
'data': { 'offset': 'int' } }
+
+##
+# @WATCHDOG
+#
+# Emitted when the watchdog device's timer is expired
+#
+# @action: action that has been taken
+#
+# Note: If action is "reset", "shutdown", or "pause" the WATCHDOG event is
+# followed respectively by the RESET, SHUTDOWN, or STOP events
+#
+# Since: 2.1
+##
+{ 'event': 'WATCHDOG',
+ 'data': { 'action': 'WatchdogExpirationAction' } }
diff --git a/qapi-schema.json b/qapi-schema.json
index d04514a..a14504d 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4799,4 +4799,28 @@
{ 'enum': 'BlockErrorAction',
'data': [ 'ignore', 'report', 'stop' ] }
+##
+# @WatchdogExpirationAction
+#
+# An enumeration of the actions taken when the watchdog device's timer is
+# expired
+#
+# @reset: system resets
+#
+# @shutdown: system shutdown, note that it is similar to @powerdown, which
+# tries to set to system status and notify guest
+#
+# @poweroff: system poweroff, the emulator program exits
+#
+# @pause: system pauses, similar to @stop
+#
+# @debug: system enters debug state
+#
+# @none: nothing is done
+#
+# Since: 2.1
+##
+{ 'enum': 'WatchdogExpirationAction',
+ 'data': [ 'reset', 'shutdown', 'poweroff', 'pause', 'debug', 'none' ] }
+
{ 'include': 'qapi-event.json' }
--
1.7.1
^ permalink raw reply related [flat|nested] 77+ messages in thread* Re: [Qemu-devel] [PATCH V6 17/29] qapi event: convert WATCHDOG
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 17/29] qapi event: convert WATCHDOG Wenchao Xia
@ 2014-06-13 21:47 ` Eric Blake
2014-06-13 22:05 ` Eric Blake
2014-06-17 9:23 ` Paolo Bonzini
0 siblings, 2 replies; 77+ messages in thread
From: Eric Blake @ 2014-06-13 21:47 UTC (permalink / raw)
To: Wenchao Xia, qemu-devel; +Cc: mdroth, armbru, lcapitulino
[-- Attachment #1: Type: text/plain, Size: 2930 bytes --]
On 06/05/2014 06:22 AM, Wenchao Xia wrote:
> Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
> ---
> docs/qmp/qmp-events.txt | 19 -------------------
> hw/watchdog/watchdog.c | 23 +++++++----------------
> monitor.c | 2 +-
> qapi-event.json | 15 +++++++++++++++
> qapi-schema.json | 24 ++++++++++++++++++++++++
> 5 files changed, 47 insertions(+), 36 deletions(-)
>
> @@ -117,31 +108,31 @@ void watchdog_perform_action(void)
> {
> switch(watchdog_action) {
Worth fixing the missing space after switch while touching this area of
code?
> case WDT_RESET: /* same as 'system_reset' in monitor */
> - watchdog_mon_event("reset");
> + qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_RESET, NULL);
More instances of ignoring the errp argument, where eliminating it might
be nicer.
> +##
> +# @WATCHDOG
> +#
> +# Emitted when the watchdog device's timer is expired
> +#
> +# @action: action that has been taken
> +#
> +# Note: If action is "reset", "shutdown", or "pause" the WATCHDOG event is
> +# followed respectively by the RESET, SHUTDOWN, or STOP events
> +#
> +# Since: 2.1
0.14.0
> + 'data': { 'action': 'WatchdogExpirationAction' } }
Hmm. You've managed to create error.json in such a manner that it is
not self-sufficient. If some other file includes error.json, it must
also define WatchdogExpirationAction or it will fail the generators.
> +++ b/qapi-schema.json
> +##
> +# @WatchdogExpirationAction
I think you will be better off to ensure that error.json is
self-sufficient, perhaps by sticking any data type it references
directly into common.json rather than qapi-schema.json, and having
error.json include common.json. (This is the first instance of
referencing an external type, but other events later in the series have
the same issue).
> +# Since: 2.1
> +##
> +{ 'enum': 'WatchdogExpirationAction',
> + 'data': [ 'reset', 'shutdown', 'poweroff', 'pause', 'debug', 'none' ] }
Nice conversion of open-coded string to an enum. While I've been asking
for the earliest QMP version for Since fields on event objects proper,
here, I think you're okay keeping the 'since 2.1' indication. Why?
Because we already have other examples in the code base of converting
open-coded strings to an enum, where the QMP wire format is the same,
but where the version claimed on those enums was the qemu version that
did the conversion rather than the age of the command being converted.
(Maybe we could audit all of those conversions and retroactively update
their Since field, or even come up with a notation for wire-stability
release vs. QAPI introspection release - but that sounds like more pain
than necessary with no obvious benefit)
--
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: 604 bytes --]
^ permalink raw reply [flat|nested] 77+ messages in thread* Re: [Qemu-devel] [PATCH V6 17/29] qapi event: convert WATCHDOG
2014-06-13 21:47 ` Eric Blake
@ 2014-06-13 22:05 ` Eric Blake
2014-06-15 0:45 ` Wenchao Xia
2014-06-17 9:23 ` Paolo Bonzini
1 sibling, 1 reply; 77+ messages in thread
From: Eric Blake @ 2014-06-13 22:05 UTC (permalink / raw)
To: Wenchao Xia, qemu-devel; +Cc: lcapitulino, mdroth, armbru
[-- Attachment #1: Type: text/plain, Size: 2851 bytes --]
On 06/13/2014 03:47 PM, Eric Blake wrote:
> On 06/05/2014 06:22 AM, Wenchao Xia wrote:
>> Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
>> ---
>> docs/qmp/qmp-events.txt | 19 -------------------
>> hw/watchdog/watchdog.c | 23 +++++++----------------
>> monitor.c | 2 +-
>> qapi-event.json | 15 +++++++++++++++
>> qapi-schema.json | 24 ++++++++++++++++++++++++
>> 5 files changed, 47 insertions(+), 36 deletions(-)
>>
>
>> + 'data': { 'action': 'WatchdogExpirationAction' } }
>
> Hmm. You've managed to create error.json in such a manner that it is
> not self-sufficient. If some other file includes error.json, it must
> also define WatchdogExpirationAction or it will fail the generators.
s/error.json/qapi-event.json/
>
>> +++ b/qapi-schema.json
>
>> +##
>> +# @WatchdogExpirationAction
>
> I think you will be better off to ensure that error.json is
and again qapi-event.json (not sure why I typed error.json).
> self-sufficient, perhaps by sticking any data type it references
> directly into common.json rather than qapi-schema.json, and having
> error.json include common.json. (This is the first instance of
> referencing an external type, but other events later in the series have
> the same issue).
Oh weird! I've managed to run all four of
scripts/qapi-{visit,types,commands,event}.py directly on
qapi-event.json, and didn't get any complaints from the generator. BUT,
the generated code is definitely different:
-void qapi_event_send_watchdog(WatchdogExpirationAction action,
+void qapi_event_send_watchdog(WatchdogExpirationAction * action,
Error **errp)
That is, when the enum type is known (because the parse was done on
qapi-schema.json), the WatchdogExpirationAction argument is treated as
an integer enum value; but when the enum type is unknown (because the
parse was done directly on an incomplete qapi-event.json), the generator
tries to treat it as a pointer to an otherwise unknown structure.
(Never mind the odd formatting of the space after the '*' - I believe
this pending patch fixes it:
https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg02385.html).
This little exercise raises red flags for me - we probably ought to
enhance the code generators to error out instead of blindly act as if
unknown types are pointers. But save it for another day - no need to
stall this series just to wait for a robustness improvement to the
generators.
Meanwhile, my suggestion of making qapi-event.json to be self-sufficient
is going to be a bit harder to test, but is still probably worth trying
(just moving common types into a common shared include).
--
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: 604 bytes --]
^ permalink raw reply [flat|nested] 77+ messages in thread* Re: [Qemu-devel] [PATCH V6 17/29] qapi event: convert WATCHDOG
2014-06-13 22:05 ` Eric Blake
@ 2014-06-15 0:45 ` Wenchao Xia
0 siblings, 0 replies; 77+ messages in thread
From: Wenchao Xia @ 2014-06-15 0:45 UTC (permalink / raw)
To: Eric Blake, qemu-devel; +Cc: lcapitulino, mdroth, armbru
于 2014/6/14 6:05, Eric Blake 写道:
> On 06/13/2014 03:47 PM, Eric Blake wrote:
>> On 06/05/2014 06:22 AM, Wenchao Xia wrote:
>>> Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
>>> ---
>>> docs/qmp/qmp-events.txt | 19 -------------------
>>> hw/watchdog/watchdog.c | 23 +++++++----------------
>>> monitor.c | 2 +-
>>> qapi-event.json | 15 +++++++++++++++
>>> qapi-schema.json | 24 ++++++++++++++++++++++++
>>> 5 files changed, 47 insertions(+), 36 deletions(-)
>>>
>
>>
>>> + 'data': { 'action': 'WatchdogExpirationAction' } }
>>
>> Hmm. You've managed to create error.json in such a manner that it is
>> not self-sufficient. If some other file includes error.json, it must
>> also define WatchdogExpirationAction or it will fail the generators.
>
> s/error.json/qapi-event.json/
>
>>
>>> +++ b/qapi-schema.json
>>
>>> +##
>>> +# @WatchdogExpirationAction
>>
>> I think you will be better off to ensure that error.json is
>
> and again qapi-event.json (not sure why I typed error.json).
>
>> self-sufficient, perhaps by sticking any data type it references
>> directly into common.json rather than qapi-schema.json, and having
>> error.json include common.json. (This is the first instance of
>> referencing an external type, but other events later in the series have
>> the same issue).
>
> Oh weird! I've managed to run all four of
> scripts/qapi-{visit,types,commands,event}.py directly on
> qapi-event.json, and didn't get any complaints from the generator. BUT,
> the generated code is definitely different:
>
> -void qapi_event_send_watchdog(WatchdogExpirationAction action,
> +void qapi_event_send_watchdog(WatchdogExpirationAction * action,
> Error **errp)
>
> That is, when the enum type is known (because the parse was done on
> qapi-schema.json), the WatchdogExpirationAction argument is treated as
> an integer enum value; but when the enum type is unknown (because the
> parse was done directly on an incomplete qapi-event.json), the generator
> tries to treat it as a pointer to an otherwise unknown structure.
> (Never mind the odd formatting of the space after the '*' - I believe
> this pending patch fixes it:
> https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg02385.html).
>
> This little exercise raises red flags for me - we probably ought to
> enhance the code generators to error out instead of blindly act as if
> unknown types are pointers. But save it for another day - no need to
> stall this series just to wait for a robustness improvement to the
> generators.
>
> Meanwhile, my suggestion of making qapi-event.json to be self-sufficient
> is going to be a bit harder to test, but is still probably worth trying
> (just moving common types into a common shared include).
>
I think it is a issue about how to orgnize the .json files. There are
some common type defines needed for different .json files, in my series
they are needed both in qapi-schema.json and qapi-event.json. So to
make qapi-event.json self-sufficient, qapi-schema.json will be
insufficient. I considered this before, and thought it is better
to reorgnize .json files as:
qapi-types.json
| |
qapi-cmd.json qapi-event.json
It is an adjusting work for existing code, So I didn't do that in my
series.
^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [Qemu-devel] [PATCH V6 17/29] qapi event: convert WATCHDOG
2014-06-13 21:47 ` Eric Blake
2014-06-13 22:05 ` Eric Blake
@ 2014-06-17 9:23 ` Paolo Bonzini
2014-06-17 13:21 ` Eric Blake
1 sibling, 1 reply; 77+ messages in thread
From: Paolo Bonzini @ 2014-06-17 9:23 UTC (permalink / raw)
To: Eric Blake, Wenchao Xia, qemu-devel; +Cc: lcapitulino, mdroth, armbru
Il 13/06/2014 23:47, Eric Blake ha scritto:
>> +##
>> > +# @WatchdogExpirationAction
> I think you will be better off to ensure that error.json is
> self-sufficient, perhaps by sticking any data type it references
> directly into common.json rather than qapi-schema.json, and having
> error.json include common.json. (This is the first instance of
> referencing an external type, but other events later in the series have
> the same issue).
>
Can be done later though, I think?
Paolo
^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [Qemu-devel] [PATCH V6 17/29] qapi event: convert WATCHDOG
2014-06-17 9:23 ` Paolo Bonzini
@ 2014-06-17 13:21 ` Eric Blake
0 siblings, 0 replies; 77+ messages in thread
From: Eric Blake @ 2014-06-17 13:21 UTC (permalink / raw)
To: Paolo Bonzini, Wenchao Xia, qemu-devel; +Cc: lcapitulino, mdroth, armbru
[-- Attachment #1: Type: text/plain, Size: 1072 bytes --]
On 06/17/2014 03:23 AM, Paolo Bonzini wrote:
> Il 13/06/2014 23:47, Eric Blake ha scritto:
>>> +##
>>> > +# @WatchdogExpirationAction
>> I think you will be better off to ensure that error.json is
>> self-sufficient, perhaps by sticking any data type it references
>> directly into common.json rather than qapi-schema.json, and having
>> error.json include common.json. (This is the first instance of
>> referencing an external type, but other events later in the series have
>> the same issue).
>>
>
> Can be done later though, I think?
Yes; as I mentioned in my other mail:
https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg03208.html
This little exercise raises red flags for me - we probably ought to
enhance the code generators to error out instead of blindly act as if
unknown types are pointers. But save it for another day - no need to
stall this series just to wait for a robustness improvement to the
generators.
--
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: 604 bytes --]
^ permalink raw reply [flat|nested] 77+ messages in thread
* [Qemu-devel] [PATCH V6 18/29] qapi event: convert DEVICE_DELETED
2014-06-05 12:21 [Qemu-devel] [PATCH V6 00/29] add direct support of event in qapi schema Wenchao Xia
` (16 preceding siblings ...)
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 17/29] qapi event: convert WATCHDOG Wenchao Xia
@ 2014-06-05 12:22 ` Wenchao Xia
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 19/29] qapi event: convert DEVICE_TRAY_MOVED Wenchao Xia
` (11 subsequent siblings)
29 siblings, 0 replies; 77+ messages in thread
From: Wenchao Xia @ 2014-06-05 12:22 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, armbru, Wenchao Xia, lcapitulino
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
---
docs/qmp/qmp-events.txt | 18 ------------------
hw/core/qdev.c | 12 ++----------
qapi-event.json | 16 ++++++++++++++++
3 files changed, 18 insertions(+), 28 deletions(-)
diff --git a/docs/qmp/qmp-events.txt b/docs/qmp/qmp-events.txt
index df15dc8..fda68df 100644
--- a/docs/qmp/qmp-events.txt
+++ b/docs/qmp/qmp-events.txt
@@ -158,24 +158,6 @@ Example:
Note: The "ready to complete" status is always reset by a BLOCK_JOB_ERROR
event.
-DEVICE_DELETED
---------------
-
-Emitted whenever the device removal completion is acknowledged
-by the guest.
-At this point, it's safe to reuse the specified device ID.
-Device removal can be initiated by the guest or by HMP/QMP commands.
-
-Data:
-
-- "device": device name (json-string, optional)
-- "path": device path (json-string)
-
-{ "event": "DEVICE_DELETED",
- "data": { "device": "virtio-net-pci-0",
- "path": "/machine/peripheral/virtio-net-pci-0" },
- "timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
-
DEVICE_TRAY_MOVED
-----------------
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index e65a5aa..a6c62b2 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -32,8 +32,8 @@
#include "qapi/qmp/qerror.h"
#include "qapi/visitor.h"
#include "qapi/qmp/qjson.h"
-#include "monitor/monitor.h"
#include "hw/hotplug.h"
+#include "qapi-event.h"
int qdev_hotplug = 0;
static bool qdev_hot_added = false;
@@ -939,7 +939,6 @@ static void device_unparent(Object *obj)
{
DeviceState *dev = DEVICE(obj);
BusState *bus;
- QObject *event_data;
bool have_realized = dev->realized;
if (dev->realized) {
@@ -959,14 +958,7 @@ static void device_unparent(Object *obj)
if (have_realized) {
gchar *path = object_get_canonical_path(OBJECT(dev));
- if (dev->id) {
- event_data = qobject_from_jsonf("{ 'device': %s, 'path': %s }",
- dev->id, path);
- } else {
- event_data = qobject_from_jsonf("{ 'path': %s }", path);
- }
- monitor_protocol_event(QEVENT_DEVICE_DELETED, event_data);
- qobject_decref(event_data);
+ qapi_event_send_device_deleted(!!dev->id, dev->id, path, NULL);
g_free(path);
}
}
diff --git a/qapi-event.json b/qapi-event.json
index 640f841..fc83fed 100644
--- a/qapi-event.json
+++ b/qapi-event.json
@@ -106,3 +106,19 @@
##
{ 'event': 'WATCHDOG',
'data': { 'action': 'WatchdogExpirationAction' } }
+
+##
+# @DEVICE_DELETED
+#
+# Emitted whenever the device removal completion is acknowledged by the guest.
+# At this point, it's safe to reuse the specified device ID. Device removal can
+# be initiated by the guest or by HMP/QMP commands.
+#
+# @device: #optional, device name
+#
+# @path: device path
+#
+# Since: 2.1
+##
+{ 'event': 'DEVICE_DELETED',
+ 'data': { '*device': 'str', 'path': 'str' } }
--
1.7.1
^ permalink raw reply related [flat|nested] 77+ messages in thread* [Qemu-devel] [PATCH V6 19/29] qapi event: convert DEVICE_TRAY_MOVED
2014-06-05 12:21 [Qemu-devel] [PATCH V6 00/29] add direct support of event in qapi schema Wenchao Xia
` (17 preceding siblings ...)
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 18/29] qapi event: convert DEVICE_DELETED Wenchao Xia
@ 2014-06-05 12:22 ` Wenchao Xia
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 20/29] qapi event: convert BLOCK_IO_ERROR and BLOCK_JOB_ERROR Wenchao Xia
` (10 subsequent siblings)
29 siblings, 0 replies; 77+ messages in thread
From: Wenchao Xia @ 2014-06-05 12:22 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, armbru, Wenchao Xia, lcapitulino
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
---
block.c | 21 +++++++--------------
docs/qmp/qmp-events.txt | 18 ------------------
qapi-event.json | 15 +++++++++++++++
3 files changed, 22 insertions(+), 32 deletions(-)
diff --git a/block.c b/block.c
index 84ad945..52f4580 100644
--- a/block.c
+++ b/block.c
@@ -35,6 +35,7 @@
#include "block/qapi.h"
#include "qmp-commands.h"
#include "qemu/timer.h"
+#include "qapi-event.h"
#ifdef CONFIG_BSD
#include <sys/types.h>
@@ -2162,17 +2163,6 @@ void bdrv_emit_qmp_error_event(const BlockDriverState *bdrv,
qobject_decref(data);
}
-static void bdrv_emit_qmp_eject_event(BlockDriverState *bs, bool ejected)
-{
- QObject *data;
-
- data = qobject_from_jsonf("{ 'device': %s, 'tray-open': %i }",
- bdrv_get_device_name(bs), ejected);
- monitor_protocol_event(QEVENT_DEVICE_TRAY_MOVED, data);
-
- qobject_decref(data);
-}
-
static void bdrv_dev_change_media_cb(BlockDriverState *bs, bool load)
{
if (bs->dev_ops && bs->dev_ops->change_media_cb) {
@@ -2180,11 +2170,13 @@ static void bdrv_dev_change_media_cb(BlockDriverState *bs, bool load)
bs->dev_ops->change_media_cb(bs->dev_opaque, load);
if (tray_was_closed) {
/* tray open */
- bdrv_emit_qmp_eject_event(bs, true);
+ qapi_event_send_device_tray_moved(bdrv_get_device_name(bs),
+ true, NULL);
}
if (load) {
/* tray close */
- bdrv_emit_qmp_eject_event(bs, false);
+ qapi_event_send_device_tray_moved(bdrv_get_device_name(bs),
+ false, NULL);
}
}
}
@@ -5173,7 +5165,8 @@ void bdrv_eject(BlockDriverState *bs, bool eject_flag)
}
if (bs->device_name[0] != '\0') {
- bdrv_emit_qmp_eject_event(bs, eject_flag);
+ qapi_event_send_device_tray_moved(bdrv_get_device_name(bs),
+ eject_flag, NULL);
}
}
diff --git a/docs/qmp/qmp-events.txt b/docs/qmp/qmp-events.txt
index fda68df..1ee6f53 100644
--- a/docs/qmp/qmp-events.txt
+++ b/docs/qmp/qmp-events.txt
@@ -158,24 +158,6 @@ Example:
Note: The "ready to complete" status is always reset by a BLOCK_JOB_ERROR
event.
-DEVICE_TRAY_MOVED
------------------
-
-It's emitted whenever the tray of a removable device is moved by the guest
-or by HMP/QMP commands.
-
-Data:
-
-- "device": device name (json-string)
-- "tray-open": true if the tray has been opened or false if it has been closed
- (json-bool)
-
-{ "event": "DEVICE_TRAY_MOVED",
- "data": { "device": "ide1-cd0",
- "tray-open": true
- },
- "timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
-
GUEST_PANICKED
--------------
diff --git a/qapi-event.json b/qapi-event.json
index fc83fed..c2c65f9 100644
--- a/qapi-event.json
+++ b/qapi-event.json
@@ -122,3 +122,18 @@
##
{ 'event': 'DEVICE_DELETED',
'data': { '*device': 'str', 'path': 'str' } }
+
+##
+# @DEVICE_TRAY_MOVED
+#
+# Emitted whenever the tray of a removable device is moved by the guest or by
+# HMP/QMP commands
+#
+# @device: device name
+#
+# @tray-open: true if the tray has been opened or false if it has been closed
+#
+# Since: 2.1
+##
+{ 'event': 'DEVICE_TRAY_MOVED',
+ 'data': { 'device': 'str', 'tray-open': 'bool' } }
--
1.7.1
^ permalink raw reply related [flat|nested] 77+ messages in thread* [Qemu-devel] [PATCH V6 20/29] qapi event: convert BLOCK_IO_ERROR and BLOCK_JOB_ERROR
2014-06-05 12:21 [Qemu-devel] [PATCH V6 00/29] add direct support of event in qapi schema Wenchao Xia
` (18 preceding siblings ...)
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 19/29] qapi event: convert DEVICE_TRAY_MOVED Wenchao Xia
@ 2014-06-05 12:22 ` Wenchao Xia
2014-06-05 12:22 ` [Qemu-devel] [PATCH V6 21/29] qapi event: convert BLOCK_IMAGE_CORRUPTED Wenchao Xia
` (9 subsequent siblings)
29 siblings, 0 replies; 77+ messages in thread
From: Wenchao Xia @ 2014-06-05 12:22 UTC (permalink / raw)
To: qemu-devel; +Cc: mdroth, armbru, Wenchao Xia, lcapitulino
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
---
block.c | 36 +++------------------------------
blockjob.c | 6 ++++-
docs/qmp/qmp-events.txt | 47 ---------------------------------------------
include/block/block_int.h | 3 --
qapi-event.json | 38 ++++++++++++++++++++++++++++++++++++
qapi-schema.json | 14 +++++++++++++
6 files changed, 61 insertions(+), 83 deletions(-)
diff --git a/block.c b/block.c
index 52f4580..1ebbce6 100644
--- a/block.c
+++ b/block.c
@@ -24,7 +24,6 @@
#include "config-host.h"
#include "qemu-common.h"
#include "trace.h"
-#include "monitor/monitor.h"
#include "block/block_int.h"
#include "block/blockjob.h"
#include "qemu/module.h"
@@ -2133,36 +2132,6 @@ void bdrv_set_dev_ops(BlockDriverState *bs, const BlockDevOps *ops,
bs->dev_opaque = opaque;
}
-void bdrv_emit_qmp_error_event(const BlockDriverState *bdrv,
- enum MonitorEvent ev,
- BlockErrorAction action, bool is_read)
-{
- QObject *data;
- const char *action_str;
-
- switch (action) {
- case BLOCK_ERROR_ACTION_REPORT:
- action_str = "report";
- break;
- case BLOCK_ERROR_ACTION_IGNORE:
- action_str = "ignore";
- break;
- case BLOCK_ERROR_ACTION_STOP:
- action_str = "stop";
- break;
- default:
- abort();
- }
-
- data = qobject_from_jsonf("{ 'device': %s, 'action': %s, 'operation': %s }",
- bdrv->device_name,
- action_str,
- is_read ? "read" : "write");
- monitor_protocol_event(ev, data);
-
- qobject_decref(data);
-}
-
static void bdrv_dev_change_media_cb(BlockDriverState *bs, bool load)
{
if (bs->dev_ops && bs->dev_ops->change_media_cb) {
@@ -3612,7 +3581,10 @@ void bdrv_error_action(BlockDriverState *bs, BlockErrorAction action,
bool is_read, int error)
{
assert(error >= 0);
- bdrv_emit_qmp_error_event(bs, QEVENT_BLOCK_IO_ERROR, action, is_read);
+ qapi_event_send_block_io_error(bdrv_get_device_name(bs),
+ is_read ? IO_OPERATION_TYPE_READ :
+ IO_OPERATION_TYPE_WRITE,
+ action, NULL);
if (action == BLOCK_ERROR_ACTION_STOP) {
vm_stop(RUN_STATE_IO_ERROR);
bdrv_iostatus_set_err(bs, error);
diff --git a/blockjob.c b/blockjob.c
index bc63d42..43f971d 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -34,6 +34,7 @@
#include "block/coroutine.h"
#include "qmp-commands.h"
#include "qemu/timer.h"
+#include "qapi-event.h"
void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
int64_t speed, BlockDriverCompletionFunc *cb,
@@ -277,7 +278,10 @@ BlockErrorAction block_job_error_action(BlockJob *job, BlockDriverState *bs,
default:
abort();
}
- bdrv_emit_qmp_error_event(job->bs, QEVENT_BLOCK_JOB_ERROR, action, is_read);
+ qapi_event_send_block_job_error(bdrv_get_device_name(bs),
+ is_read ? IO_OPERATION_TYPE_READ :
+ IO_OPERATION_TYPE_WRITE,
+ action, NULL);
if (action == BLOCK_ERROR_ACTION_STOP) {
block_job_pause(job);
block_job_iostatus_set_err(job, error);
diff --git a/docs/qmp/qmp-events.txt b/docs/qmp/qmp-events.txt
index 1ee6f53..f67a9ec 100644
--- a/docs/qmp/qmp-events.txt
+++ b/docs/qmp/qmp-events.txt
@@ -40,31 +40,6 @@ Example:
"size": 65536 },
"timestamp": { "seconds": 1378126126, "microseconds": 966463 } }
-BLOCK_IO_ERROR
---------------
-
-Emitted when a disk I/O error occurs.
-
-Data:
-
-- "device": device name (json-string)
-- "operation": I/O operation (json-string, "read" or "write")
-- "action": action that has been taken, it's one of the following (json-string):
- "ignore": error has been ignored
- "report"