[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] qapi: Adding websocket information inside VncIn
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH] qapi: Adding websocket information inside VncInfo structure. |
Date: |
Fri, 16 Oct 2015 10:50:36 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
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 <address@hidden>
> ---
> 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
signature.asc
Description: OpenPGP digital signature