[Qemu-devel] [RFC 0/3] qemu-ga: support for sending events

[Qemu-devel] [RFC 0/3] qemu-ga: support for sending events

[Tomáš Golembiovský]

This is just a draft, or a request for comments if you will.

This patch sets drafts the support of sending events by QEMU Guest Agent.
Events can plan important role in monitoring of the guest OS behaviour. The
range of use cases ranges from events important for scheduling, e.g. memory and
CPU usage statistics, to things like changes to IP addresses on network
interfaces to for example changes in the list of active users.

For now the patch set adds single periodic callback function to the GA main
loop that can perform checks and trigger events that have occured since
previous run of the callback.

We can of course take it one step further and add a general framwork for
periodically running any of the already implemented commands. Add a function
that would maintain a list of registered checks. Client would use some command
(register-monitor-command) passing it a command name and timeout in seconds and
the monitoring handler would then run the specified command and report the
result... or report only if the return value changed since previous invocation.
This feature would remove part of the communication overhead between client and
GA.

So before I invest any more time in either of these approaches, tell me. Would
somethign like this be wanted or is that too controversial? Any other thoughts
and ideas?

Tomáš Golembiovský (3):
  qemu-ga: add support for events
  qemu-ga: add simple event reporting memory statistics
  qemu-ga: add support for periodic command runner

 Makefile               |  7 +++-
 qga/Makefile.objs      |  2 +-
 qga/channel-posix.c    |  8 +++++
 qga/channel-win32.c    |  6 ++++
 qga/channel.h          |  1 +
 qga/guest-agent-core.h |  1 +
 qga/main.c             | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++
 qga/qapi-event.json    | 35 ++++++++++++++++++
 qga/qapi-schema.json   |  2 ++
 9 files changed, 158 insertions(+), 2 deletions(-)
 create mode 100644 qga/qapi-event.json

-- 
2.13.1

[Qemu-devel] [RFC 1/3] qemu-ga: add support for events

[Tomáš Golembiovský]

Events can play an integral role when monitoring internal state of the
guest OS. This patch adds the core functionality for adding events to
QEMU Guest Agent.

Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
---
 Makefile               |  7 +++++-
 qga/Makefile.objs      |  2 +-
 qga/channel-posix.c    |  8 +++++++
 qga/channel-win32.c    |  6 +++++
 qga/channel.h          |  1 +
 qga/guest-agent-core.h |  1 +
 qga/main.c             | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++
 qga/qapi-event.json    |  2 ++
 qga/qapi-schema.json   |  2 ++
 9 files changed, 86 insertions(+), 2 deletions(-)
 create mode 100644 qga/qapi-event.json

diff --git a/Makefile b/Makefile
index c830d7a46c..03e2174a18 100644
--- a/Makefile
+++ b/Makefile
@@ -408,6 +408,11 @@ $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
 	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \
 		$(gen-out-type) -o qga/qapi-generated -p "qga-" $<, \
 		"GEN","$@")
+qga/qapi-generated/qga-qapi-event.c qga/qapi-generated/qga-qapi-event.h :\
+$(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-event.py $(qapi-py)
+	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-event.py \
+		$(gen-out-type) -o qga/qapi-generated -p "qga-" $<, \
+		"GEN","$@")
 
 qapi-modules = $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/qapi/common.json \
                $(SRC_PATH)/qapi/block.json $(SRC_PATH)/qapi/block-core.json \
@@ -441,7 +446,7 @@ $(qapi-modules) $(SRC_PATH)/scripts/qapi-introspect.py $(qapi-py)
 		$(gen-out-type) -o "." $<, \
 		"GEN","$@")
 
-QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h qga-qmp-commands.h)
+QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h qga-qmp-commands.h qga-qapi-event.h)
 $(qga-obj-y) qemu-ga.o: $(QGALIB_GEN)
 
 qemu-ga$(EXESUF): $(qga-obj-y) $(COMMON_LDADDS)
