Patchwork [19/19] VNC: Convert do_info_vnc() to QObject

login
register
mail settings
Submitter Luiz Capitulino
Date Dec. 9, 2009, 4:27 p.m.
Message ID <1260376078-8694-20-git-send-email-lcapitulino@redhat.com>
Download mbox | patch
Permalink /patch/40754/
State New
Headers show

Comments

Luiz Capitulino - Dec. 9, 2009, 4:27 p.m.
Return a QDict with server information. Connected clients are returned
as a QList of QDicts.

The new functions (vnc_qdict_remote_addr(), vnc_qdict_local_addr() and
put_addr_qdict()) are used to insert 'host' and 'service' information
in the returned QDict.

This patch is big, but I don't see how to split it.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 console.h |    3 +-
 monitor.c |    3 +-
 vnc.c     |  191 +++++++++++++++++++++++++++++++++++++++++++++++++++----------
 3 files changed, 164 insertions(+), 33 deletions(-)
Markus Armbruster - Dec. 10, 2009, 10:34 a.m.
Luiz Capitulino <lcapitulino@redhat.com> writes:

> Return a QDict with server information. Connected clients are returned
> as a QList of QDicts.
>
> The new functions (vnc_qdict_remote_addr(), vnc_qdict_local_addr() and
> put_addr_qdict()) are used to insert 'host' and 'service' information
> in the returned QDict.
>
> This patch is big, but I don't see how to split it.
>
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  console.h |    3 +-
>  monitor.c |    3 +-
>  vnc.c     |  191 +++++++++++++++++++++++++++++++++++++++++++++++++++----------
>  3 files changed, 164 insertions(+), 33 deletions(-)
>
[...]
> diff --git a/vnc.c b/vnc.c
> index 32c4678..f0fea6a 100644
> --- a/vnc.c
> +++ b/vnc.c
[...]
> +/**
> + * do_info_vnc(): Show VNC server information
> + *
> + * Return a QDict with server information. Connected clients are returned
> + * as a QList of QDicts.
> + *
> + * The main QDict contains the following:
> + *
> + * - "status": "disabled" or "enabled"
> + * - "host": server's IP address
> + * - "service": server's port number
> + * - "auth": authentication method (optional)
> + * - "clients": a QList of all connected clients
> + *
> + * Clients are described by a QDict, with the following information:
> + *
> + * - "host": client's IP address
> + * - "service": client's port number
> + * - "x509 dname": TLS dname (optional)

Sure you want dict keys with spaces?  I'd prefer "x509-dname".

> + * - "username": SASL username (optional)
> + *
> + * Example:
> + *
> + * { "status": "enabled", "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)
[...]
Luiz Capitulino - Dec. 10, 2009, 11:56 a.m.
On Thu, 10 Dec 2009 11:34:37 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> > +/**
> > + * do_info_vnc(): Show VNC server information
> > + *
> > + * Return a QDict with server information. Connected clients are returned
> > + * as a QList of QDicts.
> > + *
> > + * The main QDict contains the following:
> > + *
> > + * - "status": "disabled" or "enabled"
> > + * - "host": server's IP address
> > + * - "service": server's port number
> > + * - "auth": authentication method (optional)
> > + * - "clients": a QList of all connected clients
> > + *
> > + * Clients are described by a QDict, with the following information:
> > + *
> > + * - "host": client's IP address
> > + * - "service": client's port number
> > + * - "x509 dname": TLS dname (optional)
> 
> Sure you want dict keys with spaces?  I'd prefer "x509-dname".

 I don't think it's a big deal because this string will never
change, but would be good to come with a standard style for
dict keys (at least for the protocol).

 I'm ok with either way.
Daniel P. Berrange - Dec. 10, 2009, 12:12 p.m.
On Thu, Dec 10, 2009 at 09:56:06AM -0200, Luiz Capitulino wrote:
> On Thu, 10 Dec 2009 11:34:37 +0100
> Markus Armbruster <armbru@redhat.com> wrote:
> 
> > > +/**
> > > + * do_info_vnc(): Show VNC server information
> > > + *
> > > + * Return a QDict with server information. Connected clients are returned
> > > + * as a QList of QDicts.
> > > + *
> > > + * The main QDict contains the following:
> > > + *
> > > + * - "status": "disabled" or "enabled"
> > > + * - "host": server's IP address
> > > + * - "service": server's port number
> > > + * - "auth": authentication method (optional)
> > > + * - "clients": a QList of all connected clients
> > > + *
> > > + * Clients are described by a QDict, with the following information:
> > > + *
> > > + * - "host": client's IP address
> > > + * - "service": client's port number
> > > + * - "x509 dname": TLS dname (optional)
> > 
> > Sure you want dict keys with spaces?  I'd prefer "x509-dname".
> 
>  I don't think it's a big deal because this string will never
> change, but would be good to come with a standard style for
> dict keys (at least for the protocol).

I agree with Markus, that it is nicer to avoid spaces in the dict keys,
even if technically it is allowed. I'd vote for x509-dname too.

Daniel
Anthony Liguori - Dec. 10, 2009, 1 p.m.
Markus Armbruster wrote:
> Luiz Capitulino <lcapitulino@redhat.com> writes:
>
>   
>> Return a QDict with server information. Connected clients are returned
>> as a QList of QDicts.
>>
>> The new functions (vnc_qdict_remote_addr(), vnc_qdict_local_addr() and
>> put_addr_qdict()) are used to insert 'host' and 'service' information
>> in the returned QDict.
>>
>> This patch is big, but I don't see how to split it.
>>
>> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
>> ---
>>  console.h |    3 +-
>>  monitor.c |    3 +-
>>  vnc.c     |  191 +++++++++++++++++++++++++++++++++++++++++++++++++++----------
>>  3 files changed, 164 insertions(+), 33 deletions(-)
>>
>>     
> [...]
>   
>> diff --git a/vnc.c b/vnc.c
>> index 32c4678..f0fea6a 100644
>> --- a/vnc.c
>> +++ b/vnc.c
>>     
> [...]
>   
>> +/**
>> + * do_info_vnc(): Show VNC server information
>> + *
>> + * Return a QDict with server information. Connected clients are returned
>> + * as a QList of QDicts.
>> + *
>> + * The main QDict contains the following:
>> + *
>> + * - "status": "disabled" or "enabled"
>> + * - "host": server's IP address
>> + * - "service": server's port number
>> + * - "auth": authentication method (optional)
>> + * - "clients": a QList of all connected clients
>> + *
>> + * Clients are described by a QDict, with the following information:
>> + *
>> + * - "host": client's IP address
>> + * - "service": client's port number
>> + * - "x509 dname": TLS dname (optional)
>>     
>
> Sure you want dict keys with spaces?  I'd prefer "x509-dname".
>   

+1

Actually, x509_dname is preferable.  While the JSON spec doesn't say 
this, in JavaScript, dictionaries are indistinguishable from objects.  
It's better for the key names to be valid identifiers.

Patch

diff --git a/console.h b/console.h
index c7172f6..dfc8ae4 100644
--- a/console.h
+++ b/console.h
@@ -323,7 +323,8 @@  void vnc_display_init(DisplayState *ds);
 void vnc_display_close(DisplayState *ds);
 int vnc_display_open(DisplayState *ds, const char *display);
 int vnc_display_password(DisplayState *ds, const char *password);
-void do_info_vnc(Monitor *mon);
+void do_info_vnc_print(Monitor *mon, const QObject *data);
+void do_info_vnc(Monitor *mon, QObject **ret_data);
 char *vnc_display_local_addr(DisplayState *ds);
 
 /* curses.c */
