qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 00/13] QMP and QObject patches
@ 2015-10-27  9:21 Markus Armbruster
  2015-10-27  9:21 ` [Qemu-devel] [PULL 01/13] qobject: Drop QObject_HEAD Markus Armbruster
                   ` (13 more replies)
  0 siblings, 14 replies; 16+ messages in thread
From: Markus Armbruster @ 2015-10-27  9:21 UTC (permalink / raw)
  To: qemu-devel

The following changes since commit 9666248a85fd889bfb6118f769e9c73039b998ed:

  Merge remote-tracking branch 'remotes/sstabellini/tags/xen-2015-10-26' into staging (2015-10-26 13:13:38 +0000)

are available in the git repository at:

  git://repo.or.cz/qemu/armbru.git tags/pull-monitor-2015-10-27

for you to fetch changes up to 8a4e1a93634a95997e227f3470453f07cf159fcf:

  docs: Document QMP event rate limiting (2015-10-27 10:05:54 +0100)

----------------------------------------------------------------
QMP and QObject patches

----------------------------------------------------------------
Markus Armbruster (13):
      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
      monitor: Turn monitor_qapi_event_state[] into a hash table
      monitor: Throttle event VSERPORT_CHANGE separately by "id"
      docs: Document QMP event rate limiting

 docs/qmp-events.txt        |  12 +++
 docs/qmp-spec.txt          |   5 ++
 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 +-
 monitor.c                  | 192 ++++++++++++++++++++++++++-------------------
 qapi/qmp-input-visitor.c   |  40 +++++-----
 qga/main.c                 |  11 +--
 qobject/qbool.c            |   4 +-
 qobject/qdict.c            |  39 +++------
 qobject/qfloat.c           |   4 +-
 qobject/qint.c             |   4 +-
 qobject/qlist.c            |   3 +-
 qobject/qstring.c          |   4 +-
 trace-events               |   4 +-
 19 files changed, 183 insertions(+), 155 deletions(-)

-- 
2.4.3

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

* [Qemu-devel] [PULL 01/13] qobject: Drop QObject_HEAD
  2015-10-27  9:21 [Qemu-devel] [PULL 00/13] QMP and QObject patches Markus Armbruster
@ 2015-10-27  9:21 ` Markus Armbruster
  2015-10-27  9:21 ` [Qemu-devel] [PULL 02/13] qbool: Make conversion from QObject * accept null Markus Armbruster
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Markus Armbruster @ 2015-10-27  9:21 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] 16+ messages in thread