diff --git a/qga/Makefile.objs b/qga/Makefile.objs
index 1c5986c0bb..24399b6325 100644
--- a/qga/Makefile.objs
+++ b/qga/Makefile.objs
@@ -3,6 +3,6 @@ qga-obj-$(CONFIG_POSIX) += commands-posix.o channel-posix.o
 qga-obj-$(CONFIG_WIN32) += commands-win32.o channel-win32.o service-win32.o
 qga-obj-$(CONFIG_WIN32) += vss-win32.o
 qga-obj-y += qapi-generated/qga-qapi-types.o qapi-generated/qga-qapi-visit.o
-qga-obj-y += qapi-generated/qga-qmp-marshal.o
+qga-obj-y += qapi-generated/qga-qmp-marshal.o qapi-generated/qga-qapi-event.o
 
 qga-vss-dll-obj-$(CONFIG_QGA_VSS) += vss-win32/
diff --git a/qga/channel-posix.c b/qga/channel-posix.c
index 3f34465159..22e440724c 100644
--- a/qga/channel-posix.c
+++ b/qga/channel-posix.c
@@ -118,6 +118,14 @@ static int ga_channel_client_add(GAChannel *c, int fd)
     return 0;
 }
 
+gboolean ga_channel_client_attached(GAChannel *c)
+{
+    g_assert(c);
+    /* TODO: make this work with all methods. following works only with
+     * unix-listen */
+    return c->client_channel != NULL;
+}
+
 static gboolean ga_channel_open(GAChannel *c, const gchar *path,
                                 GAChannelMethod method, int fd)
 {
diff --git a/qga/channel-win32.c b/qga/channel-win32.c
index 7e6dc4d26f..b62a6a3859 100644
--- a/qga/channel-win32.c
+++ b/qga/channel-win32.c
@@ -315,6 +315,12 @@ static gboolean ga_channel_open(GAChannel *c, GAChannelMethod method,
     return true;
 }
 
+gboolean ga_channel_client_attached(GAChannel *c)
+{
+    /* TODO: make this work with all methods */
+    return true;
+}
+
 GAChannel *ga_channel_new(GAChannelMethod method, const gchar *path,
                           int listen_fd, GAChannelCallback cb, gpointer opaque)
 {
diff --git a/qga/channel.h b/qga/channel.h
index 1778416115..030ec9e551 100644
--- a/qga/channel.h
+++ b/qga/channel.h
@@ -30,5 +30,6 @@ GAChannel *ga_channel_new(GAChannelMethod method, const gchar *path,
 void ga_channel_free(GAChannel *c);
 GIOStatus ga_channel_read(GAChannel *c, gchar *buf, gsize size, gsize *count);
 GIOStatus ga_channel_write_all(GAChannel *c, const gchar *buf, gsize size);
+gboolean ga_channel_client_attached(GAChannel *c);
 
 #endif
diff --git a/qga/guest-agent-core.h b/qga/guest-agent-core.h
index 3e8a4acff2..47d6c73458 100644
--- a/qga/guest-agent-core.h
+++ b/qga/guest-agent-core.h
@@ -13,6 +13,7 @@
 #include "qapi/qmp/dispatch.h"
 #include "qemu-common.h"
 #include "qga-qmp-commands.h"
+#include "qga-qapi-event.h"
 
 #define QGA_READ_COUNT_DEFAULT 4096
 
diff --git a/qga/main.c b/qga/main.c
index cc58d2b53d..f16abb5cbb 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -23,6 +23,8 @@
 #include "qapi/qmp/qjson.h"
 #include "qga/guest-agent-core.h"
 #include "qemu/module.h"
+#include "qapi/qmp-event.h"
+#include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qerror.h"
 #include "qapi/qmp/dispatch.h"
 #include "qga/channel.h"
@@ -674,6 +676,59 @@ static gboolean channel_event_cb(GIOCondition condition, gpointer data)
     return true;
 }
 
+/* TODO: HACK HACK HACK... can't we get a GAstate somehow? */
+QDict *queued_event;
+static void ga_event_emit(qga_QAPIEvent event, QDict *qdict, Error **errp)
+{
+    if (queued_event) {
+        error_setg(errp, "unsent event already waiting");
+    } else {
+        QINCREF(qdict);
+        queued_event = qdict;
+    }
+}
+/* HACK HACK HACK!!! */
+
+static gboolean monitoring_cb(gpointer data)
+{
+    Error *err = NULL;
+    GAState *s = (GAState *)data;
+
+    g_assert(s->channel);
+    g_warning("monitoring!");
+
+    if (!ga_channel_client_attached(s->channel)) {
+        goto ok;
+    }
+
+    /* TODO: call something */
+    goto ok;
+
+/*fail:*/
+    g_assert(err);
+    g_warning("%s", error_get_pretty(err));
+    error_free(err);
+
+ok:
+    /* Always return true. False would remove this callback. */
+    return true;
+}
+
+static gboolean monitoring_init(GAState *s)
+{
+    if (g_timeout_add_seconds(5, monitoring_cb, (gpointer)s) <= 0) {
+        g_error("failed to create monitoring timer");
+        goto fail;
+    }
+    g_debug("monitoring timer created");
+
+    qmp_event_set_func_emit(ga_event_emit);
+    return true;
+
+fail:
+    return false;
+}
+
 static gboolean channel_init(GAState *s, const gchar *method, const gchar *path,
                              int listen_fd)
 {
@@ -1330,6 +1385,10 @@ static int run_agent(GAState *s, GAConfig *config, int socket_activation)
         g_critical("failed to initialize guest agent channel");
         return EXIT_FAILURE;
     }
+
+    /* TODO: error handling? */
+    monitoring_init(ga_state);
+
 #ifndef _WIN32
     g_main_loop_run(ga_state->main_loop);
 #else
diff --git a/qga/qapi-event.json b/qga/qapi-event.json
new file mode 100644
index 0000000000..9c14e4609e
--- /dev/null
+++ b/qga/qapi-event.json
@@ -0,0 +1,2 @@
+# *-*- Mode: Python -*-*
+
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 03743ab905..f30ba183bb 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -1,5 +1,7 @@
 # *-*- Mode: Python -*-*
 
+{ 'include': 'qapi-event.json' }
+
 ##
 #
 # General note concerning the use of guest agent interfaces:
-- 
2.13.1

Re: [Qemu-devel] [RFC 1/3] qemu-ga: add support for events

[Eric Blake]

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

On 06/23/2017 08:02 AM, Tomáš Golembiovský wrote:
> Events can play an integral role when monitoring internal state of the
> guest OS. This patch adds the core functionality for adding events to
> QEMU Guest Agent.

Will sending events confuse existing clients that aren't expecting them?
Do we need to add some sort of protocol handshake when the client first
connects so that we know whether the client is able to parse events
(including ignoring unknown events)?  (It's a shame that events were
built into QMP from the beginning, so we've never had to figure out how
to portably hand-shake a client/server negotiation on additional
features to be used across the wire)

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


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

Re: [Qemu-devel] [RFC 1/3] qemu-ga: add support for events

[Tomáš Golembiovský]

On Fri, 7 Jul 2017 15:53:37 -0500
Eric Blake <eblake@redhat.com> wrote:

> On 06/23/2017 08:02 AM, Tomáš Golembiovský wrote:
> > Events can play an integral role when monitoring internal state of the
> > guest OS. This patch adds the core functionality for adding events to
> > QEMU Guest Agent.  
> 
> Will sending events confuse existing clients that aren't expecting them?
> Do we need to add some sort of protocol handshake when the client first
> connects so that we know whether the client is able to parse events
> (including ignoring unknown events)?  (It's a shame that events were
> built into QMP from the beginning, so we've never had to figure out how
> to portably hand-shake a client/server negotiation on additional
> features to be used across the wire)

You're probably right. Something like this will be necessary.

> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
> 


-- 
Tomáš Golembiovský <tgolembi@redhat.com>

[Qemu-devel] [RFC 2/3] qemu-ga: add simple event reporting memory statistics

[Tomáš Golembiovský]

work in progress

Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
---
 qga/main.c          | 19 ++++++++++++++++---
 qga/qapi-event.json | 16 ++++++++++++++++
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/qga/main.c b/qga/main.c
index f16abb5cbb..a9586e7513 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -695,16 +695,29 @@ static gboolean monitoring_cb(gpointer data)
     GAState *s = (GAState *)data;
 
     g_assert(s->channel);
-    g_warning("monitoring!");
 
     if (!ga_channel_client_attached(s->channel)) {
         goto ok;
     }
 
-    /* TODO: call something */
+    /* Fire an event */
+    qapi_event_send_guest_heartbeat(12345, &err);
+    if (err) {
+        goto fail;
+    }
+
+    if (queued_event) {
+        int ret;
+        ret = send_response(s, QOBJECT(queued_event));
+        QDECREF(queued_event);
+        queued_event = NULL;
+        if (ret < 0) {
+            g_warning("error sending event: %s", strerror(-ret));
+        }
+    }
     goto ok;
 
-/*fail:*/
+fail:
     g_assert(err);
     g_warning("%s", error_get_pretty(err));
     error_free(err);
diff --git a/qga/qapi-event.json b/qga/qapi-event.json
index 9c14e4609e..3d48ddb214 100644
--- a/qga/qapi-event.json
+++ b/qga/qapi-event.json
@@ -1,2 +1,18 @@
 # *-*- Mode: Python -*-*
 
+##
+# @GUEST_HEARTBEAT:
+#
+# Mostrly returns memory statistics. TODO
+#
+# @free-ram: Amount of free memory in kB
+#
+# Since: 2.10
+#
+# Example:
+#
+# <- { "event": "GUEST_HEARTBEAT", "data": { "free-ram": "12345" },
+#      "timestamp": { "seconds": 1267040730, "microseconds": 682951 } }
+#
+##
+{ 'event': 'GUEST_HEARTBEAT', 'data': { 'free-ram': 'size' } }
-- 
2.13.1

[Qemu-devel] [RFC 3/3] qemu-ga: add support for periodic command runner

[Tomáš Golembiovský]

This adds a support for periodicaly triggering arbitrary command
reporting its output.

Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
---
 qga/main.c          | 26 ++++++++++++++++++++++++++
 qga/qapi-event.json | 17 +++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/qga/main.c b/qga/main.c
index a9586e7513..612a6646e4 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -715,6 +715,32 @@ static gboolean monitoring_cb(gpointer data)
             g_warning("error sending event: %s", strerror(-ret));
         }
     }
