[Qemu-devel] [PATCH v0 0/8]: VNC events and cleanup

[Qemu-devel] [PATCH v0 0/8]: VNC events and cleanup

[Luiz Capitulino]

 Hi there,

 This series contains two VNC related changes. First a small cleanup
is done in the current 'query-vnc' command _response_, then the following
QMP events are introduced:

- VNC_CONNECTED: emitted when a VNC client establishes a connection
- VNC_INITIALIZED: emitted when the VNC session is made active
- VNC_DISCONNECTED: emitted when the conection is closed

 The only issue is the current events documentation. I'm using the
current format which is quite bad, things will improve when we have
proper documentation support (being worked out by Markus).

 Thanks.

[Qemu-devel] [PATCH 1/8] VNC: Use 'enabled' key instead of 'status'

[Luiz Capitulino]

Currently the 'status' key is a string whose value can be
"disabled" or "enabled", change it to the QMP's standard
'enabled' key, which is a bool.

Note that 'status' in being dropped and this wouldn't be
allowed if QMP were stable.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 vnc.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/vnc.c b/vnc.c
index 58eac73..ef86ef7 100644
--- a/vnc.c
+++ b/vnc.c
@@ -254,7 +254,7 @@ void do_info_vnc_print(Monitor *mon, const QObject *data)
     QList *clients;
 
     server = qobject_to_qdict(data);
