* [Qemu-devel] [PULL v2 01/14] qobject: Drop QObject_HEAD
2015-10-28 8:15 [Qemu-devel] [PULL v2 00/14] QMP and QObject patches Markus Armbruster
@ 2015-10-28 8:15 ` Markus Armbruster
2015-10-28 8:15 ` [Qemu-devel] [PULL v2 02/14] qbool: Make conversion from QObject * accept null Markus Armbruster
` (13 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2015-10-28 8:15 UTC (permalink / raw)
To: qemu-devel
QObject_HEAD is a macro expanding into the common part of structs that
are sub-types of QObject. It's always been just QObject base, and
unlikely to change. Drop the macro, because the code is clearer with
out it.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1444918537-18107-2-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
---
include/qapi/qmp/qbool.h | 2 +-
include/qapi/qmp/qdict.h | 2 +-
include/qapi/qmp/qfloat.h | 2 +-
include/qapi/qmp/qint.h | 2 +-
include/qapi/qmp/qlist.h | 2 +-
include/qapi/qmp/qobject.h | 4 ----
include/qapi/qmp/qstring.h | 2 +-
7 files changed, 6 insertions(+), 10 deletions(-)
diff --git a/include/qapi/qmp/qbool.h b/include/qapi/qmp/qbool.h
index 4aa6be3..d9256e4 100644
--- a/include/qapi/qmp/qbool.h
+++ b/include/qapi/qmp/qbool.h
@@ -18,7 +18,7 @@
#include "qapi/qmp/qobject.h"
typedef struct QBool {
- QObject_HEAD;
+ QObject base;
bool value;
} QBool;
diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
index a37f4c1..787c658 100644
--- a/include/qapi/qmp/qdict.h
+++ b/include/qapi/qmp/qdict.h
@@ -28,7 +28,7 @@ typedef struct QDictEntry {
} QDictEntry;
typedef struct QDict {
- QObject_HEAD;
+ QObject base;
size_t size;
QLIST_HEAD(,QDictEntry) table[QDICT_BUCKET_MAX];
} QDict;
diff --git a/include/qapi/qmp/qfloat.h b/include/qapi/qmp/qfloat.h
index a865844..46745e5 100644
--- a/include/qapi/qmp/qfloat.h
+++ b/include/qapi/qmp/qfloat.h
@@ -18,7 +18,7 @@
#include "qapi/qmp/qobject.h"
typedef struct QFloat {
- QObject_HEAD;
+ QObject base;
double value;
} QFloat;
diff --git a/include/qapi/qmp/qint.h b/include/qapi/qmp/qint.h
index 48a41b0..339a9ab 100644
--- a/include/qapi/qmp/qint.h
+++ b/include/qapi/qmp/qint.h
@@ -17,7 +17,7 @@
#include "qapi/qmp/qobject.h"
typedef struct QInt {
- QObject_HEAD;
+ QObject base;
int64_t value;
} QInt;
diff --git a/include/qapi/qmp/qlist.h b/include/qapi/qmp/qlist.h
index 6cc4831..b1bf785 100644
--- a/include/qapi/qmp/qlist.h
+++ b/include/qapi/qmp/qlist.h
@@ -22,7 +22,7 @@ typedef struct QListEntry {
} QListEntry;
typedef struct QList {
- QObject_HEAD;
+ QObject base;
QTAILQ_HEAD(,QListEntry) head;
} QList;
diff --git a/include/qapi/qmp/qobject.h b/include/qapi/qmp/qobject.h
index 260d2ed..c856f55 100644
--- a/include/qapi/qmp/qobject.h
+++ b/include/qapi/qmp/qobject.h
@@ -59,10 +59,6 @@ typedef struct QObject {
size_t refcnt;
} QObject;
-/* Objects definitions must include this */
-#define QObject_HEAD \
- QObject base
-
/* Get the 'base' part of an object */
#define QOBJECT(obj) (&(obj)->base)
diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h
index 1bc3666..34675a7 100644
--- a/include/qapi/qmp/qstring.h
+++ b/include/qapi/qmp/qstring.h
@@ -17,7 +17,7 @@
#include "qapi/qmp/qobject.h"
typedef struct QString {
- QObject_HEAD;
+ QObject base;
char *string;
size_t length;
size_t capacity;
--
2.4.3
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PULL v2 02/14] qbool: Make conversion from QObject * accept null
2015-10-28 8:15 [Qemu-devel] [PULL v2 00/14] QMP and QObject patches Markus Armbruster
2015-10-28 8:15 ` [Qemu-devel] [PULL v2 01/14] qobject: Drop QObject_HEAD Markus Armbruster
@ 2015-10-28 8:15 ` Markus Armbruster
2015-10-28 8:15 ` [Qemu-devel] [PULL v2 03/14] qdict: " Markus Armbruster
` (12 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2015-10-28 8:15 UTC (permalink / raw)
To: qemu-devel
qobject_to_qbool() crashes on null, which is a trap for the unwary.
Return null instead, and simplify a few callers.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1444918537-18107-3-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
---
qapi/qmp-input-visitor.c | 6 +++---
qobject/qbool.c | 4 ++--
qobject/qdict.c | 11 +++--------
3 files changed, 8 insertions(+), 13 deletions(-)
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 5dd9ed5..f32ce81 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -240,15 +240,15 @@ static void qmp_input_type_bool(Visitor *v, bool *obj, const char *name,
Error **errp)
{
QmpInputVisitor *qiv = to_qiv(v);
- QObject *qobj = qmp_input_get_object(qiv, name, true);
+ QBool *qbool = qobject_to_qbool(qmp_input_get_object(qiv, name, true));
- if (!qobj || qobject_type(qobj) != QTYPE_QBOOL) {
+ if (!qbool) {
error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
"boolean");
return;
}
- *obj = qbool_get_bool(qobject_to_qbool(qobj));
+ *obj = qbool_get_bool(qbool);
}
static void qmp_input_type_str(Visitor *v, char **obj, const char *name,
diff --git a/qobject/qbool.c b/qobject/qbool.c
index 5ff69f0..bc6535f 100644
--- a/qobject/qbool.c
+++ b/qobject/qbool.c
@@ -51,9 +51,9 @@ bool qbool_get_bool(const QBool *qb)
*/
QBool *qobject_to_qbool(const QObject *obj)
{
- if (qobject_type(obj) != QTYPE_QBOOL)
+ if (!obj || qobject_type(obj) != QTYPE_QBOOL) {
return NULL;
-
+ }
return container_of(obj, QBool, base);
}
diff --git a/qobject/qdict.c b/qobject/qdict.c
index 67b1a58..f179f4e 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -243,8 +243,7 @@ int64_t qdict_get_int(const QDict *qdict, const char *key)
*/
bool qdict_get_bool(const QDict *qdict, const char *key)
{
- QObject *obj = qdict_get_obj(qdict, key, QTYPE_QBOOL);
- return qbool_get_bool(qobject_to_qbool(obj));
+ return qbool_get_bool(qobject_to_qbool(qdict_get(qdict, key)));
}
/**
@@ -316,13 +315,9 @@ int64_t qdict_get_try_int(const QDict *qdict, const char *key,
*/
bool qdict_get_try_bool(const QDict *qdict, const char *key, bool def_value)
{
- QObject *obj;
+ QBool *qbool = qobject_to_qbool(qdict_get(qdict, key));
- obj = qdict_get(qdict, key);
- if (!obj || qobject_type(obj) != QTYPE_QBOOL)
- return def_value;
-
- return qbool_get_bool(qobject_to_qbool(obj));
+ return qbool ? qbool_get_bool(qbool) : def_value;
}
/**
--
2.4.3
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PULL v2 03/14] qdict: Make conversion from QObject * accept null
2015-10-28 8:15 [Qemu-devel] [PULL v2 00/14] QMP and QObject patches Markus Armbruster
2015-10-28 8:15 ` [Qemu-devel] [PULL v2 01/14] qobject: Drop QObject_HEAD Markus Armbruster
2015-10-28 8:15 ` [Qemu-devel] [PULL v2 02/14] qbool: Make conversion from QObject * accept null Markus Armbruster
@ 2015-10-28 8:15 ` Markus Armbruster
2015-10-28 8:15 ` [Qemu-devel] [PULL v2 04/14] qfloat qint: " Markus Armbruster
` (11 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2015-10-28 8:15 UTC (permalink / raw)
To: qemu-devel
qobject_to_qdict() crashes on null, which is a trap for the unwary.
Return null instead, and simplify a few callers.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1444918537-18107-4-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
---
qga/main.c | 11 +++--------
qobject/qdict.c | 6 +++---
2 files changed, 6 insertions(+), 11 deletions(-)
diff --git a/qga/main.c b/qga/main.c
index 068169f..d2a0ffc 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -573,7 +573,6 @@ static void process_command(GAState *s, QDict *req)
static void process_event(JSONMessageParser *parser, QList *tokens)
{
GAState *s = container_of(parser, GAState, parser);
- QObject *obj;
QDict *qdict;
Error *err = NULL;
int ret;
@@ -581,9 +580,9 @@ static void process_event(JSONMessageParser *parser, QList *tokens)
g_assert(s && parser);
g_debug("process_event: called");
- obj = json_parser_parse_err(tokens, NULL, &err);
- if (err || !obj || qobject_type(obj) != QTYPE_QDICT) {
- qobject_decref(obj);
+ qdict = qobject_to_qdict(json_parser_parse_err(tokens, NULL, &err));
+ if (err || !qdict) {
+ QDECREF(qdict);
qdict = qdict_new();
if (!err) {
g_warning("failed to parse event: unknown error");
@@ -593,12 +592,8 @@ static void process_event(JSONMessageParser *parser, QList *tokens)
}
qdict_put_obj(qdict, "error", qmp_build_error_object(err));
error_free(err);
- } else {
- qdict = qobject_to_qdict(obj);
}
- g_assert(qdict);
-
/* handle host->guest commands */
if (qdict_haskey(qdict, "execute")) {
process_command(s, qdict);
diff --git a/qobject/qdict.c b/qobject/qdict.c
index f179f4e..6b32285 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -46,9 +46,9 @@ QDict *qdict_new(void)
*/
QDict *qobject_to_qdict(const QObject *obj)
{
- if (qobject_type(obj) != QTYPE_QDICT)
+ if (!obj || qobject_type(obj) != QTYPE_QDICT) {
return NULL;
-
+ }
return container_of(obj, QDict, base);
}
@@ -269,7 +269,7 @@ QList *qdict_get_qlist(const QDict *qdict, const char *key)
*/
QDict *qdict_get_qdict(const QDict *qdict, const char *key)
{
- return qobject_to_qdict(qdict_get_obj(qdict, key, QTYPE_QDICT));
+ return qobject_to_qdict(qdict_get(qdict, key));
}
/**
--
2.4.3
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PULL v2 04/14] qfloat qint: Make conversion from QObject * accept null
2015-10-28 8:15 [Qemu-devel] [PULL v2 00/14] QMP and QObject patches Markus Armbruster
` (2 preceding siblings ...)
2015-10-28 8:15 ` [Qemu-devel] [PULL v2 03/14] qdict: " Markus Armbruster
@ 2015-10-28 8:15 ` Markus Armbruster
2015-10-28 8:15 ` [Qemu-devel] [PULL v2 05/14] qlist: " Markus Armbruster
` (10 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2015-10-28 8:15 UTC (permalink / raw)
To: qemu-devel
qobject_to_qfloat() and qobject_to_qint() crash on null, which is a
trap for the unwary. Return null instead, and simplify a few callers.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1444918537-18107-5-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
---
qapi/qmp-input-visitor.c | 28 ++++++++++++++++------------
qobject/qdict.c | 11 +++--------
qobject/qfloat.c | 4 ++--
qobject/qint.c | 4 ++--
4 files changed, 23 insertions(+), 24 deletions(-)
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index f32ce81..267783c 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -225,15 +225,15 @@ static void qmp_input_type_int(Visitor *v, int64_t *obj, const char *name,
Error **errp)
{
QmpInputVisitor *qiv = to_qiv(v);
- QObject *qobj = qmp_input_get_object(qiv, name, true);
+ QInt *qint = qobject_to_qint(qmp_input_get_object(qiv, name, true));
- if (!qobj || qobject_type(qobj) != QTYPE_QINT) {
+ if (!qint) {
error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
"integer");
return;
}
- *obj = qint_get_int(qobject_to_qint(qobj));
+ *obj = qint_get_int(qint);
}
static void qmp_input_type_bool(Visitor *v, bool *obj, const char *name,
@@ -271,19 +271,23 @@ static void qmp_input_type_number(Visitor *v, double *obj, const char *name,
{
QmpInputVisitor *qiv = to_qiv(v);
QObject *qobj = qmp_input_get_object(qiv, name, true);
+ QInt *qint;
+ QFloat *qfloat;
- if (!qobj || (qobject_type(qobj) != QTYPE_QFLOAT &&
- qobject_type(qobj) != QTYPE_QINT)) {
- error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
- "number");
- return;
- }
-
- if (qobject_type(qobj) == QTYPE_QINT) {
+ qint = qobject_to_qint(qobj);
+ if (qint) {
*obj = qint_get_int(qobject_to_qint(qobj));
- } else {
+ return;
+ }
+
+ qfloat = qobject_to_qfloat(qobj);
+ if (qfloat) {
*obj = qfloat_get_double(qobject_to_qfloat(qobj));
+ return;
}
+
+ error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
+ "number");
}
static void qmp_input_type_any(Visitor *v, QObject **obj, const char *name,
diff --git a/qobject/qdict.c b/qobject/qdict.c
index 6b32285..97e881b 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -229,8 +229,7 @@ double qdict_get_double(const QDict *qdict, const char *key)
*/
int64_t qdict_get_int(const QDict *qdict, const char *key)
{
- QObject *obj = qdict_get_obj(qdict, key, QTYPE_QINT);
- return qint_get_int(qobject_to_qint(obj));
+ return qint_get_int(qobject_to_qint(qdict_get(qdict, key)));
}
/**
@@ -297,13 +296,9 @@ const char *qdict_get_str(const QDict *qdict, const char *key)
int64_t qdict_get_try_int(const QDict *qdict, const char *key,
int64_t def_value)
{
- QObject *obj;
+ QInt *qint = qobject_to_qint(qdict_get(qdict, key));
- obj = qdict_get(qdict, key);
- if (!obj || qobject_type(obj) != QTYPE_QINT)
- return def_value;
-
- return qint_get_int(qobject_to_qint(obj));
+ return qint ? qint_get_int(qint) : def_value;
}
/**
diff --git a/qobject/qfloat.c b/qobject/qfloat.c
index 7de0992..c865163 100644
--- a/qobject/qfloat.c
+++ b/qobject/qfloat.c
@@ -51,9 +51,9 @@ double qfloat_get_double(const QFloat *qf)
*/
QFloat *qobject_to_qfloat(const QObject *obj)
{
- if (qobject_type(obj) != QTYPE_QFLOAT)
+ if (!obj || qobject_type(obj) != QTYPE_QFLOAT) {
return NULL;
-
+ }
return container_of(obj, QFloat, base);
}
diff --git a/qobject/qint.c b/qobject/qint.c
index 86b9b04..999688e 100644
--- a/qobject/qint.c
+++ b/qobject/qint.c
@@ -50,9 +50,9 @@ int64_t qint_get_int(const QInt *qi)
*/
QInt *qobject_to_qint(const QObject *obj)
{
- if (qobject_type(obj) != QTYPE_QINT)
+ if (!obj || qobject_type(obj) != QTYPE_QINT) {
return NULL;
-
+ }
return container_of(obj, QInt, base);
}
--
2.4.3
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PULL v2 05/14] qlist: Make conversion from QObject * accept null
2015-10-28 8:15 [Qemu-devel] [PULL v2 00/14] QMP and QObject patches Markus Armbruster
` (3 preceding siblings ...)
2015-10-28 8:15 ` [Qemu-devel] [PULL v2 04/14] qfloat qint: " Markus Armbruster
@ 2015-10-28 8:15 ` Markus Armbruster
2015-10-28 8:15 ` [Qemu-devel] [PULL v2 06/14] qstring: " Markus Armbruster
` (9 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2015-10-28 8:15 UTC (permalink / raw)
To: qemu-devel
qobject_to_qlist() crashes on null, which is a trap for the unwary.
Return null instead.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1444918537-18107-6-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
---
qobject/qlist.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/qobject/qlist.c b/qobject/qlist.c
index 1ced0de..298003a 100644
--- a/qobject/qlist.c
+++ b/qobject/qlist.c
@@ -142,10 +142,9 @@ size_t qlist_size(const QList *qlist)
*/
QList *qobject_to_qlist(const QObject *obj)
{
- if (qobject_type(obj) != QTYPE_QLIST) {
+ if (!obj || qobject_type(obj) != QTYPE_QLIST) {
return NULL;
}
-
return container_of(obj, QList, base);
}
--
2.4.3
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PULL v2 06/14] qstring: Make conversion from QObject * accept null
2015-10-28 8:15 [Qemu-devel] [PULL v2 00/14] QMP and QObject patches Markus Armbruster
` (4 preceding siblings ...)
2015-10-28 8:15 ` [Qemu-devel] [PULL v2 05/14] qlist: " Markus Armbruster
@ 2015-10-28 8:15 ` Markus Armbruster
2015-10-28 8:15 ` [Qemu-devel] [PULL v2 07/14] monitor: Reduce casting of QAPI event QDict Markus Armbruster
` (8 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2015-10-28 8:15 UTC (permalink / raw)
To: qemu-devel
qobject_to_qstring() crashes on null, which is a trap for the unwary.
Return null instead, and simplify a few callers.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1444918537-18107-7-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
---
qapi/qmp-input-visitor.c | 6 +++---
qobject/qdict.c | 11 +++--------
qobject/qstring.c | 4 ++--
3 files changed, 8 insertions(+), 13 deletions(-)
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 267783c..eb6e110 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -255,15 +255,15 @@ static void qmp_input_type_str(Visitor *v, char **obj, const char *name,
Error **errp)
{
QmpInputVisitor *qiv = to_qiv(v);
- QObject *qobj = qmp_input_get_object(qiv, name, true);
+ QString *qstr = qobject_to_qstring(qmp_input_get_object(qiv, name, true));
- if (!qobj || qobject_type(qobj) != QTYPE_QSTRING) {
+ if (!qstr) {
error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
"string");
return;
}
- *obj = g_strdup(qstring_get_str(qobject_to_qstring(qobj)));
+ *obj = g_strdup(qstring_get_str(qstr));
}
static void qmp_input_type_number(Visitor *v, double *obj, const char *name,
diff --git a/qobject/qdict.c b/qobject/qdict.c
index 97e881b..2d67bf1 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -282,8 +282,7 @@ QDict *qdict_get_qdict(const QDict *qdict, const char *key)
*/
const char *qdict_get_str(const QDict *qdict, const char *key)
{
- QObject *obj = qdict_get_obj(qdict, key, QTYPE_QSTRING);
- return qstring_get_str(qobject_to_qstring(obj));
+ return qstring_get_str(qobject_to_qstring(qdict_get(qdict, key)));
}
/**
@@ -325,13 +324,9 @@ bool qdict_get_try_bool(const QDict *qdict, const char *key, bool def_value)
*/
const char *qdict_get_try_str(const QDict *qdict, const char *key)
{
- QObject *obj;
+ QString *qstr = qobject_to_qstring(qdict_get(qdict, key));
- obj = qdict_get(qdict, key);
- if (!obj || qobject_type(obj) != QTYPE_QSTRING)
- return NULL;
-
- return qstring_get_str(qobject_to_qstring(obj));
+ return qstr ? qstring_get_str(qstr) : NULL;
}
/**
diff --git a/qobject/qstring.c b/qobject/qstring.c
index 607b7a1..cb72dfb 100644
--- a/qobject/qstring.c
+++ b/qobject/qstring.c
@@ -117,9 +117,9 @@ void qstring_append_chr(QString *qstring, int c)
*/
QString *qobject_to_qstring(const QObject *obj)
{
- if (qobject_type(obj) != QTYPE_QSTRING)
+ if (!obj || qobject_type(obj) != QTYPE_QSTRING) {
return NULL;
-
+ }
return container_of(obj, QString, base);
}
--
2.4.3
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PULL v2 07/14] monitor: Reduce casting of QAPI event QDict
2015-10-28 8:15 [Qemu-devel] [PULL v2 00/14] QMP and QObject patches Markus Armbruster
` (5 preceding siblings ...)
2015-10-28 8:15 ` [Qemu-devel] [PULL v2 06/14] qstring: " Markus Armbruster
@ 2015-10-28 8:15 ` Markus Armbruster
2015-10-28 8:15 ` [Qemu-devel] [PULL v2 08/14] monitor: Simplify event throttling Markus Armbruster
` (7 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2015-10-28 8:15 UTC (permalink / raw)
To: qemu-devel
Make the variables holding the event QDict instead of QObject.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1444921716-9511-2-git-send-email-armbru@redhat.com>
---
monitor.c | 38 +++++++++++++++++++-------------------
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/monitor.c b/monitor.c
index 301a143..f90a42f 100644
--- a/monitor.c
+++ b/monitor.c
@@ -185,7 +185,7 @@ typedef struct MonitorQAPIEventState {
int64_t rate; /* Minimum time (in ns) between two events */
int64_t last; /* QEMU_CLOCK_REALTIME value at last emission */
QEMUTimer *timer; /* Timer for handling delayed events */
- QObject *data; /* Event pending delayed dispatch */
+ QDict *qdict; /* Delayed event (if any) */
} MonitorQAPIEventState;
struct Monitor {
@@ -444,14 +444,14 @@ static MonitorQAPIEventState monitor_qapi_event_state[QAPI_EVENT_MAX];
* Emits the event to every monitor instance, @event is only used for trace
* Called with monitor_lock held.
*/
-static void monitor_qapi_event_emit(QAPIEvent event, QObject *data)
+static void monitor_qapi_event_emit(QAPIEvent event, QDict *qdict)
{
Monitor *mon;
- trace_monitor_protocol_event_emit(event, data);
+ trace_monitor_protocol_event_emit(event, qdict);
QLIST_FOREACH(mon, &mon_list, entry) {
if (monitor_is_qmp(mon) && mon->qmp.in_command_mode) {
- monitor_json_emitter(mon, data);
+ monitor_json_emitter(mon, QOBJECT(qdict));
}
}
}
@@ -461,7 +461,7 @@ static void monitor_qapi_event_emit(QAPIEvent event, QObject *data)
* applying any rate limiting if required.
*/
static void
-monitor_qapi_event_queue(QAPIEvent event, QDict *data, Error **errp)
+monitor_qapi_event_queue(QAPIEvent event, QDict *qdict, Error **errp)
{
MonitorQAPIEventState *evstate;
assert(event < QAPI_EVENT_MAX);
@@ -469,7 +469,7 @@ monitor_qapi_event_queue(QAPIEvent event, QDict *data, Error **errp)
evstate = &(monitor_qapi_event_state[event]);
trace_monitor_protocol_event_queue(event,
- data,
+ qdict,
evstate->rate,
evstate->last,
now);
@@ -477,26 +477,26 @@ monitor_qapi_event_queue(QAPIEvent event, QDict *data, Error **errp)
/* Rate limit of 0 indicates no throttling */
qemu_mutex_lock(&monitor_lock);
if (!evstate->rate) {
- monitor_qapi_event_emit(event, QOBJECT(data));
+ monitor_qapi_event_emit(event, qdict);
evstate->last = now;
} else {
int64_t delta = now - evstate->last;
- if (evstate->data ||
+ if (evstate->qdict ||
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);
+ if (evstate->qdict) {
+ QDECREF(evstate->qdict);
} else {
int64_t then = evstate->last + evstate->rate;
timer_mod_ns(evstate->timer, then);
}
- evstate->data = QOBJECT(data);
- qobject_incref(evstate->data);
+ evstate->qdict = qdict;
+ QINCREF(evstate->qdict);
} else {
- monitor_qapi_event_emit(event, QOBJECT(data));
+ monitor_qapi_event_emit(event, qdict);
evstate->last = now;
}
}
@@ -513,14 +513,14 @@ static void monitor_qapi_event_handler(void *opaque)
int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
trace_monitor_protocol_event_handler(evstate->event,
- evstate->data,
+ evstate->qdict,
evstate->last,
now);
qemu_mutex_lock(&monitor_lock);
- if (evstate->data) {
- monitor_qapi_event_emit(evstate->event, evstate->data);
- qobject_decref(evstate->data);
- evstate->data = NULL;
+ if (evstate->qdict) {
+ monitor_qapi_event_emit(evstate->event, evstate->qdict);
+ QDECREF(evstate->qdict);
+ evstate->qdict = NULL;
}
evstate->last = now;
qemu_mutex_unlock(&monitor_lock);
@@ -547,7 +547,7 @@ monitor_qapi_event_throttle(QAPIEvent event, int64_t rate)
assert(rate * SCALE_MS <= INT64_MAX);
evstate->rate = rate * SCALE_MS;
evstate->last = 0;
- evstate->data = NULL;
+ evstate->qdict = NULL;
evstate->timer = timer_new(QEMU_CLOCK_REALTIME,
SCALE_MS,
monitor_qapi_event_handler,
--
2.4.3
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PULL v2 08/14] monitor: Simplify event throttling
2015-10-28 8:15 [Qemu-devel] [PULL v2 00/14] QMP and QObject patches Markus Armbruster
` (6 preceding siblings ...)
2015-10-28 8:15 ` [Qemu-devel] [PULL v2 07/14] monitor: Reduce casting of QAPI event QDict Markus Armbruster
@ 2015-10-28 8:15 ` Markus Armbruster
2015-10-28 8:15 ` [Qemu-devel] [PULL v2 09/14] monitor: Switch from timer_new() to timer_new_ns() Markus Armbruster
` (6 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2015-10-28 8:15 UTC (permalink / raw)
To: qemu-devel
The event throttling state machine is hard to understand. I'm not
sure it's entirely correct. Rewrite it in a more straightforward
manner:
State 1: No event sent recently (less than evconf->rate ns ago)
Invariant: evstate->timer is not pending, evstate->qdict is null
On event: send event, arm timer, goto state 2
State 2: Event sent recently, no additional event being delayed
Invariant: evstate->timer is pending, evstate->qdict is null
On event: store it in evstate->qdict, goto state 3
On timer: goto state 1
State 3: Event sent recently, additional event being delayed
Invariant: evstate->timer is pending, evstate->qdict is non-null
On event: store it in evstate->qdict, goto state 3
On timer: send evstate->qdict, clear evstate->qdict,
arm timer, goto state 2
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1444921716-9511-3-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
monitor.c | 63 +++++++++++++++++++++++++++++-------------------------------
trace-events | 4 ++--
2 files changed, 32 insertions(+), 35 deletions(-)
diff --git a/monitor.c b/monitor.c
index f90a42f..ec4096b 100644
--- a/monitor.c
+++ b/monitor.c
@@ -183,7 +183,6 @@ typedef struct {
typedef struct MonitorQAPIEventState {
QAPIEvent event; /* Event being tracked */
int64_t rate; /* Minimum time (in ns) between two events */
- int64_t last; /* QEMU_CLOCK_REALTIME value at last emission */
QEMUTimer *timer; /* Timer for handling delayed events */
QDict *qdict; /* Delayed event (if any) */
} MonitorQAPIEventState;
@@ -464,65 +463,64 @@ static void
monitor_qapi_event_queue(QAPIEvent event, QDict *qdict, Error **errp)
{
MonitorQAPIEventState *evstate;
+
assert(event < QAPI_EVENT_MAX);
- int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
+ evstate = &monitor_qapi_event_state[event];
+ trace_monitor_protocol_event_queue(event, qdict, evstate->rate);
- evstate = &(monitor_qapi_event_state[event]);
- trace_monitor_protocol_event_queue(event,
- qdict,
- evstate->rate,
- evstate->last,
- now);
-
- /* Rate limit of 0 indicates no throttling */
qemu_mutex_lock(&monitor_lock);
+
if (!evstate->rate) {
+ /* Unthrottled event */
monitor_qapi_event_emit(event, qdict);
- evstate->last = now;
} else {
- int64_t delta = now - evstate->last;
- if (evstate->qdict ||
- delta < evstate->rate) {
- /* If there's an existing event pending, replace
- * it with the new event, otherwise schedule a
- * timer for delayed emission
+ if (timer_pending(evstate->timer)) {
+ /*
+ * Timer is pending for (at least) evstate->rate ns after
+ * last send. Store event for sending when timer fires,
+ * replacing a prior stored event if any.
*/
- if (evstate->qdict) {
- QDECREF(evstate->qdict);
- } else {
- int64_t then = evstate->last + evstate->rate;
- timer_mod_ns(evstate->timer, then);
- }
+ QDECREF(evstate->qdict);
evstate->qdict = qdict;
QINCREF(evstate->qdict);
} else {
+ /*
+ * Last send was (at least) evstate->rate ns ago.
+ * Send immediately, and arm the timer to call
+ * monitor_qapi_event_handler() in evstate->rate ns. Any
+ * events arriving before then will be delayed until then.
+ */
+ int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
+
monitor_qapi_event_emit(event, qdict);
- evstate->last = now;
+ timer_mod_ns(evstate->timer, now + evstate->rate);
}
}
+
qemu_mutex_unlock(&monitor_lock);
}
/*
- * The callback invoked by QemuTimer when a delayed
- * event is ready to be emitted
+ * This function runs evstate->rate ns after sending a throttled
+ * event.
+ * If another event has since been stored, send it.
*/
static void monitor_qapi_event_handler(void *opaque)
{
MonitorQAPIEventState *evstate = opaque;
- int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
- trace_monitor_protocol_event_handler(evstate->event,
- evstate->qdict,
- evstate->last,
- now);
+ trace_monitor_protocol_event_handler(evstate->event, evstate->qdict);
qemu_mutex_lock(&monitor_lock);
+
if (evstate->qdict) {
+ int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
+
monitor_qapi_event_emit(evstate->event, evstate->qdict);
QDECREF(evstate->qdict);
evstate->qdict = NULL;
+ timer_mod_ns(evstate->timer, now + evstate->rate);
}
- evstate->last = now;
+
qemu_mutex_unlock(&monitor_lock);
}
@@ -546,7 +544,6 @@ monitor_qapi_event_throttle(QAPIEvent event, int64_t rate)
evstate->event = event;
assert(rate * SCALE_MS <= INT64_MAX);
evstate->rate = rate * SCALE_MS;
- evstate->last = 0;
evstate->qdict = NULL;
evstate->timer = timer_new(QEMU_CLOCK_REALTIME,
SCALE_MS,
diff --git a/trace-events b/trace-events
index bdfe79f..72136b9 100644
--- a/trace-events
+++ b/trace-events
@@ -1031,9 +1031,9 @@ esp_pci_sbac_write(uint32_t reg, uint32_t val) "sbac: 0x%8.8x -> 0x%8.8x"
# monitor.c
handle_qmp_command(void *mon, const char *cmd_name) "mon %p cmd_name \"%s\""
monitor_protocol_emitter(void *mon) "mon %p"
-monitor_protocol_event_handler(uint32_t event, void *data, uint64_t last, uint64_t now) "event=%d data=%p last=%" PRId64 " now=%" PRId64
+monitor_protocol_event_handler(uint32_t event, void *qdict) "event=%d data=%p"
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_queue(uint32_t event, void *qdict, uint64_t rate) "event=%d data=%p rate=%" PRId64
monitor_protocol_event_throttle(uint32_t event, uint64_t rate) "event=%d rate=%" PRId64
# hw/net/opencores_eth.c
--
2.4.3
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PULL v2 09/14] monitor: Switch from timer_new() to timer_new_ns()
2015-10-28 8:15 [Qemu-devel] [PULL v2 00/14] QMP and QObject patches Markus Armbruster
` (7 preceding siblings ...)
2015-10-28 8:15 ` [Qemu-devel] [PULL v2 08/14] monitor: Simplify event throttling Markus Armbruster
@ 2015-10-28 8:15 ` Markus Armbruster
2015-10-28 8:15 ` [Qemu-devel] [PULL v2 10/14] monitor: Split MonitorQAPIEventConf off MonitorQAPIEventState Markus Armbruster
` (5 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2015-10-28 8:15 UTC (permalink / raw)
To: qemu-devel
We don't actually care for the scale, so we can just as well use the
simpler interface.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1444921716-9511-4-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
monitor.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/monitor.c b/monitor.c
index ec4096b..c2aedbe 100644
--- a/monitor.c
+++ b/monitor.c
@@ -545,10 +545,9 @@ monitor_qapi_event_throttle(QAPIEvent event, int64_t rate)
assert(rate * SCALE_MS <= INT64_MAX);
evstate->rate = rate * SCALE_MS;
evstate->qdict = NULL;
- evstate->timer = timer_new(QEMU_CLOCK_REALTIME,
- SCALE_MS,
- monitor_qapi_event_handler,
- evstate);
+ evstate->timer = timer_new_ns(QEMU_CLOCK_REALTIME,
+ monitor_qapi_event_handler,
+ evstate);
}
static void monitor_qapi_event_init(void)
--
2.4.3
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PULL v2 10/14] monitor: Split MonitorQAPIEventConf off MonitorQAPIEventState
2015-10-28 8:15 [Qemu-devel] [PULL v2 00/14] QMP and QObject patches Markus Armbruster
` (8 preceding siblings ...)
2015-10-28 8:15 ` [Qemu-devel] [PULL v2 09/14] monitor: Switch from timer_new() to timer_new_ns() Markus Armbruster
@ 2015-10-28 8:15 ` Markus Armbruster
2015-10-28 8:15 ` [Qemu-devel] [PULL v2 11/14] glib: add compatibility interface for g_hash_table_add() Markus Armbruster
` (4 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2015-10-28 8:15 UTC (permalink / raw)
To: qemu-devel
In preparation of turning monitor_qapi_event_state[] into a hash table
for finer grained throttling.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1444921716-9511-5-git-send-email-armbru@redhat.com>
---
monitor.c | 80 ++++++++++++++++++++++++++++++---------------------------------
1 file changed, 38 insertions(+), 42 deletions(-)
diff --git a/monitor.c b/monitor.c
index c2aedbe..07a0f76 100644
--- a/monitor.c
+++ b/monitor.c
@@ -182,11 +182,14 @@ typedef struct {
*/
typedef struct MonitorQAPIEventState {
QAPIEvent event; /* Event being tracked */
- int64_t rate; /* Minimum time (in ns) between two events */
QEMUTimer *timer; /* Timer for handling delayed events */
QDict *qdict; /* Delayed event (if any) */
} MonitorQAPIEventState;
+typedef struct {
+ int64_t rate; /* Minimum time (in ns) between two events */
+} MonitorQAPIEventConf;
+
struct Monitor {
CharDriverState *chr;
int reset_seen;
@@ -437,6 +440,16 @@ static void monitor_protocol_emitter(Monitor *mon, QObject *data,
}
+static MonitorQAPIEventConf monitor_qapi_event_conf[QAPI_EVENT_MAX] = {
+ /* Limit guest-triggerable events to 1 per second */
+ [QAPI_EVENT_RTC_CHANGE] = { 1000 * SCALE_MS },
+ [QAPI_EVENT_WATCHDOG] = { 1000 * SCALE_MS },
+ [QAPI_EVENT_BALLOON_CHANGE] = { 1000 * SCALE_MS },
+ [QAPI_EVENT_QUORUM_REPORT_BAD] = { 1000 * SCALE_MS },
+ [QAPI_EVENT_QUORUM_FAILURE] = { 1000 * SCALE_MS },
+ [QAPI_EVENT_VSERPORT_CHANGE] = { 1000 * SCALE_MS },
+};
+
static MonitorQAPIEventState monitor_qapi_event_state[QAPI_EVENT_MAX];
/*
@@ -462,21 +475,23 @@ static void monitor_qapi_event_emit(QAPIEvent event, QDict *qdict)
static void
monitor_qapi_event_queue(QAPIEvent event, QDict *qdict, Error **errp)
{
+ MonitorQAPIEventConf *evconf;
MonitorQAPIEventState *evstate;
assert(event < QAPI_EVENT_MAX);
+ evconf = &monitor_qapi_event_conf[event];
evstate = &monitor_qapi_event_state[event];
- trace_monitor_protocol_event_queue(event, qdict, evstate->rate);
+ trace_monitor_protocol_event_queue(event, qdict, evconf->rate);
qemu_mutex_lock(&monitor_lock);
- if (!evstate->rate) {
+ if (!evconf->rate) {
/* Unthrottled event */
monitor_qapi_event_emit(event, qdict);
} else {
if (timer_pending(evstate->timer)) {
/*
- * Timer is pending for (at least) evstate->rate ns after
+ * Timer is pending for (at least) evconf->rate ns after
* last send. Store event for sending when timer fires,
* replacing a prior stored event if any.
*/
@@ -485,15 +500,15 @@ monitor_qapi_event_queue(QAPIEvent event, QDict *qdict, Error **errp)
QINCREF(evstate->qdict);
} else {
/*
- * Last send was (at least) evstate->rate ns ago.
+ * Last send was (at least) evconf->rate ns ago.
* Send immediately, and arm the timer to call
- * monitor_qapi_event_handler() in evstate->rate ns. Any
+ * monitor_qapi_event_handler() in evconf->rate ns. Any
* events arriving before then will be delayed until then.
*/
int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
monitor_qapi_event_emit(event, qdict);
- timer_mod_ns(evstate->timer, now + evstate->rate);
+ timer_mod_ns(evstate->timer, now + evconf->rate);
}
}
@@ -501,13 +516,14 @@ monitor_qapi_event_queue(QAPIEvent event, QDict *qdict, Error **errp)
}
/*
- * This function runs evstate->rate ns after sending a throttled
+ * This function runs evconf->rate ns after sending a throttled
* event.
* If another event has since been stored, send it.
*/
static void monitor_qapi_event_handler(void *opaque)
{
MonitorQAPIEventState *evstate = opaque;
+ MonitorQAPIEventConf *evconf = &monitor_qapi_event_conf[evstate->event];
trace_monitor_protocol_event_handler(evstate->event, evstate->qdict);
qemu_mutex_lock(&monitor_lock);
@@ -518,47 +534,27 @@ static void monitor_qapi_event_handler(void *opaque)
monitor_qapi_event_emit(evstate->event, evstate->qdict);
QDECREF(evstate->qdict);
evstate->qdict = NULL;
- timer_mod_ns(evstate->timer, now + evstate->rate);
+ timer_mod_ns(evstate->timer, now + evconf->rate);
}
qemu_mutex_unlock(&monitor_lock);
}
-/*
- * @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_protocol_event_throttle(event, rate);
- evstate->event = event;
- assert(rate * SCALE_MS <= INT64_MAX);
- evstate->rate = rate * SCALE_MS;
- evstate->qdict = NULL;
- evstate->timer = timer_new_ns(QEMU_CLOCK_REALTIME,
- monitor_qapi_event_handler,
- evstate);
-}
-
static void monitor_qapi_event_init(void)
{
- /* Limit guest-triggerable events to 1 per second */
- monitor_qapi_event_throttle(QAPI_EVENT_RTC_CHANGE, 1000);
- monitor_qapi_event_throttle(QAPI_EVENT_WATCHDOG, 1000);
- monitor_qapi_event_throttle(QAPI_EVENT_BALLOON_CHANGE, 1000);
- monitor_qapi_event_throttle(QAPI_EVENT_QUORUM_REPORT_BAD, 1000);
- monitor_qapi_event_throttle(QAPI_EVENT_QUORUM_FAILURE, 1000);
- monitor_qapi_event_throttle(QAPI_EVENT_VSERPORT_CHANGE, 1000);
+ int i;
+ MonitorQAPIEventState *evstate;
+
+ for (i = 0; i < QAPI_EVENT_MAX; i++) {
+ if (monitor_qapi_event_conf[i].rate) {
+ evstate = &monitor_qapi_event_state[i];
+ evstate->event = i;
+ evstate->qdict = NULL;
+ evstate->timer = timer_new_ns(QEMU_CLOCK_REALTIME,
+ monitor_qapi_event_handler,
+ evstate);
+ }
+ }
qmp_event_set_func_emit(monitor_qapi_event_queue);
}
--
2.4.3
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PULL v2 11/14] glib: add compatibility interface for g_hash_table_add()
2015-10-28 8:15 [Qemu-devel] [PULL v2 00/14] QMP and QObject patches Markus Armbruster
` (9 preceding siblings ...)
2015-10-28 8:15 ` [Qemu-devel] [PULL v2 10/14] monitor: Split MonitorQAPIEventConf off MonitorQAPIEventState Markus Armbruster
@ 2015-10-28 8:15 ` Markus Armbruster
2015-10-29 13:39 ` Markus Armbruster
2015-10-28 8:15 ` [Qemu-devel] [PULL v2 12/14] monitor: Turn monitor_qapi_event_state[] into a hash table Markus Armbruster
` (3 subsequent siblings)
14 siblings, 1 reply; 19+ messages in thread
From: Markus Armbruster @ 2015-10-28 8:15 UTC (permalink / raw)
To: qemu-devel
The next commit will use it.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
include/glib-compat.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/include/glib-compat.h b/include/glib-compat.h
index fb25f43..6d13b5f 100644
--- a/include/glib-compat.h
+++ b/include/glib-compat.h
@@ -165,6 +165,13 @@ static inline GThread *g_thread_new(const char *name,
#define CompatGCond GCond
#endif /* glib 2.31 */
+#if !GLIB_CHECK_VERSION(2, 32, 0)
+static inline gboolean g_hash_table_add(GHashTable *hash_table, gpointer key)
+{
+ return g_hash_table_replace(hash_table, key, key)
+}
+#endif
+
#ifndef g_assert_true
#define g_assert_true(expr) \
do { \
--
2.4.3
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [Qemu-devel] [PULL v2 11/14] glib: add compatibility interface for g_hash_table_add()
2015-10-28 8:15 ` [Qemu-devel] [PULL v2 11/14] glib: add compatibility interface for g_hash_table_add() Markus Armbruster
@ 2015-10-29 13:39 ` Markus Armbruster
0 siblings, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2015-10-29 13:39 UTC (permalink / raw)
To: qemu-devel
Markus Armbruster <armbru@redhat.com> writes:
> The next commit will use it.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
> include/glib-compat.h | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/include/glib-compat.h b/include/glib-compat.h
> index fb25f43..6d13b5f 100644
> --- a/include/glib-compat.h
> +++ b/include/glib-compat.h
> @@ -165,6 +165,13 @@ static inline GThread *g_thread_new(const char *name,
> #define CompatGCond GCond
> #endif /* glib 2.31 */
>
> +#if !GLIB_CHECK_VERSION(2, 32, 0)
> +static inline gboolean g_hash_table_add(GHashTable *hash_table, gpointer key)
> +{
> + return g_hash_table_replace(hash_table, key, key)
> +}
> +#endif
> +
> #ifndef g_assert_true
> #define g_assert_true(expr) \
> do { \
Squashing fixup:
diff --git a/include/glib-compat.h b/include/glib-compat.h
index 6d13b5f..e75dd96 100644
--- a/include/glib-compat.h
+++ b/include/glib-compat.h
@@ -166,9 +166,10 @@ static inline GThread *g_thread_new(const char *name,
#endif /* glib 2.31 */
#if !GLIB_CHECK_VERSION(2, 32, 0)
-static inline gboolean g_hash_table_add(GHashTable *hash_table, gpointer key)
+/* Beware, function returns gboolean since 2.39.2, see GLib commit 9101915 */
+static inline void g_hash_table_add(GHashTable *hash_table, gpointer key)
{
- return g_hash_table_replace(hash_table, key, key)
+ g_hash_table_replace(hash_table, key, key)
}
#endif
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PULL v2 12/14] monitor: Turn monitor_qapi_event_state[] into a hash table
2015-10-28 8:15 [Qemu-devel] [PULL v2 00/14] QMP and QObject patches Markus Armbruster
` (10 preceding siblings ...)
2015-10-28 8:15 ` [Qemu-devel] [PULL v2 11/14] glib: add compatibility interface for g_hash_table_add() Markus Armbruster
@ 2015-10-28 8:15 ` Markus Armbruster
2015-10-28 8:16 ` [Qemu-devel] [PULL v2 13/14] monitor: Throttle event VSERPORT_CHANGE separately by "id" Markus Armbruster
` (2 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2015-10-28 8:15 UTC (permalink / raw)
To: qemu-devel
In preparation of finer grained throttling.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1444921716-9511-6-git-send-email-armbru@redhat.com>
---
monitor.c | 55 ++++++++++++++++++++++++++++++++++++++-----------------
1 file changed, 38 insertions(+), 17 deletions(-)
diff --git a/monitor.c b/monitor.c
index 07a0f76..525d8e4 100644
--- a/monitor.c
+++ b/monitor.c
@@ -450,7 +450,7 @@ static MonitorQAPIEventConf monitor_qapi_event_conf[QAPI_EVENT_MAX] = {
[QAPI_EVENT_VSERPORT_CHANGE] = { 1000 * SCALE_MS },
};
-static MonitorQAPIEventState monitor_qapi_event_state[QAPI_EVENT_MAX];
+GHashTable *monitor_qapi_event_state;
/*
* Emits the event to every monitor instance, @event is only used for trace
@@ -468,6 +468,8 @@ static void monitor_qapi_event_emit(QAPIEvent event, QDict *qdict)
}
}
+static void monitor_qapi_event_handler(void *opaque);
+
/*
* Queue a new event for emission to Monitor instances,
* applying any rate limiting if required.
@@ -480,7 +482,6 @@ monitor_qapi_event_queue(QAPIEvent event, QDict *qdict, Error **errp)
assert(event < QAPI_EVENT_MAX);
evconf = &monitor_qapi_event_conf[event];
- evstate = &monitor_qapi_event_state[event];
trace_monitor_protocol_event_queue(event, qdict, evconf->rate);
qemu_mutex_lock(&monitor_lock);
@@ -489,7 +490,12 @@ monitor_qapi_event_queue(QAPIEvent event, QDict *qdict, Error **errp)
/* Unthrottled event */
monitor_qapi_event_emit(event, qdict);
} else {
- if (timer_pending(evstate->timer)) {
+ MonitorQAPIEventState key = { .event = event };
+
+ evstate = g_hash_table_lookup(monitor_qapi_event_state, &key);
+ assert(!evstate || timer_pending(evstate->timer));
+
+ if (evstate) {
/*
* Timer is pending for (at least) evconf->rate ns after
* last send. Store event for sending when timer fires,
@@ -508,6 +514,14 @@ monitor_qapi_event_queue(QAPIEvent event, QDict *qdict, Error **errp)
int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
monitor_qapi_event_emit(event, qdict);
+
+ evstate = g_new(MonitorQAPIEventState, 1);
+ evstate->event = event;
+ evstate->qdict = NULL;
+ evstate->timer = timer_new_ns(QEMU_CLOCK_REALTIME,
+ monitor_qapi_event_handler,
+ evstate);
+ g_hash_table_add(monitor_qapi_event_state, evstate);
timer_mod_ns(evstate->timer, now + evconf->rate);
}
}
@@ -535,27 +549,34 @@ static void monitor_qapi_event_handler(void *opaque)
QDECREF(evstate->qdict);
evstate->qdict = NULL;
timer_mod_ns(evstate->timer, now + evconf->rate);
+ } else {
+ g_hash_table_remove(monitor_qapi_event_state, evstate);
+ timer_free(evstate->timer);
+ g_free(evstate);
}
qemu_mutex_unlock(&monitor_lock);
}
+static unsigned int qapi_event_throttle_hash(const void *key)
+{
+ const MonitorQAPIEventState *evstate = key;
+
+ return evstate->event * 255;
+}
+
+static gboolean qapi_event_throttle_equal(const void *a, const void *b)
+{
+ const MonitorQAPIEventState *eva = a;
+ const MonitorQAPIEventState *evb = b;
+
+ return eva->event == evb->event;
+}
+
static void monitor_qapi_event_init(void)
{
- int i;
- MonitorQAPIEventState *evstate;
-
- for (i = 0; i < QAPI_EVENT_MAX; i++) {
- if (monitor_qapi_event_conf[i].rate) {
- evstate = &monitor_qapi_event_state[i];
- evstate->event = i;
- evstate->qdict = NULL;
- evstate->timer = timer_new_ns(QEMU_CLOCK_REALTIME,
- monitor_qapi_event_handler,
- evstate);
- }
- }
-
+ monitor_qapi_event_state = g_hash_table_new(qapi_event_throttle_hash,
+ qapi_event_throttle_equal);
qmp_event_set_func_emit(monitor_qapi_event_queue);
}
--
2.4.3
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PULL v2 13/14] monitor: Throttle event VSERPORT_CHANGE separately by "id"
2015-10-28 8:15 [Qemu-devel] [PULL v2 00/14] QMP and QObject patches Markus Armbruster
` (11 preceding siblings ...)
2015-10-28 8:15 ` [Qemu-devel] [PULL v2 12/14] monitor: Turn monitor_qapi_event_state[] into a hash table Markus Armbruster
@ 2015-10-28 8:16 ` Markus Armbruster
2015-10-28 8:16 ` [Qemu-devel] [PULL v2 14/14] docs: Document QMP event rate limiting Markus Armbruster
2015-10-28 15:07 ` [Qemu-devel] [PULL v2 00/14] QMP and QObject patches Peter Maydell
14 siblings, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2015-10-28 8:16 UTC (permalink / raw)
To: qemu-devel
VSERPORT_CHANGE is emitted when the guest opens or closes a
virtio-serial port. The event's member "id" identifies the port.
When several events arrive quickly, throttling drops all but the last
of them. Because of that, a QMP client must assume that *any* port
may have changed state when it receives a VSERPORT_CHANGE event and
throttling may have happened.
Make the event more useful by throttling it for each port separately.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1444921716-9511-7-git-send-email-armbru@redhat.com>
---
monitor.c | 27 +++++++++++++++++++++++----
1 file changed, 23 insertions(+), 4 deletions(-)
diff --git a/monitor.c b/monitor.c
index 525d8e4..6cd747f 100644
--- a/monitor.c
+++ b/monitor.c
@@ -181,7 +181,8 @@ typedef struct {
* instance.
*/
typedef struct MonitorQAPIEventState {
- QAPIEvent event; /* Event being tracked */
+ QAPIEvent event; /* Throttling state for this event type and... */
+ QDict *data; /* ... data, see qapi_event_throttle_equal() */
QEMUTimer *timer; /* Timer for handling delayed events */
QDict *qdict; /* Delayed event (if any) */
} MonitorQAPIEventState;
@@ -490,7 +491,8 @@ monitor_qapi_event_queue(QAPIEvent event, QDict *qdict, Error **errp)
/* Unthrottled event */
monitor_qapi_event_emit(event, qdict);
} else {
- MonitorQAPIEventState key = { .event = event };
+ QDict *data = qobject_to_qdict(qdict_get(qdict, "data"));
+ MonitorQAPIEventState key = { .event = event, .data = data };
evstate = g_hash_table_lookup(monitor_qapi_event_state, &key);
assert(!evstate || timer_pending(evstate->timer));
@@ -517,6 +519,8 @@ monitor_qapi_event_queue(QAPIEvent event, QDict *qdict, Error **errp)
evstate = g_new(MonitorQAPIEventState, 1);
evstate->event = event;
+ evstate->data = data;
+ QINCREF(evstate->data);
evstate->qdict = NULL;
evstate->timer = timer_new_ns(QEMU_CLOCK_REALTIME,
monitor_qapi_event_handler,
@@ -551,6 +555,7 @@ static void monitor_qapi_event_handler(void *opaque)
timer_mod_ns(evstate->timer, now + evconf->rate);
} else {
g_hash_table_remove(monitor_qapi_event_state, evstate);
+ QDECREF(evstate->data);
timer_free(evstate->timer);
g_free(evstate);
}
@@ -561,8 +566,13 @@ static void monitor_qapi_event_handler(void *opaque)
static unsigned int qapi_event_throttle_hash(const void *key)
{
const MonitorQAPIEventState *evstate = key;
+ unsigned int hash = evstate->event * 255;
- return evstate->event * 255;
+ if (evstate->event == QAPI_EVENT_VSERPORT_CHANGE) {
+ hash += g_str_hash(qdict_get_str(evstate->data, "id"));
+ }
+
+ return hash;
}
static gboolean qapi_event_throttle_equal(const void *a, const void *b)
@@ -570,7 +580,16 @@ static gboolean qapi_event_throttle_equal(const void *a, const void *b)
const MonitorQAPIEventState *eva = a;
const MonitorQAPIEventState *evb = b;
- return eva->event == evb->event;
+ if (eva->event != evb->event) {
+ return FALSE;
+ }
+
+ if (eva->event == QAPI_EVENT_VSERPORT_CHANGE) {
+ return !strcmp(qdict_get_str(eva->data, "id"),
+ qdict_get_str(evb->data, "id"));
+ }
+
+ return TRUE;
}
static void monitor_qapi_event_init(void)
--
2.4.3
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [PULL v2 14/14] docs: Document QMP event rate limiting
2015-10-28 8:15 [Qemu-devel] [PULL v2 00/14] QMP and QObject patches Markus Armbruster
` (12 preceding siblings ...)
2015-10-28 8:16 ` [Qemu-devel] [PULL v2 13/14] monitor: Throttle event VSERPORT_CHANGE separately by "id" Markus Armbruster
@ 2015-10-28 8:16 ` Markus Armbruster
2015-10-28 15:07 ` [Qemu-devel] [PULL v2 00/14] QMP and QObject patches Peter Maydell
14 siblings, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2015-10-28 8:16 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1444921716-9511-8-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
docs/qmp-events.txt | 12 ++++++++++++
docs/qmp-spec.txt | 5 +++++
2 files changed, 17 insertions(+)
diff --git a/docs/qmp-events.txt b/docs/qmp-events.txt
index d92cc48..d2f1ce4 100644
--- a/docs/qmp-events.txt
+++ b/docs/qmp-events.txt
@@ -28,6 +28,8 @@ Example:
"data": { "actual": 944766976 },
"timestamp": { "seconds": 1267020223, "microseconds": 435656 } }
+Note: this event is rate-limited.
+
BLOCK_IMAGE_CORRUPTED
---------------------
@@ -296,6 +298,8 @@ Example:
"data": { "reference": "usr1", "sector-num": 345435, "sectors-count": 5 },
"timestamp": { "seconds": 1344522075, "microseconds": 745528 } }
+Note: this event is rate-limited.
+
QUORUM_REPORT_BAD
-----------------
@@ -318,6 +322,8 @@ Example:
"data": { "node-name": "1.raw", "sector-num": 345435, "sectors-count": 5 },
"timestamp": { "seconds": 1344522075, "microseconds": 745528 } }
+Note: this event is rate-limited.
+
RESET
-----
@@ -358,6 +364,8 @@ Example:
"data": { "offset": 78 },
"timestamp": { "seconds": 1267020223, "microseconds": 435656 } }
+Note: this event is rate-limited.
+
SHUTDOWN
--------
@@ -632,6 +640,8 @@ Example:
"data": { "id": "channel0", "open": true },
"timestamp": { "seconds": 1401385907, "microseconds": 422329 } }
+Note: this event is rate-limited separately for each "id".
+
WAKEUP
------
@@ -662,3 +672,5 @@ Example:
Note: If action is "reset", "shutdown", or "pause" the WATCHDOG event is
followed respectively by the RESET, SHUTDOWN, or STOP events.
+
+Note: this event is rate-limited.
diff --git a/docs/qmp-spec.txt b/docs/qmp-spec.txt
index 4c28cd9..4fb10a5 100644
--- a/docs/qmp-spec.txt
+++ b/docs/qmp-spec.txt
@@ -175,6 +175,11 @@ The format of asynchronous events is:
For a listing of supported asynchronous events, please, refer to the
qmp-events.txt file.
+Some events are rate-limited to at most one per second. If additional
+"similar" events arrive within one second, all but the last one are
+dropped, and the last one is delayed. "Similar" normally means same
+event type. See qmp-events.txt for details.
+
2.5 QGA Synchronization
-----------------------
--
2.4.3
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [Qemu-devel] [PULL v2 00/14] QMP and QObject patches
2015-10-28 8:15 [Qemu-devel] [PULL v2 00/14] QMP and QObject patches Markus Armbruster
` (13 preceding siblings ...)
2015-10-28 8:16 ` [Qemu-devel] [PULL v2 14/14] docs: Document QMP event rate limiting Markus Armbruster
@ 2015-10-28 15:07 ` Peter Maydell
2015-10-28 15:24 ` Eric Blake
14 siblings, 1 reply; 19+ messages in thread
From: Peter Maydell @ 2015-10-28 15:07 UTC (permalink / raw)
To: Markus Armbruster; +Cc: QEMU Developers
On 28 October 2015 at 08:15, Markus Armbruster <armbru@redhat.com> wrote:
> The following changes since commit 7e038b94e74e1c2d1b3598e2e4b0b5c8b79a7278:
>
> Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into staging (2015-10-27 10:10:46 +0000)
>
> are available in the git repository at:
>
> git://repo.or.cz/qemu/armbru.git tags/pull-monitor-2015-10-28
>
> for you to fetch changes up to 81387a94e3a95068b45d15a4db67f3942c46ad4e:
>
> docs: Document QMP event rate limiting (2015-10-28 09:07:48 +0100)
>
> ----------------------------------------------------------------
> QMP and QObject patches
>
> ----------------------------------------------------------------
> Markus Armbruster (14):
> qobject: Drop QObject_HEAD
> qbool: Make conversion from QObject * accept null
> qdict: Make conversion from QObject * accept null
> qfloat qint: Make conversion from QObject * accept null
> qlist: Make conversion from QObject * accept null
> qstring: Make conversion from QObject * accept null
> monitor: Reduce casting of QAPI event QDict
> monitor: Simplify event throttling
> monitor: Switch from timer_new() to timer_new_ns()
> monitor: Split MonitorQAPIEventConf off MonitorQAPIEventState
> glib: add compatibility interface for g_hash_table_add()
> monitor: Turn monitor_qapi_event_state[] into a hash table
> monitor: Throttle event VSERPORT_CHANGE separately by "id"
> docs: Document QMP event rate limiting
OSX build failure :-(
CC qga/commands-posix.o
In file included from /Users/pm215/src/qemu-for-merges/qga/main.c:25:
In file included from
/Users/pm215/src/qemu-for-merges/include/qapi/qmp/json-parser.h:17:
In file included from /Users/pm215/src/qemu-for-merges/include/qemu-common.h:25:
/Users/pm215/src/qemu-for-merges/include/glib-compat.h:171:12: error:
returning 'void' from a function with incompatible result type
'gboolean' (aka 'int')
return g_hash_table_replace(hash_table, key, key)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Looks like g_hash_table_replace was originally a 'void' return
and switched to 'gboolean' return at some later date:
https://github.com/GNOME/glib/commit/910191597a6c2e5d5d460e9ce9efb4f47d9cc63c
thanks
-- PMM
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [Qemu-devel] [PULL v2 00/14] QMP and QObject patches
2015-10-28 15:07 ` [Qemu-devel] [PULL v2 00/14] QMP and QObject patches Peter Maydell
@ 2015-10-28 15:24 ` Eric Blake
2015-10-29 13:05 ` Markus Armbruster
0 siblings, 1 reply; 19+ messages in thread
From: Eric Blake @ 2015-10-28 15:24 UTC (permalink / raw)
To: Peter Maydell, Markus Armbruster; +Cc: QEMU Developers
[-- Attachment #1: Type: text/plain, Size: 1492 bytes --]
On 10/28/2015 09:07 AM, Peter Maydell wrote:
>> glib: add compatibility interface for g_hash_table_add()
>> monitor: Turn monitor_qapi_event_state[] into a hash table
>
> CC qga/commands-posix.o
> In file included from /Users/pm215/src/qemu-for-merges/qga/main.c:25:
> In file included from
> /Users/pm215/src/qemu-for-merges/include/qapi/qmp/json-parser.h:17:
> In file included from /Users/pm215/src/qemu-for-merges/include/qemu-common.h:25:
> /Users/pm215/src/qemu-for-merges/include/glib-compat.h:171:12: error:
> returning 'void' from a function with incompatible result type
> 'gboolean' (aka 'int')
> return g_hash_table_replace(hash_table, key, key)
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Looks like g_hash_table_replace was originally a 'void' return
> and switched to 'gboolean' return at some later date:
>
> https://github.com/GNOME/glib/commit/910191597a6c2e5d5d460e9ce9efb4f47d9cc63c
This patch series isn't using the return value of g_hash_table_add, so
our glib replacement could be changed to return void.
On the other hand, would it be better to proactively retrofit the return
type into ALL of the g_hash_table_* functions that were swapped to
return a value, so that future uses of the functions with qemu can make
use of the modern contract, even though this series doesn't use it?
--
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] 19+ messages in thread
* Re: [Qemu-devel] [PULL v2 00/14] QMP and QObject patches
2015-10-28 15:24 ` Eric Blake
@ 2015-10-29 13:05 ` Markus Armbruster
0 siblings, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2015-10-29 13:05 UTC (permalink / raw)
To: Eric Blake; +Cc: Peter Maydell, QEMU Developers
Eric Blake <eblake@redhat.com> writes:
> On 10/28/2015 09:07 AM, Peter Maydell wrote:
>
>>> glib: add compatibility interface for g_hash_table_add()
>>> monitor: Turn monitor_qapi_event_state[] into a hash table
>
>>
>> CC qga/commands-posix.o
>> In file included from /Users/pm215/src/qemu-for-merges/qga/main.c:25:
>> In file included from
>> /Users/pm215/src/qemu-for-merges/include/qapi/qmp/json-parser.h:17:
>> In file included from /Users/pm215/src/qemu-for-merges/include/qemu-common.h:25:
>> /Users/pm215/src/qemu-for-merges/include/glib-compat.h:171:12: error:
>> returning 'void' from a function with incompatible result type
>> 'gboolean' (aka 'int')
>> return g_hash_table_replace(hash_table, key, key)
>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> Looks like g_hash_table_replace was originally a 'void' return
>> and switched to 'gboolean' return at some later date:
>>
>> https://github.com/GNOME/glib/commit/910191597a6c2e5d5d460e9ce9efb4f47d9cc63c
The 80s are calling: cavalier API break. The GLib maintainers should
know better.
Anyway, this change is in 2.39.2. The replacement function is for
<2.32. Therefore, g_hash_table_replace() will always return void then.
> This patch series isn't using the return value of g_hash_table_add, so
> our glib replacement could be changed to return void.
Yes. Matches GLib's function before 2.39.2.
> On the other hand, would it be better to proactively retrofit the return
> type into ALL of the g_hash_table_* functions that were swapped to
> return a value, so that future uses of the functions with qemu can make
> use of the modern contract, even though this series doesn't use it?
I think we should add glib-compat cruft only when we actually need it.
^ permalink raw reply [flat|nested] 19+ messages in thread