+
+    /* Run arbitrary command */
+    const char *command = "guest-get-users";
+    QDict *args = qdict_new();
+
+    QmpCommand *cmd = qmp_find_command(&ga_commands, command);
+    g_assert(cmd);
+    g_assert(cmd->fn);
+    QObject *ret;
+    qmp_marshal_guest_get_users(args, &ret, &err);
+    QDECREF(args);
+    if (err) {
+        goto fail;
+    }
+    qapi_event_send_guest_monitor_command(command, ret, &err);
+
+    if (queued_event) {
+        int ret;
+        ret = send_response(s, QOBJECT(queued_event));
+        QDECREF(queued_event);
+        queued_event = NULL;
+        if (ret < 0) {
+            g_warning("error sending event: %s", strerror(-ret));
+        }
+    }
+
     goto ok;
 
 fail:
diff --git a/qga/qapi-event.json b/qga/qapi-event.json
index 3d48ddb214..e31ea0f0e9 100644
--- a/qga/qapi-event.json
+++ b/qga/qapi-event.json
@@ -16,3 +16,20 @@
 #
 ##
 { 'event': 'GUEST_HEARTBEAT', 'data': { 'free-ram': 'size' } }
+
+##
+# @GUEST_MONITOR_COMMAND:
+#
+# Run periodicaly arbitrary command and return it's result.
+#
+# @command: The executed command.
+# @return: Returned value.
+#
+# Since: 2.10
+#
+# Example:
+#
+# TODO
+#
+##
+{ 'event': 'GUEST_MONITOR_COMMAND', 'data': { 'command': 'str', 'return': 'any' } }
-- 
2.13.1