* [Qemu-devel] [PULL 02/13] qbool: Make conversion from QObject * accept null
  2015-10-27  9:21 [Qemu-devel] [PULL 00/13] QMP and QObject patches Markus Armbruster
  2015-10-27  9:21 ` [Qemu-devel] [PULL 01/13] qobject: Drop QObject_HEAD Markus Armbruster
@ 2015-10-27  9:21 ` Markus Armbruster
  2015-10-27  9:21 ` [Qemu-devel] [PULL 03/13] qdict: " Markus Armbruster
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Markus Armbruster @ 2015-10-27  9:21 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] 16+ messages in thread

* [Qemu-devel] [PULL 03/13] qdict: Make conversion from QObject * accept null
  2015-10-27  9:21 [Qemu-devel] [PULL 00/13] QMP and QObject patches Markus Armbruster
  2015-10-27  9:21 ` [Qemu-devel] [PULL 01/13] qobject: Drop QObject_HEAD Markus Armbruster
  2015-10-27  9:21 ` [Qemu-devel] [PULL 02/13] qbool: Make conversion from QObject * accept null Markus Armbruster
@ 2015-10-27  9:21 ` Markus Armbruster
  2015-10-27  9:21 ` [Qemu-devel] [PULL 04/13] qfloat qint: " Markus Armbruster
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Markus Armbruster @ 2015-10-27  9:21 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] 16+ messages in thread

* [Qemu-devel] [PULL 04/13] qfloat qint: Make conversion from QObject * accept null
  2015-10-27  9:21 [Qemu-devel] [PULL 00/13] QMP and QObject patches Markus Armbruster
                   ` (2 preceding siblings ...)
  2015-10-27  9:21 ` [Qemu-devel] [PULL 03/13] qdict: " Markus Armbruster
@ 2015-10-27  9:21 ` Markus Armbruster
  2015-10-27  9:21 ` [Qemu-devel] [PULL 05/13] qlist: " Markus Armbruster
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Markus Armbruster @ 2015-10-27  9:21 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] 16+ messages in thread

* [Qemu-devel] [PULL 05/13] qlist: Make conversion from QObject * accept null
  2015-10-27  9:21 [Qemu-devel] [PULL 00/13] QMP and QObject patches Markus Armbruster
                   ` (3 preceding siblings ...)
  2015-10-27  9:21 ` [Qemu-devel] [PULL 04/13] qfloat qint: " Markus Armbruster
@ 2015-10-27  9:21 ` Markus Armbruster
  2015-10-27  9:21 ` [Qemu-devel] [PULL 06/13] qstring: " Markus Armbruster
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Markus Armbruster @ 2015-10-27  9:21 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] 16+ messages in thread

* [Qemu-devel] [PULL 06/13] qstring: Make conversion from QObject * accept null
  2015-10-27  9:21 [Qemu-devel] [PULL 00/13] QMP and QObject patches Markus Armbruster
                   ` (4 preceding siblings ...)
  2015-10-27  9:21 ` [Qemu-devel] [PULL 05/13] qlist: " Markus Armbruster
@ 2015-10-27  9:21 ` Markus Armbruster
  2015-10-27  9:21 ` [Qemu-devel] [PULL 07/13] monitor: Reduce casting of QAPI event QDict Markus Armbruster
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Markus Armbruster @ 2015-10-27  9:21 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] 16+ messages in thread

* [Qemu-devel] [PULL 07/13] monitor: Reduce casting of QAPI event QDict
  2015-10-27  9:21 [Qemu-devel] [PULL 00/13] QMP and QObject patches Markus Armbruster
                   ` (5 preceding siblings ...)
  2015-10-27  9:21 ` [Qemu-devel] [PULL 06/13] qstring: " Markus Armbruster
@ 2015-10-27  9:21 ` Markus Armbruster
  2015-10-27  9:21 ` [Qemu-devel] [PULL 08/13] monitor: Simplify event throttling Markus Armbruster
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Markus Armbruster @ 2015-10-27  9:21 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] 16+ messages in thread

* [Qemu-devel] [PULL 08/13] monitor: Simplify event throttling
  2015-10-27  9:21 [Qemu-devel] [PULL 00/13] QMP and QObject patches Markus Armbruster
                   ` (6 preceding siblings ...)
  2015-10-27  9:21 ` [Qemu-devel] [PULL 07/13] monitor: Reduce casting of QAPI event QDict Markus Armbruster
@ 2015-10-27  9:21 ` Markus Armbruster
  2015-10-27  9:21 ` [Qemu-devel] [PULL 09/13] monitor: Switch from timer_new() to timer_new_ns() Markus Armbruster
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Markus Armbruster @ 2015-10-27  9:21 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] 16+ messages in thread

* [Qemu-devel] [PULL 09/13] monitor: Switch from timer_new() to timer_new_ns()
  2015-10-27  9:21 [Qemu-devel] [PULL 00/13] QMP and QObject patches Markus Armbruster
                   ` (7 preceding siblings ...)
  2015-10-27  9:21 ` [Qemu-devel] [PULL 08/13] monitor: Simplify event throttling Markus Armbruster
@ 2015-10-27  9:21 ` Markus Armbruster
  2015-10-27  9:21 ` [Qemu-devel] [PULL 10/13] monitor: Split MonitorQAPIEventConf off MonitorQAPIEventState Markus Armbruster
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Markus Armbruster @ 2015-10-27  9:21 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] 16+ messages in thread

* [Qemu-devel] [PULL 10/13] monitor: Split MonitorQAPIEventConf off MonitorQAPIEventState
  2015-10-27  9:21 [Qemu-devel] [PULL 00/13] QMP and QObject patches Markus Armbruster
                   ` (8 preceding siblings ...)
  2015-10-27  9:21 ` [Qemu-devel] [PULL 09/13] monitor: Switch from timer_new() to timer_new_ns() Markus Armbruster
@ 2015-10-27  9:21 ` Markus Armbruster
  2015-10-27  9:21 ` [Qemu-devel] [PULL 11/13] monitor: Turn monitor_qapi_event_state[] into a hash table Markus Armbruster
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Markus Armbruster @ 2015-10-27  9:21 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] 16+ messages in thread

* [Qemu-devel] [PULL 11/13] monitor: Turn monitor_qapi_event_state[] into a hash table
  2015-10-27  9:21 [Qemu-devel] [PULL 00/13] QMP and QObject patches Markus Armbruster
                   ` (9 preceding siblings ...)
  2015-10-27  9:21 ` [Qemu-devel] [PULL 10/13] monitor: Split MonitorQAPIEventConf off MonitorQAPIEventState Markus Armbruster
@ 2015-10-27  9:21 ` Markus Armbruster
  2015-10-27  9:21 ` [Qemu-devel] [PULL 12/13] monitor: Throttle event VSERPORT_CHANGE separately by "id" Markus Armbruster
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 16+ messages in thread
From: Markus Armbruster @ 2015-10-27  9:21 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] 16+ messages in thread

* [Qemu-devel] [PULL 12/13] monitor: Throttle event VSERPORT_CHANGE separately by "id"
  2015-10-27  9:21 [Qemu-devel] [PULL 00/13] QMP and QObject patches Markus Armbruster
                   ` (10 preceding siblings ...)
  2015-10-27  9:21 ` [Qemu-devel] [PULL 11/13] monitor: Turn monitor_qapi_event_state[] into a hash table Markus Armbruster
@ 2015-10-27  9:21 ` Markus Armbruster
  2015-10-27  9:21 ` [Qemu-devel] [PULL 13/13] docs: Document QMP event rate limiting Markus Armbruster
  2015-10-27 13:06 ` [Qemu-devel] [PULL 00/13] QMP and QObject patches Peter Maydell
  13 siblings, 0 replies; 16+ messages in thread
