[Qemu-devel] [PATCH] qapi: Adding websocket information inside VncInfo structure.

[Qemu-devel] [PATCH] qapi: Adding websocket information inside VncInfo structure.

[Julio Faracco]

When the user setup a vnc server to QEMU using the argument "-vnc
0.0.0.0:0,websocket=5701" for example, he cannot get information about which
websocket port VNC is running. The command "info vnc" shows only details about
the main VNC server and the clients. You can confirm that a websocket service
is running, checking the ports that are opened. This commit introduce two new
fields inside "VncInfo" either: "webservice" which contains the port as a
string and the boolean value "has_websocket" that copies the status of
ws_enabled from VNC Display.

Fixes #1486278

Signed-Off-By: Julio Faracco <jcfaracco@gmail.com>
---
 hmp.c            |  3 +++
 qapi-schema.json |  3 ++-
 ui/vnc.c         | 25 +++++++++++++++++++++++++
 3 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/hmp.c b/hmp.c
index 5048eee..3fc74e0 100644
--- a/hmp.c
+++ b/hmp.c
@@ -559,6 +559,9 @@ void hmp_info_vnc(Monitor *mon, const QDict *qdict)
     if (info->has_host && info->has_service) {
         monitor_printf(mon, "     address: %s:%s\n", info->host, info->service);
     }
+    if(info->has_websocket) {
+        monitor_printf(mon, "   websocket: %s:%s\n", info->host, info->webservice);
+    }
     if (info->has_auth) {
         monitor_printf(mon, "        auth: %s\n", info->auth);
     }
diff --git a/qapi-schema.json b/qapi-schema.json
index a386605..e97f78f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -916,7 +916,8 @@
 { 'struct': 'VncInfo',
   'data': {'enabled': 'bool', '*host': 'str',
            '*family': 'NetworkAddressFamily',
-           '*service': 'str', '*auth': 'str', '*clients': ['VncClientInfo']} }
+           '*service': 'str', '*auth': 'str', '*clients': ['VncClientInfo'],
+           '*webservice': 'str', 'has_websocket': 'bool'} }
 
 ##
 # @VncPriAuth:
diff --git a/ui/vnc.c b/ui/vnc.c
index d73966a..68b5960 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -411,6 +411,7 @@ VncInfo *qmp_query_vnc(Error **errp)
         socklen_t salen = sizeof(sa);
         char host[NI_MAXHOST];
         char serv[NI_MAXSERV];
+        char webserv[NI_MAXSERV];
 
         info->enabled = true;
 
@@ -447,6 +448,30 @@ VncInfo *qmp_query_vnc(Error **errp)
 
         info->has_auth = true;
         info->auth = g_strdup(vnc_auth_name(vd));
+
+        info->has_websocket = vd->ws_enabled;
+
+        if (vd->ws_enabled) {
+            if (vd->lwebsock == -1) {
+                return info;
+            }
+
+            if (getsockname(vd->lwebsock, (struct sockaddr *)&sa,
+                            &salen) == -1) {
+                error_setg(errp, QERR_UNDEFINED_ERROR);
+                goto out_error;
+            }
+
+            if (getnameinfo((struct sockaddr *)&sa, salen,
+                            host, sizeof(host),
+                            webserv, sizeof(serv),
+                            NI_NUMERICHOST | NI_NUMERICSERV) < 0) {
+                error_setg(errp, QERR_UNDEFINED_ERROR);
+                goto out_error;
+            }
+        }
+
+        info->webservice = g_strdup(webserv);
     }
 
     return info;
-- 
1.9.1

Re: [Qemu-devel] [PATCH] qapi: Adding websocket information inside VncInfo structure.

[Eric Blake]

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

On 10/16/2015 10:37 AM, Julio Faracco wrote:
> When the user setup a vnc server to QEMU using the argument "-vnc
> 0.0.0.0:0,websocket=5701" for example, he cannot get information about which
> websocket port VNC is running. The command "info vnc" shows only details about
> the main VNC server and the clients. You can confirm that a websocket service
> is running, checking the ports that are opened. This commit introduce two new
> fields inside "VncInfo" either: "webservice" which contains the port as a
> string and the boolean value "has_websocket" that copies the status of
> ws_enabled from VNC Display.
> 
> Fixes #1486278
> 
> Signed-Off-By: Julio Faracco <jcfaracco@gmail.com>
> ---
>  hmp.c            |  3 +++
>  qapi-schema.json |  3 ++-
>  ui/vnc.c         | 25 +++++++++++++++++++++++++
>  3 files changed, 30 insertions(+), 1 deletion(-)
> 