Re: [Qemu-devel] [RFC 0/3] qemu-ga: support for sending events

[Marc-André Lureau]

Hi

On Fri, Jun 23, 2017 at 3:03 PM Tomáš Golembiovský <tgolembi@redhat.com>
wrote:

> This is just a draft, or a request for comments if you will.
>
> This patch sets drafts the support of sending events by QEMU Guest Agent.
> Events can plan important role in monitoring of the guest OS behaviour. The
> range of use cases ranges from events important for scheduling, e.g.
> memory and
> CPU usage statistics, to things like changes to IP addresses on network
> interfaces to for example changes in the list of active users.
>
> For now the patch set adds single periodic callback function to the GA main
> loop that can perform checks and trigger events that have occured since
> previous run of the callback.
>
> We can of course take it one step further and add a general framwork for
> periodically running any of the already implemented commands. Add a
> function
> that would maintain a list of registered checks. Client would use some
> command
> (register-monitor-command) passing it a command name and timeout in
> seconds and
> the monitoring handler would then run the specified command and report the
> result... or report only if the return value changed since previous
> invocation.
> This feature would remove part of the communication overhead between
> client and
> GA.
>
> So before I invest any more time in either of these approaches, tell me.
> Would
> somethign like this be wanted or is that too controversial? Any other
> thoughts
> and ideas?
>
>
It doesn't feel wrong, but Is there really too much overhead and/or latency
if a request is periodic from the client? ie did you do some measurements
before coming to this proposal?

