[Qemu-devel] [PATCH 13/19] qapi: Convert query-vnc

[Luiz Capitulino]

There are three important remarks in relation to the non-qapi command:

 1. This commit also fixes the behavior of the 'query-vnc' and 'info vnc'
    commands to return an error when qemu is built without VNC support
    (ie. --disable-vnc). The non-qapi command would return the OK
    response in QMP and no response in HMP

 2. The qapi version explicitly marks the fields 'host', 'family',
    'service' and 'auth' as optional. Their are not documented as optional
    in the non-qapi command doc, but they would not be returned if
    vnc support is disabled. The qapi version maintains the same
    semantics, but documents those fields correctly

 3. The 'clients' field, which is a list, is marked as optional but is
    always returned. If there are no clients connected an empty list
    is returned. This is not the Right Way to this in the qapi but it's
    how the non-qapi command used to work

Signed-off-by: Anthony Liguori <address@hidden>
Signed-off-by: Luiz Capitulino <address@hidden>
---
 console.h        |    9 ----
 hmp.c            |   46 ++++++++++++++++++
 hmp.h            |    1 +
 monitor.c        |   11 +----
 qapi-schema.json |   81 ++++++++++++++++++++++++++++++++
 qmp-commands.hx  |    6 ++
 qmp.c            |   10 ++++
 ui/vnc.c         |  135 +++++++++++++++++++++++++++++++++---------------------
 8 files changed, 228 insertions(+), 71 deletions(-)

diff --git a/console.h b/console.h
index 9c1487e..6ac4ed3 100644
--- a/console.h
+++ b/console.h
@@ -383,8 +383,6 @@ char *vnc_display_local_addr(DisplayState *ds);
 #ifdef CONFIG_VNC
 int vnc_display_password(DisplayState *ds, const char *password);
 int vnc_display_pw_expire(DisplayState *ds, time_t expires);
-void do_info_vnc_print(Monitor *mon, const QObject *data);
-void do_info_vnc(Monitor *mon, QObject **ret_data);
 #else
 static inline int vnc_display_password(DisplayState *ds, const char *password)
 {
@@ -396,13 +394,6 @@ static inline int vnc_display_pw_expire(DisplayState *ds, 
time_t expires)
     qerror_report(QERR_FEATURE_DISABLED, "vnc");
     return -ENODEV;
 };
-static inline void do_info_vnc(Monitor *mon, QObject **ret_data)
-{
-};
-static inline void do_info_vnc_print(Monitor *mon, const QObject *data)
-{
-    monitor_printf(mon, "VNC support disabled\n");
-};
 #endif
 
 /* curses.c */
diff --git a/hmp.c b/hmp.c
index eab2ddf..6d86fe3 100644
--- a/hmp.c
+++ b/hmp.c
@@ -260,6 +260,52 @@ void hmp_info_blockstats(Monitor *mon)
     qapi_free_BlockStatsList(stats_list);
 }
 