diff --git a/monitor.c b/monitor.c
index b2d0384..ddf9eac 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2503,7 +2503,8 @@  static const mon_cmd_t info_cmds[] = {
         .args_type  = "",
         .params     = "",
         .help       = "show the vnc server status",
-        .mhandler.info = do_info_vnc,
+        .user_print = do_info_vnc_print,
+        .mhandler.info_new = do_info_vnc,
     },
     {
         .name       = "name",
diff --git a/vnc.c b/vnc.c
index 32c4678..f0fea6a 100644
--- a/vnc.c
+++ b/vnc.c
@@ -29,6 +29,7 @@ 
 #include "qemu_socket.h"
 #include "qemu-timer.h"
 #include "acl.h"
+#include "qemu-objects.h"
 
 #define VNC_REFRESH_INTERVAL_BASE 30
 #define VNC_REFRESH_INTERVAL_INC  50
@@ -99,6 +100,52 @@  char *vnc_socket_remote_addr(const char *format, int fd) {
     return addr_to_string(format, &sa, salen);
 }
 
+static int put_addr_qdict(QDict *qdict, struct sockaddr_storage *sa,
+                          socklen_t salen)
+{
+    char host[NI_MAXHOST];
+    char serv[NI_MAXSERV];
+    int err;
+
+    if ((err = getnameinfo((struct sockaddr *)sa, salen,
+                           host, sizeof(host),
+                           serv, sizeof(serv),
+                           NI_NUMERICHOST | NI_NUMERICSERV)) != 0) {
+        VNC_DEBUG("Cannot resolve address %d: %s\n",
+                  err, gai_strerror(err));
+        return -1;
+    }
+
+    qdict_put(qdict, "host", qstring_from_str(host));
+    qdict_put(qdict, "service", qstring_from_str(serv));
+
+    return 0;
+}
+
+static int vnc_qdict_local_addr(QDict *qdict, int fd)
+{
+    struct sockaddr_storage sa;
+    socklen_t salen;
+
+    salen = sizeof(sa);
+    if (getsockname(fd, (struct sockaddr*)&sa, &salen) < 0)
+        return -1;
+
+    return put_addr_qdict(qdict, &sa, salen);
+}
+
+static int vnc_qdict_remote_addr(QDict *qdict, int fd)
+{
+    struct sockaddr_storage sa;
+    socklen_t salen;
+
+    salen = sizeof(sa);
+    if (getpeername(fd, (struct sockaddr*)&sa, &salen) < 0)
+        return -1;
+
+    return put_addr_qdict(qdict, &sa, salen);
+}
+
 static const char *vnc_auth_name(VncDisplay *vd) {
     switch (vd->auth) {
     case VNC_AUTH_INVALID:
@@ -150,58 +197,140 @@  static const char *vnc_auth_name(VncDisplay *vd) {
     return "unknown";
 }
 
-static void do_info_vnc_client(Monitor *mon, VncState *client)
+static QDict *do_info_vnc_client(Monitor *mon, VncState *client)
 {
-    char *clientAddr =
-        vnc_socket_remote_addr("     address: %s:%s\n",
-                               client->csock);
-    if (!clientAddr)
-        return;
+    QDict *qdict;
 
-    monitor_printf(mon, "Client:\n");
-    monitor_printf(mon, "%s", clientAddr);
-    free(clientAddr);
+    qdict = qdict_new();
+    if (vnc_qdict_remote_addr(qdict, client->csock) < 0) {
+        QDECREF(qdict);
+        return NULL;
+    }
 
 #ifdef CONFIG_VNC_TLS
     if (client->tls.session &&
-        client->tls.dname)
-        monitor_printf(mon, "  x509 dname: %s\n", client->tls.dname);
-    else
-        monitor_printf(mon, "  x509 dname: none\n");
+        client->tls.dname) {
+        qdict_put(qdict, "x509 dname", qstring_from_str(client->tls.dname));
+    }
 #endif
 #ifdef CONFIG_VNC_SASL
     if (client->sasl.conn &&
-        client->sasl.username)
-        monitor_printf(mon, "    username: %s\n", client->sasl.username);
-    else
-        monitor_printf(mon, "    username: none\n");
+        client->sasl.username) {
+        qdict_put(qdict, "username", qstring_from_str(client->sasl.username));
+    }
 #endif
+
+    return qdict;
 }
 
-void do_info_vnc(Monitor *mon)
+static void info_vnc_iter(QObject *obj, void *opaque)
 {
-    if (vnc_display == NULL || vnc_display->display == NULL) {
+    QDict *client;
+    Monitor *mon = opaque;
+
+    client = qobject_to_qdict(obj);
+    monitor_printf(mon, "Client:\n");
+    monitor_printf(mon, "     address: %s:%s\n",
+                   qdict_get_str(client, "host"),
+                   qdict_get_str(client, "service"));
+
+#ifdef CONFIG_VNC_TLS
+    monitor_printf(mon, "  x509 dname: %s\n",
+        qdict_haskey(client, "x509 dname") ?
+        qdict_get_str(client, "x509 dname") : "none");
+#endif
+#ifdef CONFIG_VNC_SASL
+    monitor_printf(mon, "    username: %s\n",
+        qdict_haskey(client, "username") ?
+        qdict_get_str(client, "username") : "none");
+#endif
+}
+
+void do_info_vnc_print(Monitor *mon, const QObject *data)
+{
+    QDict *server;
+    QList *clients;
+
+    server = qobject_to_qdict(data);
+    if (strcmp(qdict_get_str(server, "status"), "disabled") == 0) {
         monitor_printf(mon, "Server: disabled\n");
-    } else {
-        char *serverAddr = vnc_socket_local_addr("     address: %s:%s\n",
-                                                 vnc_display->lsock);
+        return;
+    }
 
-        if (!serverAddr)
-            return;
+    monitor_printf(mon, "Server:\n");
+    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");
+
+    clients = qdict_get_qlist(server, "clients");
+    if (qlist_empty(clients)) {
+        monitor_printf(mon, "Client: none\n");
+    } else {
+        qlist_iter(clients, info_vnc_iter, mon);
+    }
+}
 
-        monitor_printf(mon, "Server:\n");
-        monitor_printf(mon, "%s", serverAddr);
-        free(serverAddr);
-        monitor_printf(mon, "        auth: %s\n", vnc_auth_name(vnc_display));
+/**
+ * do_info_vnc(): Show VNC server information
+ *
+ * Return a QDict with server information. Connected clients are returned
+ * as a QList of QDicts.
+ *
+ * The main QDict contains the following:
+ *
+ * - "status": "disabled" or "enabled"
+ * - "host": server's IP address
+ * - "service": server's port number
+ * - "auth": authentication method (optional)
+ * - "clients": a QList of all connected clients
+ *
+ * Clients are described by a QDict, with the following information:
+ *
+ * - "host": client's IP address
+ * - "service": client's port number
+ * - "x509 dname": TLS dname (optional)
+ * - "username": SASL username (optional)
+ *
+ * Example:
+ *
+ * { "status": "enabled", "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' }");
+    } else {
+        QDict *qdict;
+        QList *clist;
 
+        clist = qlist_new();
         if (vnc_display->clients) {
             VncState *client = vnc_display->clients;
             while (client) {
-                do_info_vnc_client(mon, client);
+                qdict = do_info_vnc_client(mon, client);
+                if (qdict)
+                    qlist_append(clist, qdict);
                 client = client->next;
             }
-        } else {
-            monitor_printf(mon, "Client: none\n");
+        }
+
+        *ret_data = qobject_from_jsonf("{ 'status': 'enabled', 'clients': %p }",
+                                       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) {
+            qobject_decref(*ret_data);
+            *ret_data = NULL;
         }
     }
 }