Tomáš Golembiovský (3):
>   qemu-ga: add support for events
>   qemu-ga: add simple event reporting memory statistics
>   qemu-ga: add support for periodic command runner
>
>  Makefile               |  7 +++-
>  qga/Makefile.objs      |  2 +-
>  qga/channel-posix.c    |  8 +++++
>  qga/channel-win32.c    |  6 ++++
>  qga/channel.h          |  1 +
>  qga/guest-agent-core.h |  1 +
>  qga/main.c             | 98
> ++++++++++++++++++++++++++++++++++++++++++++++++++
>  qga/qapi-event.json    | 35 ++++++++++++++++++
>  qga/qapi-schema.json   |  2 ++
>  9 files changed, 158 insertions(+), 2 deletions(-)
>  create mode 100644 qga/qapi-event.json
>
> --
> 2.13.1
>
>
> --
Marc-André Lureau

Re: [Qemu-devel] [RFC 0/3] qemu-ga: support for sending events

[Tomáš Golembiovský]

Hi,

On Fri, 23 Jun 2017 13:25:34 +0000
Marc-André Lureau <marcandre.lureau@gmail.com> wrote:

> Hi
> 
> On Fri, Jun 23, 2017 at 3:03 PM Tomáš Golembiovský <tgolembi@redhat.com>
> wrote:
> 
> > This is just a draft, or a request for comments if you will.
> >
> > This patch sets drafts the support of sending events by QEMU Guest Agent.
> > Events can plan important role in monitoring of the guest OS behaviour. The
> > range of use cases ranges from events important for scheduling, e.g.
> > memory and
> > CPU usage statistics, to things like changes to IP addresses on network
> > interfaces to for example changes in the list of active users.
> >
> > For now the patch set adds single periodic callback function to the GA main
> > loop that can perform checks and trigger events that have occured since
> > previous run of the callback.
> >
> > We can of course take it one step further and add a general framwork for
> > periodically running any of the already implemented commands. Add a
> > function
> > that would maintain a list of registered checks. Client would use some
> > command
> > (register-monitor-command) passing it a command name and timeout in
> > seconds and
> > the monitoring handler would then run the specified command and report the
> > result... or report only if the return value changed since previous
> > invocation.
> > This feature would remove part of the communication overhead between
> > client and
> > GA.
> >
> > So before I invest any more time in either of these approaches, tell me.
> > Would
> > somethign like this be wanted or is that too controversial? Any other
> > thoughts
> > and ideas?
> >
> >  
> It doesn't feel wrong, but Is there really too much overhead and/or latency
> if a request is periodic from the client? ie did you do some measurements
> before coming to this proposal?

No, I didn't do any measurements. And it may be even true that in the
grand scheme of things the overhead/latency may be insignificant, if we
imagine a client that repeatedly calls about 5 to 10 commands every 5 or
10 seconds. Still, it just feels like a more correct approach to me. But
that may be just my feeling, that's why I brought this to the list to
get the opinion of others.

    Tomas