+void hmp_info_vnc(Monitor *mon)
+{
+    VncInfo *info;
+    Error *err = NULL;
+    VncClientInfoList *client;
+
+    info = qmp_query_vnc(&err);
+    if (err) {
+        monitor_printf(mon, "%s\n", error_get_pretty(err));
+        error_free(err);
+        return;
+    }
+
+    if (!info->enabled) {
+        monitor_printf(mon, "Server: disabled\n");
+        goto out;
+    }
+
+    monitor_printf(mon, "Server:\n");
+    if (info->has_host && info->has_service) {
+        monitor_printf(mon, "     address: %s:%s\n", info->host, 
info->service);
+    }
+    if (info->has_auth) {
+        monitor_printf(mon, "        auth: %s\n", info->auth);
+    }
+
+    if (!info->has_clients || info->clients == NULL) {
+        monitor_printf(mon, "Client: none\n");
+    } else {
+        for (client = info->clients; client; client = client->next) {
+            monitor_printf(mon, "Client:\n");
+            monitor_printf(mon, "     address: %s:%s\n",
+                           client->value->host, client->value->service);
+            monitor_printf(mon, "  x509_dname: %s\n",
+                           client->value->x509_dname ?
+                           client->value->x509_dname : "none");
+            monitor_printf(mon, "    username: %s\n",
+                           client->value->has_sasl_username ?
+                           client->value->sasl_username : "none");
+        }
+    }
+
+out:
+    qapi_free_VncInfo(info);
+}
+
 void hmp_quit(Monitor *mon, const QDict *qdict)
 {
     monitor_suspend(mon);
diff --git a/hmp.h b/hmp.h
index f8c50d4..7713348 100644
--- a/hmp.h
+++ b/hmp.h
@@ -28,6 +28,7 @@ void hmp_info_migrate(Monitor *mon);
 void hmp_info_cpus(Monitor *mon);
 void hmp_info_block(Monitor *mon);
 void hmp_info_blockstats(Monitor *mon);
+void hmp_info_vnc(Monitor *mon);
 void hmp_quit(Monitor *mon, const QDict *qdict);
 void hmp_stop(Monitor *mon, const QDict *qdict);
 void hmp_system_reset(Monitor *mon, const QDict *qdict);
diff --git a/monitor.c b/monitor.c
index 70e5460..d7c72bb 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2849,8 +2849,7 @@ static const mon_cmd_t info_cmds[] = {
         .args_type  = "",
         .params     = "",
         .help       = "show the vnc server status",
-        .user_print = do_info_vnc_print,
-        .mhandler.info_new = do_info_vnc,
+        .mhandler.info = hmp_info_vnc,
     },
 #if defined(CONFIG_SPICE)
     {
@@ -2966,14 +2965,6 @@ static const mon_cmd_t qmp_query_cmds[] = {
         .user_print = do_pci_info_print,
         .mhandler.info_new = do_pci_info,
     },
-    {
-        .name       = "vnc",
-        .args_type  = "",
-        .params     = "",
-        .help       = "show the vnc server status",
-        .user_print = do_info_vnc_print,
-        .mhandler.info_new = do_info_vnc,
-    },
 #if defined(CONFIG_SPICE)
     {
         .name       = "spice",
diff --git a/qapi-schema.json b/qapi-schema.json
index 45494d8..23be143 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -504,6 +504,87 @@
 { 'command': 'query-blockstats', 'returns': ['BlockStats'] }
 
 ##
+# @VncClientInfo:
+#
+# Information about a connected VNC client.
+#
+# @host: The host name of the client.  QEMU tries to resolve this to a DNS name
+#        when possible.
+#
+# @family: 'ipv6' if the client is connected via IPv6 and TCP
+#          'ipv4' if the client is connected via IPv4 and TCP
+#          'unix' if the client is connected via a unix domain socket
+#          'unknown' otherwise
+#
+# @service: The service name of the client's port.  This may depends on the
+#           host system's service database so symbolic names should not be
+#           relied on.
+#
+# @x509_dname: #optional If x509 authentication is in use, the Distinguished
+#              Name of the client.
+#
+# @sasl_username: #optional If SASL authentication is in use, the SASL username
+#                 used for authentication.
+#
+# Since: 0.14.0
+##
+{ 'type': 'VncClientInfo',
+  'data': {'host': 'str', 'family': 'str', 'service': 'str',
+           '*x509_dname': 'str', '*sasl_username': 'str'} }
+
+##
+# @VncInfo:
+#
+# Information about the VNC session.
+#
+# @enabled: true if the VNC server is enabled, false otherwise
+#
+# @host: #optional The hostname the VNC server is bound to.  This depends on
+#        the name resolution on the host and may be an IP address.
+#
+# @family: #optional 'ipv6' if the host is listening for IPv6 connections
+#                    'ipv4' if the host is listening for IPv4 connections
+#                    'unix' if the host is listening on a unix domain socket
+#                    'unknown' otherwise
+#
+# @service: #optional The service name of the server's port.  This may depends
+#           on the host system's service database so symbolic names should not
+#           be relied on.
+#
+# @auth: #optional the current authentication type used by the server
+#        'none' if no authentication is being used
+#        'vnc' if VNC authentication is being used
+#        'vencrypt+plain' if VEncrypt is used with plain text authentication
+#        'vencrypt+tls+none' if VEncrypt is used with TLS and no authentication
+#        'vencrypt+tls+vnc' if VEncrypt is used with TLS and VNC authentication
+#        'vencrypt+tls+plain' if VEncrypt is used with TLS and plain text auth
+#        'vencrypt+x509+none' if VEncrypt is used with x509 and no auth
+#        'vencrypt+x509+vnc' if VEncrypt is used with x509 and VNC auth
+#        'vencrypt+x509+plain' if VEncrypt is used with x509 and plain text 
auth
+#        'vencrypt+tls+sasl' if VEncrypt is used with TLS and SASL auth
+#        'vencrypt+x509+sasl' if VEncrypt is used with x509 and SASL auth
+#
+# @clients: a list of @VncClientInfo of all currently connected clients
+#
+# Since: 0.14.0
+##
+{ 'type': 'VncInfo',
+  'data': {'enabled': 'bool', '*host': 'str', '*family': 'str',
+           '*service': 'str', '*auth': 'str', '*clients': ['VncClientInfo']} }
+
+##
+# @query-vnc:
+#
+# Returns information about the current VNC server
+#
+# Returns: @VncInfo
+#          If VNC support is not compiled in, FeatureDisabled
+#
+# Since: 0.14.0
+##
+{ 'command': 'query-vnc', 'returns': 'VncInfo' }
+
+##
 # @quit:
 #
 # This command will cause the QEMU process to exit gracefully.  While every
diff --git a/qmp-commands.hx b/qmp-commands.hx
index a250a7a..0953d3f 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1741,6 +1741,12 @@ Example:
 
 EQMP
 
+    {
+        .name       = "query-vnc",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_query_vnc,
+    },
+
 SQMP
 query-spice
 -----------
diff --git a/qmp.c b/qmp.c
index e84922b..3a58cfe 100644
--- a/qmp.c
+++ b/qmp.c
@@ -95,3 +95,13 @@ void qmp_cpu(int64_t index, Error **errp)
 {
     /* Just do nothing */
 }
+
+#ifndef CONFIG_VNC
+/* If VNC support is enabled, the "true" query-vnc command is
+   defined in the VNC subsystem */
+VncInfo *qmp_query_vnc(Error **errp)
+{
+    error_set(errp, QERR_FEATURE_DISABLED, "vnc");
+    return NULL;
+};
+#endif
diff --git a/ui/vnc.c b/ui/vnc.c
index fc3a612..32d4cb7 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -31,6 +31,7 @@
 #include "qemu-timer.h"
 #include "acl.h"
 #include "qemu-objects.h"
+#include "qmp-commands.h"
 
 #define VNC_REFRESH_INTERVAL_BASE 30
 #define VNC_REFRESH_INTERVAL_INC  50
@@ -274,80 +275,110 @@ static void vnc_qmp_event(VncState *vs, MonitorEvent 
event)
     qobject_decref(data);
 }
 
-static void info_vnc_iter(QObject *obj, void *opaque)
+static VncClientInfo *qmp_query_vnc_client(const VncState *client)
 {
-    QDict *client;
-    Monitor *mon = opaque;
+    struct sockaddr_storage sa;
+    socklen_t salen = sizeof(sa);
+    char host[NI_MAXHOST];
+    char serv[NI_MAXSERV];
+    VncClientInfo *info;
+
+    if (getpeername(client->csock, (struct sockaddr *)&sa, &salen) < 0) {
+        return NULL;
+    }
+
+    if (getnameinfo((struct sockaddr *)&sa, salen,
+                    host, sizeof(host),
+                    serv, sizeof(serv),
+                    NI_NUMERICHOST | NI_NUMERICSERV) < 0) {
+        return NULL;
+    }
 
-    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"));
+    info = g_malloc0(sizeof(*info));
+    info->host = g_strdup(host);
+    info->service = g_strdup(serv);
+    info->family = g_strdup(inet_strfamily(sa.ss_family));
 
 #ifdef CONFIG_VNC_TLS
-    monitor_printf(mon, "  x509_dname: %s\n",
-        qdict_haskey(client, "x509_dname") ?
-        qdict_get_str(client, "x509_dname") : "none");
+    if (client->tls.session && client->tls.dname) {
+        info->has_x509_dname = true;
+        info->x509_dname = g_strdup(client->tls.dname);
+    }
 #endif
 #ifdef CONFIG_VNC_SASL
-    monitor_printf(mon, "    username: %s\n",
-        qdict_haskey(client, "sasl_username") ?
-        qdict_get_str(client, "sasl_username") : "none");
-#endif
-}
-
-void do_info_vnc_print(Monitor *mon, const QObject *data)
-{
-    QDict *server;
-    QList *clients;
-
-    server = qobject_to_qdict(data);
-    if (qdict_get_bool(server, "enabled") == 0) {
-        monitor_printf(mon, "Server: disabled\n");
-        return;
+    if (client->sasl.conn && client->sasl.username) {
+        info->has_sasl_username = true;
+        info->sasl_username = g_strdup(client->sasl.username);
     }
+#endif
 
-    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_get_str(server, "auth"));
-
-    clients = qdict_get_qlist(server, "clients");
-    if (qlist_empty(clients)) {
-        monitor_printf(mon, "Client: none\n");
-    } else {
-        qlist_iter(clients, info_vnc_iter, mon);
-    }
+    return info;
 }
 
-void do_info_vnc(Monitor *mon, QObject **ret_data)
+VncInfo *qmp_query_vnc(Error **errp)
 {
+    VncInfo *info = g_malloc0(sizeof(*info));
+
     if (vnc_display == NULL || vnc_display->display == NULL) {
-        *ret_data = qobject_from_jsonf("{ 'enabled': false }");
+        info->enabled = false;
     } else {
-        QList *clist;
+        VncClientInfoList *cur_item = NULL;
+        struct sockaddr_storage sa;
+        socklen_t salen = sizeof(sa);
+        char host[NI_MAXHOST];
+        char serv[NI_MAXSERV];
         VncState *client;
 
-        clist = qlist_new();
+        info->enabled = true;
+
+        /* for compatibility with the original command */
+        info->has_clients = true;
+
         QTAILQ_FOREACH(client, &vnc_display->clients, next) {
-            if (client->info) {
-                /* incref so that it's not freed by upper layers */
-                qobject_incref(client->info);
-                qlist_append_obj(clist, client->info);
+            VncClientInfoList *cinfo = g_malloc0(sizeof(*info));
+            cinfo->value = qmp_query_vnc_client(client);
+
+            /* XXX: waiting for the qapi to support GSList */
+            if (!cur_item) {
+                info->clients = cur_item = cinfo;
+            } else {
+                cur_item->next = cinfo;
+                cur_item = cinfo;
             }
         }
 
-        *ret_data = qobject_from_jsonf("{ 'enabled': true, 'clients': %p }",
-                                       QOBJECT(clist));
-        assert(*ret_data != NULL);
+        if (getsockname(vnc_display->lsock, (struct sockaddr *)&sa,
+                        &salen) == -1) {
+            error_set(errp, QERR_UNDEFINED_ERROR);
+            goto out_error;
+        }
 
-        if (vnc_server_info_put(qobject_to_qdict(*ret_data)) < 0) {
-            qobject_decref(*ret_data);
-            *ret_data = NULL;
+        if (getnameinfo((struct sockaddr *)&sa, salen,
+                        host, sizeof(host),
+                        serv, sizeof(serv),
+                        NI_NUMERICHOST | NI_NUMERICSERV) < 0) {
+            error_set(errp, QERR_UNDEFINED_ERROR);
+            goto out_error;
         }
+
+        info->has_host = true;
+        info->host = g_strdup(host);
+
+        info->has_service = true;
+        info->service = g_strdup(serv);
+
+        info->has_family = true;
+        info->family = g_strdup(inet_strfamily(sa.ss_family));
+
+        info->has_auth = true;
+        info->auth = g_strdup(vnc_auth_name(vnc_display));
     }
+
+    return info;
+
+out_error:
+    qapi_free_VncInfo(info);
+    return NULL;
 }
 
 /* TODO
-- 
1.7.7.1.431.g10b2a