diff mbox

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

Message ID 1260376078-8694-20-git-send-email-lcapitulino@redhat.com
State New
Headers show

Commit Message

Luiz Capitulino Dec. 9, 2009, 4:27 p.m. UTC
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(-)

Comments

Markus Armbruster Dec. 10, 2009, 10:34 a.m. UTC | #1
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. UTC | #2
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. Berrangé Dec. 10, 2009, 12:12 p.m. UTC | #3
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. UTC | #4
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.
diff mbox

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;
         }
     }
 }