> 
> Tomáš Golembiovský (3):
> >   qemu-ga: add support for events
> >   qemu-ga: add simple event reporting memory statistics
> >   qemu-ga: add support for periodic command runner
> >
> >  Makefile               |  7 +++-
> >  qga/Makefile.objs      |  2 +-
> >  qga/channel-posix.c    |  8 +++++
> >  qga/channel-win32.c    |  6 ++++
> >  qga/channel.h          |  1 +
> >  qga/guest-agent-core.h |  1 +
> >  qga/main.c             | 98
> > ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  qga/qapi-event.json    | 35 ++++++++++++++++++
> >  qga/qapi-schema.json   |  2 ++
> >  9 files changed, 158 insertions(+), 2 deletions(-)
> >  create mode 100644 qga/qapi-event.json
> >
> > --
> > 2.13.1
> >
> >
> > --  
> Marc-André Lureau


-- 
Tomáš Golembiovský <tgolembi@redhat.com>

Re: [Qemu-devel] [RFC 0/3] qemu-ga: support for sending events

[Tomáš Golembiovský]

Hi,

On Sun, 25 Jun 2017 23:25:17 +0200
Tomáš Golembiovský <tgolembi@redhat.com> wrote:

> Hi,
> 
> On Fri, 23 Jun 2017 13:25:34 +0000
> Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
> 
> > Hi
> > 
> > On Fri, Jun 23, 2017 at 3:03 PM Tomáš Golembiovský <tgolembi@redhat.com>
> > wrote:
> >   
> > > This is just a draft, or a request for comments if you will.
> > >
> > > This patch sets drafts the support of sending events by QEMU Guest Agent.
> > > Events can plan important role in monitoring of the guest OS behaviour. The
> > > range of use cases ranges from events important for scheduling, e.g.
> > > memory and
> > > CPU usage statistics, to things like changes to IP addresses on network
> > > interfaces to for example changes in the list of active users.
> > >
> > > For now the patch set adds single periodic callback function to the GA main
> > > loop that can perform checks and trigger events that have occured since
> > > previous run of the callback.
> > >
> > > We can of course take it one step further and add a general framwork for
> > > periodically running any of the already implemented commands. Add a
> > > function
> > > that would maintain a list of registered checks. Client would use some
> > > command
> > > (register-monitor-command) passing it a command name and timeout in
> > > seconds and
> > > the monitoring handler would then run the specified command and report the
> > > result... or report only if the return value changed since previous
> > > invocation.
> > > This feature would remove part of the communication overhead between
> > > client and
> > > GA.
> > >
> > > So before I invest any more time in either of these approaches, tell me.
> > > Would
> > > somethign like this be wanted or is that too controversial? Any other
> > > thoughts
> > > and ideas?
> > >
> > >    
> > It doesn't feel wrong, but Is there really too much overhead and/or latency
> > if a request is periodic from the client? ie did you do some measurements
> > before coming to this proposal?  
> 
> No, I didn't do any measurements. And it may be even true that in the
> grand scheme of things the overhead/latency may be insignificant, if we
> imagine a client that repeatedly calls about 5 to 10 commands every 5 or
> 10 seconds. Still, it just feels like a more correct approach to me. But
> that may be just my feeling, that's why I brought this to the list to
> get the opinion of others.
> 

This is not really a wild discussion I have anticipated. Or does the
silence mean I should drop the idea?

If some measurements are necessary can you suggest how to construct the
benchmark? What numbers would be convincing to support the idea?

Still, let me restate that I see it more as an architectural decision
rather than something where latency or overhead would be the main
factor.


    Tomas