The choice of name 'has_websocket' won't work; it is forbidden by my
pending qapi patch:
https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg03733.html

> diff --git a/hmp.c b/hmp.c
> index 5048eee..3fc74e0 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -559,6 +559,9 @@ void hmp_info_vnc(Monitor *mon, const QDict *qdict)
>      if (info->has_host && info->has_service) {
>          monitor_printf(mon, "     address: %s:%s\n", info->host, info->service);
>      }
> +    if(info->has_websocket) {

Missing space after if.

> +        monitor_printf(mon, "   websocket: %s:%s\n", info->host, info->webservice);
> +    }

But you don't need has_websocket in the first place. Since you declared
webservice as optional in qapi, this should be:

if (info->has_webservice) {
   ... use info->webservice
}

>      if (info->has_auth) {
>          monitor_printf(mon, "        auth: %s\n", info->auth);
>      }
> diff --git a/qapi-schema.json b/qapi-schema.json
> index a386605..e97f78f 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -916,7 +916,8 @@
>  { 'struct': 'VncInfo',
>    'data': {'enabled': 'bool', '*host': 'str',
>             '*family': 'NetworkAddressFamily',
> -           '*service': 'str', '*auth': 'str', '*clients': ['VncClientInfo']} }
> +           '*service': 'str', '*auth': 'str', '*clients': ['VncClientInfo'],
> +           '*webservice': 'str', 'has_websocket': 'bool'} }

Missing documentation on the new optional webservice; it should look like:

# @webservice: #optional string that means something... (since 2.5)

and as argued above, I don't think you want has_websocket in the user
interface. The fact that webservice is optional is sufficient - if it is
present in QMP, then there is a web service; if it is absent, then
either this qemu is too old to have the schema support, or there is no
webservice.  And if qemu is new enough for introspection, then using
query-qmp-schema will tell us whether qemu was new enough to support it
and therefore that it is intentionally omitted because there is no
webservice.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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

Re: [Qemu-devel] [PATCH] qapi: Adding websocket information inside VncInfo structure.

[Gerd Hoffmann]

  Hi,

> diff --git a/qapi-schema.json b/qapi-schema.json
> index a386605..e97f78f 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -916,7 +916,8 @@
>  { 'struct': 'VncInfo',
>    'data': {'enabled': 'bool', '*host': 'str',
>             '*family': 'NetworkAddressFamily',
> -           '*service': 'str', '*auth': 'str', '*clients': ['VncClientInfo']} }
> +           '*service': 'str', '*auth': 'str', '*clients': ['VncClientInfo'],
> +           '*webservice': 'str', 'has_websocket': 'bool'} }
>  
>  ##
>  # @VncPriAuth:

Please leave VncInfo alone.  There is VncInfo2 which should fill all
your needs.

You might consider switching hmp_info_vnc() to use the new
qmp_query_vnc_servers() instead of the old qmp_query_vnc().

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] qapi: Adding websocket information inside VncInfo structure.

[Julio Faracco]

Thanks Erik and Gerd for you feedback...

I will rebase the changes consider the opinions...
I agree about VncInfo2... because some structures are ready to use,
including websocket.
Without adding new fields or other changes.

Julio Cesar Faracco


2015-10-19 6:17 GMT-02:00 Gerd Hoffmann <kraxel@redhat.com>:
>   Hi,
>
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index a386605..e97f78f 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -916,7 +916,8 @@
>>  { 'struct': 'VncInfo',
>>    'data': {'enabled': 'bool', '*host': 'str',
>>             '*family': 'NetworkAddressFamily',
>> -           '*service': 'str', '*auth': 'str', '*clients': ['VncClientInfo']} }
>> +           '*service': 'str', '*auth': 'str', '*clients': ['VncClientInfo'],
>> +           '*webservice': 'str', 'has_websocket': 'bool'} }
>>
>>  ##
>>  # @VncPriAuth:
>
> Please leave VncInfo alone.  There is VncInfo2 which should fill all
> your needs.
>
> You might consider switching hmp_info_vnc() to use the new
> qmp_query_vnc_servers() instead of the old qmp_query_vnc().
>
> cheers,
>   Gerd
>
>