-    if (strcmp(qdict_get_str(server, "status"), "disabled") == 0) {
+    if (qdict_get_bool(server, "enabled") == 0) {
         monitor_printf(mon, "Server: disabled\n");
         return;
     }
@@ -282,7 +282,7 @@ void do_info_vnc_print(Monitor *mon, const QObject *data)
  *
  * The main QDict contains the following:
  *
- * - "status": "disabled" or "enabled"
+ * - "enabled": true or false
  * - "host": server's IP address
  * - "service": server's port number
  * - "auth": authentication method (optional)
@@ -297,13 +297,13 @@ void do_info_vnc_print(Monitor *mon, const QObject *data)
  *
  * Example:
  *
- * { "status": "enabled", "host": "0.0.0.0", "service": "50402", "auth": "vnc",
+ * { "enabled": true, "host": "0.0.0.0", "service": "50402", "auth": "vnc",
  *   "clients": [ { "host": "127.0.0.1", "service": "50401" } ] }
  */
 void do_info_vnc(Monitor *mon, QObject **ret_data)
 {
     if (vnc_display == NULL || vnc_display->display == NULL) {
-        *ret_data = qobject_from_jsonf("{ 'status': 'disabled' }");
+        *ret_data = qobject_from_jsonf("{ 'enabled': false }");
     } else {
         QDict *qdict;
         QList *clist;
@@ -319,7 +319,7 @@ void do_info_vnc(Monitor *mon, QObject **ret_data)
             }
         }
 
-        *ret_data = qobject_from_jsonf("{ 'status': 'enabled', 'clients': %p }",
+        *ret_data = qobject_from_jsonf("{ 'enabled': true, 'clients': %p }",
                                        QOBJECT(clist));
         assert(*ret_data != NULL);
 
-- 
1.6.6

[Qemu-devel] [PATCH 2/8] VNC: Make 'auth' key mandatory

[Luiz Capitulino]

There is no reason to have it as optional and the code
in the server and client gets slightly simpler if the
key is mandatory.

While there also do some cleanup on how the server info is
collected.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 vnc.c |   26 ++++++++++++++------------
 1 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/vnc.c b/vnc.c
index ef86ef7..b5f3b64 100644
--- a/vnc.c
+++ b/vnc.c
@@ -122,7 +122,7 @@ static int put_addr_qdict(QDict *qdict, struct sockaddr_storage *sa,
     return 0;
 }
 
-static int vnc_qdict_local_addr(QDict *qdict, int fd)
+static int vnc_server_addr_put(QDict *qdict, int fd)
 {
     struct sockaddr_storage sa;
     socklen_t salen;
@@ -199,6 +199,16 @@ static const char *vnc_auth_name(VncDisplay *vd) {
     return "unknown";
 }
 
+static int vnc_server_info_put(QDict *qdict)
+{
+    if (vnc_server_addr_put(qdict, vnc_display->lsock) < 0) {
+        return -1;
+    }
+
+    qdict_put(qdict, "auth", qstring_from_str(vnc_auth_name(vnc_display)));
+    return 0;
+}
+
 static QDict *do_info_vnc_client(Monitor *mon, VncState *client)
 {
     QDict *qdict;
@@ -263,8 +273,7 @@ void do_info_vnc_print(Monitor *mon, const QObject *data)
     monitor_printf(mon, "     address: %s:%s\n",
                    qdict_get_str(server, "host"),
                    qdict_get_str(server, "service"));
-    monitor_printf(mon, "        auth: %s\n",
-        qdict_haskey(server, "auth") ? qdict_get_str(server, "auth") : "none");
+    monitor_printf(mon, "        auth: %s\n", qdict_get_str(server, "auth"));
 
     clients = qdict_get_qlist(server, "clients");
     if (qlist_empty(clients)) {
@@ -285,7 +294,7 @@ void do_info_vnc_print(Monitor *mon, const QObject *data)
  * - "enabled": true or false
  * - "host": server's IP address
  * - "service": server's port number
- * - "auth": authentication method (optional)
+ * - "auth": authentication method
  * - "clients": a QList of all connected clients
  *
  * Clients are described by a QDict, with the following information:
@@ -323,14 +332,7 @@ void do_info_vnc(Monitor *mon, QObject **ret_data)
                                        QOBJECT(clist));
         assert(*ret_data != NULL);
 
-        qdict = qobject_to_qdict(*ret_data);
-
-        if (vnc_display->auth != VNC_AUTH_NONE) {
-            qdict_put(qdict, "auth",
-                      qstring_from_str(vnc_auth_name(vnc_display)));
-        }
-
-        if (vnc_qdict_local_addr(qdict, vnc_display->lsock) < 0) {
+        if (vnc_server_info_put(qobject_to_qdict(*ret_data)) < 0) {
             qobject_decref(*ret_data);
             *ret_data = NULL;
         }
-- 
1.6.6

[Qemu-devel] [PATCH 3/8] VNC: Rename client's 'username' key

[Luiz Capitulino]

It's the SASL username, so it's better to call it 'sasl_username'
to be consistent.

Note that this change wouldn't be allowed if QMP were stable.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 vnc.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/vnc.c b/vnc.c
index b5f3b64..e35cc36 100644
--- a/vnc.c
+++ b/vnc.c
@@ -228,7 +228,8 @@ static QDict *do_info_vnc_client(Monitor *mon, VncState *client)
 #ifdef CONFIG_VNC_SASL
     if (client->sasl.conn &&
         client->sasl.username) {
-        qdict_put(qdict, "username", qstring_from_str(client->sasl.username));
+        qdict_put(qdict, "sasl_username",
+                  qstring_from_str(client->sasl.username));
     }
 #endif
 
@@ -253,8 +254,8 @@ static void info_vnc_iter(QObject *obj, void *opaque)
 #endif
 #ifdef CONFIG_VNC_SASL
     monitor_printf(mon, "    username: %s\n",
-        qdict_haskey(client, "username") ?
-        qdict_get_str(client, "username") : "none");
+        qdict_haskey(client, "sasl_username") ?
+        qdict_get_str(client, "sasl_username") : "none");
 #endif
 }
 
@@ -302,7 +303,7 @@ void do_info_vnc_print(Monitor *mon, const QObject *data)
  * - "host": client's IP address
  * - "service": client's port number
  * - "x509_dname": TLS dname (optional)
- * - "username": SASL username (optional)
+ * - "sasl_username": SASL username (optional)
  *
  * Example:
  *
-- 
1.6.6

[Qemu-devel] [PATCH 4/8] VNC: Add 'family' key

[Luiz Capitulino]

It contains the socket adress family name, like "ipv4" or
"ipv6".

This is useful for clients so that they can interpret the
'host' key reliably.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 vnc.c |   26 +++++++++++++++++++++++++-
 1 files changed, 25 insertions(+), 1 deletions(-)

diff --git a/vnc.c b/vnc.c
index e35cc36..e023824 100644
--- a/vnc.c
+++ b/vnc.c
@@ -100,6 +100,26 @@ char *vnc_socket_remote_addr(const char *format, int fd) {
     return addr_to_string(format, &sa, salen);
 }
 
+static QString *get_sock_family(const struct sockaddr_storage *sa)
+{
+    const char *name;
+
+    switch (sa->ss_family)
+    {
+        case AF_INET:
+            name = "ipv4";
+            break;
+        case AF_INET6:
+            name = "ipv6";
+            break;
+        default:
+            name = "unknown";
+            break;
+    }
+
+    return qstring_from_str(name);
+}
+
 static int put_addr_qdict(QDict *qdict, struct sockaddr_storage *sa,
                           socklen_t salen)
 {
@@ -118,6 +138,7 @@ static int put_addr_qdict(QDict *qdict, struct sockaddr_storage *sa,
 
     qdict_put(qdict, "host", qstring_from_str(host));
     qdict_put(qdict, "service", qstring_from_str(serv));
+    qdict_put(qdict, "family", get_sock_family(sa));
 
     return 0;
 }
@@ -294,6 +315,7 @@ void do_info_vnc_print(Monitor *mon, const QObject *data)
  *
  * - "enabled": true or false
  * - "host": server's IP address
+ * - "family": address family ("ipv4" or "ipv6")
  * - "service": server's port number
  * - "auth": authentication method
  * - "clients": a QList of all connected clients
@@ -301,6 +323,7 @@ void do_info_vnc_print(Monitor *mon, const QObject *data)
  * Clients are described by a QDict, with the following information:
  *
  * - "host": client's IP address
+ * - "family": address family ("ipv4" or "ipv6")
  * - "service": client's port number
  * - "x509_dname": TLS dname (optional)
  * - "sasl_username": SASL username (optional)
@@ -308,7 +331,8 @@ void do_info_vnc_print(Monitor *mon, const QObject *data)
  * Example:
  *
  * { "enabled": true, "host": "0.0.0.0", "service": "50402", "auth": "vnc",
- *   "clients": [ { "host": "127.0.0.1", "service": "50401" } ] }
+ *   "family": "ipv4",
+ *   "clients": [{ "host": "127.0.0.1", "service": "50401", "family": "ipv4" }]}
  */
 void do_info_vnc(Monitor *mon, QObject **ret_data)
 {
-- 
1.6.6

[Qemu-devel] [PATCH 5/8] VNC: Cache client info at connection time

[Luiz Capitulino]

When a disconnection happens the client's socket on QEMU
side may become invalid, this way it won't be possible
to query it to get client information, which is going to
be needed by the future QMP VNC_DISCONNECTED event.

To always have this information available we query the
socket at connection time and cache the client info in
struct VncState.

Two function are introduced to perform this job.

vnc_client_cache_addr() is called right when the connection
is made, however the authentication information is not
available at that moment so vnc_client_cache_auth() is
called from protocol_client_init() to get auth info.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 vnc.c |   40 ++++++++++++++++++++++++++++++----------
 vnc.h |    2 ++
 2 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/vnc.c b/vnc.c
index e023824..d37fa60 100644
--- a/vnc.c
+++ b/vnc.c
@@ -230,16 +230,16 @@ static int vnc_server_info_put(QDict *qdict)
     return 0;
 }
 
-static QDict *do_info_vnc_client(Monitor *mon, VncState *client)
+static void vnc_client_cache_auth(VncState *client)
 {
     QDict *qdict;
 
-    qdict = qdict_new();
-    if (vnc_qdict_remote_addr(qdict, client->csock) < 0) {
-        QDECREF(qdict);
-        return NULL;
+    if (!client->info) {
+        return;
     }
 
+    qdict = qobject_to_qdict(client->info);
+
 #ifdef CONFIG_VNC_TLS
     if (client->tls.session &&
         client->tls.dname) {
@@ -253,8 +253,20 @@ static QDict *do_info_vnc_client(Monitor *mon, VncState *client)
                   qstring_from_str(client->sasl.username));
     }
 #endif
+}
 
-    return qdict;
+static void vnc_client_cache_addr(VncState *client)
+{
+    QDict *qdict;
+
+    qdict = qdict_new();
+    if (vnc_qdict_remote_addr(qdict, client->csock) < 0) {
+        QDECREF(qdict);
+        /* XXX: how to report the error? */
+        return;
+    }
+
+    client->info = QOBJECT(qdict);
 }
 
 static void info_vnc_iter(QObject *obj, void *opaque)
@@ -339,16 +351,17 @@ void do_info_vnc(Monitor *mon, QObject **ret_data)
     if (vnc_display == NULL || vnc_display->display == NULL) {
         *ret_data = qobject_from_jsonf("{ 'enabled': false }");
     } else {
-        QDict *qdict;
         QList *clist;
 
         clist = qlist_new();
         if (vnc_display->clients) {
             VncState *client = vnc_display->clients;
             while (client) {
-                qdict = do_info_vnc_client(mon, client);
-                if (qdict)
-                    qlist_append(clist, qdict);
+                if (client->info) {
+                    /* incref so that it's not freed by upper layers */
+                    qobject_incref(client->info);
+                    qlist_append_obj(clist, client->info);
+                }
                 client = client->next;
             }
         }
@@ -1079,6 +1092,9 @@ static void vnc_disconnect_finish(VncState *vs)
         qemu_free(vs->output.buffer);
         vs->output.buffer = NULL;
     }
+
+    qobject_decref(vs->info);
+
 #ifdef CONFIG_VNC_TLS
     vnc_tls_client_cleanup(vs);
 #endif /* CONFIG_VNC_TLS */
@@ -2069,6 +2085,8 @@ static int protocol_client_init(VncState *vs, uint8_t *data, size_t len)
     vnc_write(vs, buf, size);
     vnc_flush(vs);
 
+    vnc_client_cache_auth(vs);
+
     vnc_read_when(vs, protocol_client_msg, 1);
 
     return 0;
@@ -2377,6 +2395,8 @@ static void vnc_connect(VncDisplay *vd, int csock)
     socket_set_nonblock(vs->csock);
     qemu_set_fd_handler2(vs->csock, NULL, vnc_client_read, NULL, vs);
 
+    vnc_client_cache_addr(vs);
+
     vs->vd = vd;
     vs->ds = vd->ds;
     vs->last_x = -1;
diff --git a/vnc.h b/vnc.h
index fcc6824..1210824 100644
--- a/vnc.h
+++ b/vnc.h
@@ -144,6 +144,8 @@ struct VncState
     VncStateSASL sasl;
 #endif
 
+    QObject *info;
+
     Buffer output;
     Buffer input;
     /* current output mode information */
-- 
1.6.6

[Qemu-devel] [PATCH 6/8] QMP: Introduce VNC_CONNECTED event

[Luiz Capitulino]

It's emitted when a VNC client connects to QEMU, client's information
such as port and IP address are provided.

Note that this event is emitted right when the connection is
established. This means that it happens before authentication
procedure and session initialization.

Event example:

{ "event": "VNC_CONNECTED",
    "timestamp": { "seconds": 1262976601, "microseconds": 975795 },
    "data": {
        "server": { "auth": "sasl", "family": "ipv4",
                    "service": "5901", "host": "0.0.0.0" },
        "client": { "family": "ipv4", "service": "58425",
                    "host": "127.0.0.1" } } }

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 QMP/qmp-events.txt |    7 +++++++
 monitor.c          |    3 +++
 monitor.h          |    1 +
 vnc.c              |   25 +++++++++++++++++++++++++
 4 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
index 682a5e5..d36da46 100644
--- a/QMP/qmp-events.txt
+++ b/QMP/qmp-events.txt
@@ -24,3 +24,10 @@ Data: None.
 
 Description: Issued when the Virtual Machine enters debug mode.
 Data: None.
+
+4 VNC_CONNECTED
+---------------
+
+Description: Issued when a VNC client establishes a connection.
+Data: 'server' and 'client' keys with the same keys as 'query-vnc',
+except that authentication ID is not provided.
diff --git a/monitor.c b/monitor.c
index 8e00f68..4a6af42 100644
--- a/monitor.c
+++ b/monitor.c
@@ -357,6 +357,9 @@ void monitor_protocol_event(MonitorEvent event, QObject *data)
         case QEVENT_STOP:
             event_name = "STOP";
             break;
+        case QEVENT_VNC_CONNECTED:
+            event_name = "VNC_CONNECTED";
+            break;
         default:
             abort();
             break;
diff --git a/monitor.h b/monitor.h
index 6ed117a..4d57679 100644
--- a/monitor.h
+++ b/monitor.h
@@ -20,6 +20,7 @@ typedef enum MonitorEvent {
     QEVENT_RESET,
     QEVENT_POWERDOWN,
     QEVENT_STOP,
+    QEVENT_VNC_CONNECTED,
     QEVENT_MAX,
 } MonitorEvent;
 
diff --git a/vnc.c b/vnc.c
index d37fa60..6d488e5 100644
--- a/vnc.c
+++ b/vnc.c
@@ -269,6 +269,30 @@ static void vnc_client_cache_addr(VncState *client)
     client->info = QOBJECT(qdict);
 }
 
+static void vnc_qmp_event(VncState *vs, MonitorEvent event)
+{
+    QDict *server;
+    QObject *data;
+
+    if (!vs->info) {
+        return;
+    }
+
+    server = qdict_new();
+    if (vnc_server_info_put(server) < 0) {
+        QDECREF(server);
+        return;
+    }
+
+    data = qobject_from_jsonf("{ 'client': %p, 'server': %p }",
+                              vs->info, QOBJECT(server));
+
+    monitor_protocol_event(event, data);
+
+    qobject_incref(vs->info);
+    qobject_decref(data);
+}
+
 static void info_vnc_iter(QObject *obj, void *opaque)
 {
     QDict *client;
@@ -2396,6 +2420,7 @@ static void vnc_connect(VncDisplay *vd, int csock)
     qemu_set_fd_handler2(vs->csock, NULL, vnc_client_read, NULL, vs);
 
     vnc_client_cache_addr(vs);
+    vnc_qmp_event(vs, QEVENT_VNC_CONNECTED);
 
     vs->vd = vd;
     vs->ds = vd->ds;
-- 
1.6.6

[Qemu-devel] [PATCH 7/8] QMP: Introduce VNC_DISCONNECTED event

[Luiz Capitulino]

It's emitted when a VNC client disconnects from QEMU, client's
information such as port and IP address are provided.

Event example:

{ "event": "VNC_DISCONNECTED",
    "timestamp": { "seconds": 1262976601, "microseconds": 975795 },
    "data": {
        "server": { "auth": "sasl", "family": "ipv4",
                    "service": "5901", "host": "0.0.0.0" },
        "client": { "family": "ipv4", "service": "58425",
                    "host": "127.0.0.1", "sasl_username": "foo" } } }

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 QMP/qmp-events.txt |    6 ++++++
 monitor.c          |    3 +++
 monitor.h          |    1 +
 vnc.c              |    2 ++
 4 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
index d36da46..1e87eb1 100644
--- a/QMP/qmp-events.txt
+++ b/QMP/qmp-events.txt
@@ -31,3 +31,9 @@ Data: None.
 Description: Issued when a VNC client establishes a connection.
 Data: 'server' and 'client' keys with the same keys as 'query-vnc',
 except that authentication ID is not provided.
+
+5 VNC_DISCONNECTED
+------------------
+
+Description: Issued when the conection is closed.
+Data: 'server' and 'client' keys with the same keys as 'query-vnc'.
diff --git a/monitor.c b/monitor.c
index 4a6af42..5ac1c5c 100644
--- a/monitor.c
+++ b/monitor.c
@@ -360,6 +360,9 @@ void monitor_protocol_event(MonitorEvent event, QObject *data)
         case QEVENT_VNC_CONNECTED:
             event_name = "VNC_CONNECTED";
             break;
+        case QEVENT_VNC_DISCONNECTED:
+            event_name = "VNC_DISCONNECTED";
+            break;
         default:
             abort();
             break;
diff --git a/monitor.h b/monitor.h
index 4d57679..42386de 100644
--- a/monitor.h
+++ b/monitor.h
@@ -21,6 +21,7 @@ typedef enum MonitorEvent {
     QEVENT_POWERDOWN,
     QEVENT_STOP,
     QEVENT_VNC_CONNECTED,
+    QEVENT_VNC_DISCONNECTED,
     QEVENT_MAX,
 } MonitorEvent;
 
diff --git a/vnc.c b/vnc.c
index 6d488e5..a590bb9 100644
--- a/vnc.c
+++ b/vnc.c
@@ -1108,6 +1108,8 @@ static void vnc_disconnect_start(VncState *vs)
 
 static void vnc_disconnect_finish(VncState *vs)
 {
+    vnc_qmp_event(vs, QEVENT_VNC_DISCONNECTED);
+
     if (vs->input.buffer) {
         qemu_free(vs->input.buffer);
         vs->input.buffer = NULL;
-- 
1.6.6

[Qemu-devel] [PATCH 8/8] QMP: Introduce VNC_INITIALIZED event

[Luiz Capitulino]

It's emitted when a VNC client session is activated by QEMU,
client's information such as port, IP and auth ID (if the
session is authenticated) are provided.

Event example:

{ "event": "VNC_INITIALIZED",
    "timestamp": {"seconds": 1263475302, "microseconds": 150772},
    "data": {
        "server": { "auth": "sasl", "family": "ipv4",
                    "service": "5901", "host": "0.0.0.0"},
        "client": { "family": "ipv4", "service": "46089",
                    "host": "127.0.0.1", "sasl_username": "lcapitulino" } } }

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 QMP/qmp-events.txt |    6 ++++++
 monitor.c          |    3 +++
 monitor.h          |    1 +
 vnc.c              |    1 +
 4 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
index 1e87eb1..dc48ccc 100644
--- a/QMP/qmp-events.txt
+++ b/QMP/qmp-events.txt
@@ -37,3 +37,9 @@ except that authentication ID is not provided.
 
 Description: Issued when the conection is closed.
 Data: 'server' and 'client' keys with the same keys as 'query-vnc'.
+
+6 VNC_INITIALIZED
+-----------------
+
+Description: Issued when the VNC session is made active.
+Data: 'server' and 'client' keys with the same keys as 'query-vnc'.
diff --git a/monitor.c b/monitor.c
index 5ac1c5c..f2abd3c 100644
--- a/monitor.c
+++ b/monitor.c
@@ -360,6 +360,9 @@ void monitor_protocol_event(MonitorEvent event, QObject *data)
         case QEVENT_VNC_CONNECTED:
             event_name = "VNC_CONNECTED";
             break;
+        case QEVENT_VNC_INITIALIZED:
+            event_name = "VNC_INITIALIZED";
+            break;
         case QEVENT_VNC_DISCONNECTED:
             event_name = "VNC_DISCONNECTED";
             break;
diff --git a/monitor.h b/monitor.h
index 42386de..2da30e8 100644
--- a/monitor.h
+++ b/monitor.h
@@ -21,6 +21,7 @@ typedef enum MonitorEvent {
     QEVENT_POWERDOWN,
     QEVENT_STOP,
     QEVENT_VNC_CONNECTED,
+    QEVENT_VNC_INITIALIZED,
     QEVENT_VNC_DISCONNECTED,
     QEVENT_MAX,
 } MonitorEvent;
diff --git a/vnc.c b/vnc.c
index a590bb9..c7d6652 100644
--- a/vnc.c
+++ b/vnc.c
@@ -2112,6 +2112,7 @@ static int protocol_client_init(VncState *vs, uint8_t *data, size_t len)
     vnc_flush(vs);
 
     vnc_client_cache_auth(vs);
+    vnc_qmp_event(vs, QEVENT_VNC_INITIALIZED);
 
     vnc_read_when(vs, protocol_client_msg, 1);
 
-- 
1.6.6

[Qemu-devel] Re: [PATCH v0 0/8]: VNC events and cleanup

[Daniel P. Berrange]

On Thu, Jan 14, 2010 at 02:50:51PM -0200, Luiz Capitulino wrote:
>  Hi there,
> 
>  This series contains two VNC related changes. First a small cleanup
> is done in the current 'query-vnc' command _response_, then the following
> QMP events are introduced:
> 
> - VNC_CONNECTED: emitted when a VNC client establishes a connection
> - VNC_INITIALIZED: emitted when the VNC session is made active
> - VNC_DISCONNECTED: emitted when the conection is closed
> 
>  The only issue is the current events documentation. I'm using the
> current format which is quite bad, things will improve when we have
> proper documentation support (being worked out by Markus).

This series looks reasonable to me wrt VNC.

Should we try to think about what we'll do with other network services ?
eg will we add SPICE_CONNECTED / SPICE_DISCONNECTED in the future ?
Similar question for chardevs using networking ?


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

[Qemu-devel] Re: [PATCH v0 0/8]: VNC events and cleanup

[Luiz Capitulino]

On Thu, 14 Jan 2010 17:32:14 +0000
"Daniel P. Berrange" <berrange@redhat.com> wrote:

> On Thu, Jan 14, 2010 at 02:50:51PM -0200, Luiz Capitulino wrote:
> >  Hi there,
> > 
> >  This series contains two VNC related changes. First a small cleanup
> > is done in the current 'query-vnc' command _response_, then the following
> > QMP events are introduced:
> > 
> > - VNC_CONNECTED: emitted when a VNC client establishes a connection
> > - VNC_INITIALIZED: emitted when the VNC session is made active
> > - VNC_DISCONNECTED: emitted when the conection is closed
> > 
> >  The only issue is the current events documentation. I'm using the
> > current format which is quite bad, things will improve when we have
> > proper documentation support (being worked out by Markus).
> 
> This series looks reasonable to me wrt VNC.
> 
> Should we try to think about what we'll do with other network services ?
> eg will we add SPICE_CONNECTED / SPICE_DISCONNECTED in the future ?
> Similar question for chardevs using networking ?

 That's a good question, you think they should converge some way?

 If the chardev API can tell that a fd belongs to a certain subsystem,
then maybe we could move the machinery of connected/disconnected events
down there.

 But that's only an internal refactoring, at first I think we'll have
to add different events for each subsystem we're interested in.

Re: [Qemu-devel] [PATCH 4/8] VNC: Add 'family' key

[Gerd Hoffmann]

> +static QString *get_sock_family(const struct sockaddr_storage *sa)
> +{
> +    const char *name;
> +
> +    switch (sa->ss_family)
> +    {
> +        case AF_INET:
> +            name = "ipv4";
> +            break;
> +        case AF_INET6:
> +            name = "ipv6";
> +            break;
> +        default:
> +            name = "unknown";
> +            break;
> +    }
> +
> +    return qstring_from_str(name);
> +}

qemu-socket has inet_strfamily() already.  You might want to un-static 
that one, then simply do

   return qstring_from_str(inet_strfamily(sa->ss_family));

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 4/8] VNC: Add 'family' key

[Luiz Capitulino]

On Fri, 15 Jan 2010 08:54:56 +0100
Gerd Hoffmann <kraxel@redhat.com> wrote:

> > +static QString *get_sock_family(const struct sockaddr_storage *sa)
> > +{
> > +    const char *name;
> > +
> > +    switch (sa->ss_family)
> > +    {
> > +        case AF_INET:
> > +            name = "ipv4";
> > +            break;
> > +        case AF_INET6:
> > +            name = "ipv6";
> > +            break;
> > +        default:
> > +            name = "unknown";
> > +            break;
> > +    }
> > +
> > +    return qstring_from_str(name);
> > +}
> 
> qemu-socket has inet_strfamily() already.  You might want to un-static 
> that one, then simply do
> 
>    return qstring_from_str(inet_strfamily(sa->ss_family));

 Will do and resend.

 Thanks Gerd.

Re: [Qemu-devel] [PATCH 1/8] VNC: Use 'enabled' key instead of 'status'

[Anthony Liguori]

On 01/14/2010 10:50 AM, Luiz Capitulino wrote:
> Currently the 'status' key is a string whose value can be
> "disabled" or "enabled", change it to the QMP's standard
> 'enabled' key, which is a bool.
>
> Note that 'status' in being dropped and this wouldn't be
> allowed if QMP were stable.
>
> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
>    

Applied all.  Thanks.

Regards,

Anthony Liguori
> ---
>   vnc.c |   10 +++++-----
>   1 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/vnc.c b/vnc.c
> index 58eac73..ef86ef7 100644
> --- a/vnc.c
> +++ b/vnc.c
> @@ -254,7 +254,7 @@ void do_info_vnc_print(Monitor *mon, const QObject *data)
>       QList *clients;
>
>       server = qobject_to_qdict(data);
> -    if (strcmp(qdict_get_str(server, "status"), "disabled") == 0) {
> +    if (qdict_get_bool(server, "enabled") == 0) {
>           monitor_printf(mon, "Server: disabled\n");
>           return;
>       }
> @@ -282,7 +282,7 @@ void do_info_vnc_print(Monitor *mon, const QObject *data)
>    *
>    * The main QDict contains the following:
>    *
> - * - "status": "disabled" or "enabled"
> + * - "enabled": true or false
>    * - "host": server's IP address
>    * - "service": server's port number
>    * - "auth": authentication method (optional)
> @@ -297,13 +297,13 @@ void do_info_vnc_print(Monitor *mon, const QObject *data)
>    *
>    * Example:
>    *
> - * { "status": "enabled", "host": "0.0.0.0", "service": "50402", "auth": "vnc",
> + * { "enabled": true, "host": "0.0.0.0", "service": "50402", "auth": "vnc",
>    *   "clients": [ { "host": "127.0.0.1", "service": "50401" } ] }
>    */
>   void do_info_vnc(Monitor *mon, QObject **ret_data)
>   {
>       if (vnc_display == NULL || vnc_display->display == NULL) {
> -        *ret_data = qobject_from_jsonf("{ 'status': 'disabled' }");
> +        *ret_data = qobject_from_jsonf("{ 'enabled': false }");
>       } else {
>           QDict *qdict;
>           QList *clist;
> @@ -319,7 +319,7 @@ void do_info_vnc(Monitor *mon, QObject **ret_data)
>               }
>           }
>
> -        *ret_data = qobject_from_jsonf("{ 'status': 'enabled', 'clients': %p }",
> +        *ret_data = qobject_from_jsonf("{ 'enabled': true, 'clients': %p }",
>                                          QOBJECT(clist));
>           assert(*ret_data != NULL);
>
>