> 
> > 
> > Tomáš Golembiovský (3):  
> > >   qemu-ga: add support for events
> > >   qemu-ga: add simple event reporting memory statistics
> > >   qemu-ga: add support for periodic command runner
> > >
> > >  Makefile               |  7 +++-
> > >  qga/Makefile.objs      |  2 +-
> > >  qga/channel-posix.c    |  8 +++++
> > >  qga/channel-win32.c    |  6 ++++
> > >  qga/channel.h          |  1 +
> > >  qga/guest-agent-core.h |  1 +
> > >  qga/main.c             | 98
> > > ++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  qga/qapi-event.json    | 35 ++++++++++++++++++
> > >  qga/qapi-schema.json   |  2 ++
> > >  9 files changed, 158 insertions(+), 2 deletions(-)
> > >  create mode 100644 qga/qapi-event.json
> > >
> > > --
> > > 2.13.1
> > >
> > >
> > > --    
> > Marc-André Lureau  
> 
> 
> -- 
> Tomáš Golembiovský <tgolembi@redhat.com>


-- 
Tomáš Golembiovský <tgolembi@redhat.com>

Re: [Qemu-devel] [RFC 0/3] qemu-ga: support for sending events

[Eric Blake]

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

On 06/23/2017 08:02 AM, Tomáš Golembiovský wrote:
> This is just a draft, or a request for comments if you will.
> 
> This patch sets drafts the support of sending events by QEMU Guest Agent.
> Events can plan important role in monitoring of the guest OS behaviour. The
> range of use cases ranges from events important for scheduling, e.g. memory and
> CPU usage statistics, to things like changes to IP addresses on network
> interfaces to for example changes in the list of active users.
> 
> For now the patch set adds single periodic callback function to the GA main
> loop that can perform checks and trigger events that have occured since
> previous run of the callback.

How do we guarantee that the guest cannot flood qemu with too many events?

Obviously, qga is already used where we (in general) trust the guest to
not be malicious, but we still have to assume that a guest can be
compromised, and will try to abuse qga to escape to an attack against qemu.

> 
> We can of course take it one step further and add a general framwork for
> periodically running any of the already implemented commands. Add a function
> that would maintain a list of registered checks. Client would use some command
> (register-monitor-command) passing it a command name and timeout in seconds and
> the monitoring handler would then run the specified command and report the
> result... or report only if the return value changed since previous invocation.
> This feature would remove part of the communication overhead between client and
> GA.
> 
> So before I invest any more time in either of these approaches, tell me. Would
> somethign like this be wanted or is that too controversial? Any other thoughts
> and ideas?
> 
-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


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

Re: [Qemu-devel] [RFC 0/3] qemu-ga: support for sending events

[Tomáš Golembiovský]

On Fri, 7 Jul 2017 15:55:31 -0500
Eric Blake <eblake@redhat.com> wrote:

> On 06/23/2017 08:02 AM, Tomáš Golembiovský wrote:
> > This is just a draft, or a request for comments if you will.
> > 
> > This patch sets drafts the support of sending events by QEMU Guest Agent.
> > Events can plan important role in monitoring of the guest OS behaviour. The
> > range of use cases ranges from events important for scheduling, e.g. memory and
> > CPU usage statistics, to things like changes to IP addresses on network
> > interfaces to for example changes in the list of active users.
> > 
> > For now the patch set adds single periodic callback function to the GA main
> > loop that can perform checks and trigger events that have occured since
> > previous run of the callback.  
> 
> How do we guarantee that the guest cannot flood qemu with too many events?
> 
> Obviously, qga is already used where we (in general) trust the guest to
> not be malicious, but we still have to assume that a guest can be
> compromised, and will try to abuse qga to escape to an attack against qemu.
> 

If you mean situation where something in guest will be triggering events
that qga will be sending then we can implement some throttling in qga. I
belive qemu does something similar for QMP events.

But if you're considering situation where something kills qga and will
attach itself to the serial channel then this is of course more
complex issue.

> > 
> > We can of course take it one step further and add a general framwork for
> > periodically running any of the already implemented commands. Add a function
> > that would maintain a list of registered checks. Client would use some command
> > (register-monitor-command) passing it a command name and timeout in seconds and
> > the monitoring handler would then run the specified command and report the
> > result... or report only if the return value changed since previous invocation.
> > This feature would remove part of the communication overhead between client and
> > GA.
> > 
> > So before I invest any more time in either of these approaches, tell me. Would
> > somethign like this be wanted or is that too controversial? Any other thoughts
> > and ideas?
> >   
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
> 


-- 
Tomáš Golembiovský <tgolembi@redhat.com>