From: Markus Armbruster @ 2015-10-27  9:21 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] 16+ messages in thread

* [Qemu-devel] [PULL 13/13] docs: Document QMP event rate limiting
  2015-10-27  9:21 [Qemu-devel] [PULL 00/13] QMP and QObject patches Markus Armbruster
                   ` (11 preceding siblings ...)
  2015-10-27  9:21 ` [Qemu-devel] [PULL 12/13] monitor: Throttle event VSERPORT_CHANGE separately by "id" Markus Armbruster
@ 2015-10-27  9:21 ` Markus Armbruster
  2015-10-27 13:06 ` [Qemu-devel] [PULL 00/13] QMP and QObject patches Peter Maydell
  13 siblings, 0 replies; 16+ messages in thread
From: Markus Armbruster @ 2015-10-27  9:21 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] 16+ messages in thread

* Re: [Qemu-devel] [PULL 00/13] QMP and QObject patches
  2015-10-27  9:21 [Qemu-devel] [PULL 00/13] QMP and QObject patches Markus Armbruster
                   ` (12 preceding siblings ...)
  2015-10-27  9:21 ` [Qemu-devel] [PULL 13/13] docs: Document QMP event rate limiting Markus Armbruster
@ 2015-10-27 13:06 ` Peter Maydell
  2015-10-27 14:28   ` Markus Armbruster
  13 siblings, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2015-10-27 13:06 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: QEMU Developers

On 27 October 2015 at 09:21, Markus Armbruster <armbru@redhat.com> wrote:
> The following changes since commit 9666248a85fd889bfb6118f769e9c73039b998ed:
>
>   Merge remote-tracking branch 'remotes/sstabellini/tags/xen-2015-10-26' into staging (2015-10-26 13:13:38 +0000)
>
> are available in the git repository at:
>
>   git://repo.or.cz/qemu/armbru.git tags/pull-monitor-2015-10-27
>
> for you to fetch changes up to 8a4e1a93634a95997e227f3470453f07cf159fcf:
>
>   docs: Document QMP event rate limiting (2015-10-27 10:05:54 +0100)
>
> ----------------------------------------------------------------
> QMP and QObject patches
>
> ----------------------------------------------------------------
> Markus Armbruster (13):
>       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
>       monitor: Turn monitor_qapi_event_state[] into a hash table
>       monitor: Throttle event VSERPORT_CHANGE separately by "id"
>       docs: Document QMP event rate limiting

Hi. I'm afraid this fails to build against our minimum
glib version:

Undefined symbols for architecture x86_64:
  "_g_hash_table_add", referenced from:
      _monitor_qapi_event_queue in monitor.o
ld: symbol(s) not found for architecture x86_64

g_hash_table_add() was only introduced in glib 2.32.

thanks
-- PMM

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

* Re: [Qemu-devel] [PULL 00/13] QMP and QObject patches
  2015-10-27 13:06 ` [Qemu-devel] [PULL 00/13] QMP and QObject patches Peter Maydell
@ 2015-10-27 14:28   ` Markus Armbruster
  0 siblings, 0 replies; 16+ messages in thread
From: Markus Armbruster @ 2015-10-27 14:28 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Developers

Peter Maydell <peter.maydell@linaro.org> writes:

> On 27 October 2015 at 09:21, Markus Armbruster <armbru@redhat.com> wrote:
>> The following changes since commit 9666248a85fd889bfb6118f769e9c73039b998ed:
>>
>>   Merge remote-tracking branch 'remotes/sstabellini/tags/xen-2015-10-26' into staging (2015-10-26 13:13:38 +0000)
>>
>> are available in the git repository at:
>>
>>   git://repo.or.cz/qemu/armbru.git tags/pull-monitor-2015-10-27
>>
>> for you to fetch changes up to 8a4e1a93634a95997e227f3470453f07cf159fcf:
>>
>>   docs: Document QMP event rate limiting (2015-10-27 10:05:54 +0100)
>>
>> ----------------------------------------------------------------
>> QMP and QObject patches
>>
>> ----------------------------------------------------------------
>> Markus Armbruster (13):
>>       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
>>       monitor: Turn monitor_qapi_event_state[] into a hash table
>>       monitor: Throttle event VSERPORT_CHANGE separately by "id"
>>       docs: Document QMP event rate limiting
>
> Hi. I'm afraid this fails to build against our minimum
> glib version:
>
> Undefined symbols for architecture x86_64:
>   "_g_hash_table_add", referenced from:
>       _monitor_qapi_event_queue in monitor.o
> ld: symbol(s) not found for architecture x86_64
>
> g_hash_table_add() was only introduced in glib 2.32.

My mistake, thanks for catching it!

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

end of thread, other threads:[~2015-10-27 14:28 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-27  9:21 [Qemu-devel] [PULL 00/13] QMP and QObject patches Markus Armbruster
2015-10-27  9:21 ` [Qemu-devel] [PULL 01/13] qobject: Drop QObject_HEAD Markus Armbruster
2015-10-27  9:21 ` [Qemu-devel] [PULL 02/13] qbool: Make conversion from QObject * accept null Markus Armbruster
2015-10-27  9:21 ` [Qemu-devel] [PULL 03/13] qdict: " Markus Armbruster
2015-10-27  9:21 ` [Qemu-devel] [PULL 04/13] qfloat qint: " Markus Armbruster
2015-10-27  9:21 ` [Qemu-devel] [PULL 05/13] qlist: " Markus Armbruster
2015-10-27  9:21 ` [Qemu-devel] [PULL 06/13] qstring: " Markus Armbruster
2015-10-27  9:21 ` [Qemu-devel] [PULL 07/13] monitor: Reduce casting of QAPI event QDict Markus Armbruster
2015-10-27  9:21 ` [Qemu-devel] [PULL 08/13] monitor: Simplify event throttling Markus Armbruster
2015-10-27  9:21 ` [Qemu-devel] [PULL 09/13] monitor: Switch from timer_new() to timer_new_ns() Markus Armbruster
2015-10-27  9:21 ` [Qemu-devel] [PULL 10/13] monitor: Split MonitorQAPIEventConf off MonitorQAPIEventState Markus Armbruster
2015-10-27  9:21 ` [Qemu-devel] [PULL 11/13] monitor: Turn monitor_qapi_event_state[] into a hash table Markus Armbruster
2015-10-27  9:21 ` [Qemu-devel] [PULL 12/13] monitor: Throttle event VSERPORT_CHANGE separately by "id" Markus Armbruster
2015-10-27  9:21 ` [Qemu-devel] [PULL 13/13] docs: Document QMP event rate limiting Markus Armbruster
2015-10-27 13:06 ` [Qemu-devel] [PULL 00/13] QMP and QObject patches Peter Maydell
2015-10-27 14:28   ` Markus Armbruster

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).