[Qemu-devel] [PULL 0/3] Ui 20170717 patches

[Qemu-devel] [PULL 0/3] Ui 20170717 patches

[Gerd Hoffmann]

The following changes since commit 6e2c46334385c7e295ac883c801c81b4925fb54f:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20170711' into staging (2017-07-13 10:47:10 +0100)

are available in the git repository at:

  git://git.kraxel.org/qemu tags/ui-20170717-pull-request

for you to fetch changes up to feb47cf2fac5a0b8fe8509b18edf077e065dc97f:

  keymaps: fr-ca: add missing keys (2017-07-17 11:36:41 +0200)

----------------------------------------------------------------
ui: vnc and keymap updates for 2.10

----------------------------------------------------------------

Alexander Graf (1):
  vnc: Set default kbd delay to 10ms

Dr. David Alan Gilbert (1):
  hmp: Update info vnc

Gerd Hoffmann (1):
  keymaps: fr-ca: add missing keys

 hmp.c                 | 104 +++++++++++++++++++++++++++++++++++---------------
 ui/vnc.c              |   2 +-
 pc-bios/keymaps/fr-ca |   2 +
 qemu-options.hx       |   2 +-
 4 files changed, 77 insertions(+), 33 deletions(-)

-- 
2.9.3

[Qemu-devel] [PULL 1/3] vnc: Set default kbd delay to 10ms

[Gerd Hoffmann]

From: Alexander Graf <agraf@suse.de>

The current VNC default keyboard delay is 1ms. With that we're constantly
typing faster than the guest receives keyboard events from an XHCI attached
USB HID device.

The default keyboard delay time in the input layer however is 10ms. I don't know
how that number came to be, but empirical tests on some OpenQA driven ARM
systems show that 10ms really is a reasonable default number for the delay.

This patch moves the VNC delay also to 10ms. That way our default is much
safer (good!) and also consistent with the input layer default (also good!).

Signed-off-by: Alexander Graf <agraf@suse.de>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Message-id: 1499863425-103133-1-git-send-email-agraf@suse.de
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/vnc.c        | 2 +-
 qemu-options.hx | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 26136f5d29..eb91559b6b 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3808,7 +3808,7 @@ void vnc_display_open(const char *id, Error **errp)
     }
 
     lock_key_sync = qemu_opt_get_bool(opts, "lock-key-sync", true);
-    key_delay_ms = qemu_opt_get_number(opts, "key-delay-ms", 1);
+    key_delay_ms = qemu_opt_get_number(opts, "key-delay-ms", 10);
     sasl = qemu_opt_get_bool(opts, "sasl", false);
 #ifndef CONFIG_VNC_SASL
     if (sasl) {
diff --git a/qemu-options.hx b/qemu-options.hx
index 76b1c67737..6909285e85 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1753,7 +1753,7 @@ spec but is traditional QEMU behavior.
 @item key-delay-ms
 
 Set keyboard delay, for key down and key up events, in milliseconds.
-Default is 1.  Keyboards are low-bandwidth devices, so this slowdown
+Default is 10.  Keyboards are low-bandwidth devices, so this slowdown
 can help the device and guest to keep up and not lose events in case
 events are arriving in bulk.  Possible causes for the latter are flaky
 network connections, or scripts for automated testing.
-- 
2.9.3

[Qemu-devel] [PULL 2/3] hmp: Update info vnc

[Gerd Hoffmann]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

The QMP query-vnc interfaces have gained a lot more information that
the HMP interfaces hasn't got yet. Update it.

Note the output format has changed, but this is HMP so that's OK.

In particular, this now includes client information for reverse
connections:

-vnc :0
(qemu) info vnc
default:
  Server: 0.0.0.0:5900 (ipv4)
    Auth: none (Sub: none)

  (Now connect a client)

(qemu) info vnc
default:
  Server: 0.0.0.0:5900 (ipv4)
    Auth: none (Sub: none)
  Client: 127.0.0.1:51828 (ipv4)
    x509_dname: none
    sasl_username: none

-vnc localhost:7000,reverse
(qemu) info vnc
default:
  Client: ::1:7000 (ipv6)
    x509_dname: none
    sasl_username: none
  Auth: none (Sub: none)

-vnc :1,password,id=pass -vnc localhost:7000,reverse
(qemu) info vnc
default:
  Client: ::1:7000 (ipv6)
    x509_dname: none
    sasl_username: none
  Auth: none (Sub: none)
rev:
  Server: 0.0.0.0:5901 (ipv4)
    Auth: vnc (Sub: none)
  Client: 127.0.0.1:53616 (ipv4)
    x509_dname: none
    sasl_username: none

This was originally RH bz 1461682

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-id: 20170711154414.21111-1-dgilbert@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hmp.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 73 insertions(+), 31 deletions(-)

diff --git a/hmp.c b/hmp.c
index dee40284c1..3e1e766593 100644
--- a/hmp.c
+++ b/hmp.c
@@ -600,50 +600,92 @@ void hmp_info_blockstats(Monitor *mon, const QDict *qdict)
     qapi_free_BlockStatsList(stats_list);
 }
 
+/* Helper for hmp_info_vnc_clients, _servers */
+static void hmp_info_VncBasicInfo(Monitor *mon, VncBasicInfo *info,
+                                  const char *name)
+{
+    monitor_printf(mon, "  %s: %s:%s (%s%s)\n",
+                   name,
+                   info->host,
+                   info->service,
+                   NetworkAddressFamily_lookup[info->family],
+                   info->websocket ? " (Websocket)" : "");
+}
+
+/* Helper displaying and auth and crypt info */
+static void hmp_info_vnc_authcrypt(Monitor *mon, const char *indent,
+                                   VncPrimaryAuth auth,
+                                   VncVencryptSubAuth *vencrypt)
+{
+    monitor_printf(mon, "%sAuth: %s (Sub: %s)\n", indent,
+                   VncPrimaryAuth_lookup[auth],
+                   vencrypt ? VncVencryptSubAuth_lookup[*vencrypt] : "none");
+}
+
+static void hmp_info_vnc_clients(Monitor *mon, VncClientInfoList *client)
+{
+    while (client) {
+        VncClientInfo *cinfo = client->value;
+
+        hmp_info_VncBasicInfo(mon, qapi_VncClientInfo_base(cinfo), "Client");
+        monitor_printf(mon, "    x509_dname: %s\n",
+                       cinfo->has_x509_dname ?
+                       cinfo->x509_dname : "none");
+        monitor_printf(mon, "    sasl_username: %s\n",
+                       cinfo->has_sasl_username ?
+                       cinfo->sasl_username : "none");
+
+        client = client->next;
+    }
+}
+
+static void hmp_info_vnc_servers(Monitor *mon, VncServerInfo2List *server)
+{
+    while (server) {
+        VncServerInfo2 *sinfo = server->value;
+        hmp_info_VncBasicInfo(mon, qapi_VncServerInfo2_base(sinfo), "Server");
+        hmp_info_vnc_authcrypt(mon, "    ", sinfo->auth,
+                               sinfo->has_vencrypt ? &sinfo->vencrypt : NULL);
+        server = server->next;
+    }
+}
+
 void hmp_info_vnc(Monitor *mon, const QDict *qdict)
 {
-    VncInfo *info;
+    VncInfo2List *info2l;
     Error *err = NULL;
-    VncClientInfoList *client;
 
-    info = qmp_query_vnc(&err);
+    info2l = qmp_query_vnc_servers(&err);
     if (err) {
         error_report_err(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 (!info2l) {
+        monitor_printf(mon, "None\n");
+        return;
     }
 
-    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");
+    while (info2l) {
+        VncInfo2 *info = info2l->value;
+        monitor_printf(mon, "%s:\n", info->id);
+        hmp_info_vnc_servers(mon, info->server);
+        hmp_info_vnc_clients(mon, info->clients);
+        if (!info->server) {
+            /* The server entry displays its auth, we only
+             * need to display in the case of 'reverse' connections
+             * where there's no server.
+             */
+            hmp_info_vnc_authcrypt(mon, "  ", info->auth,
+                               info->has_vencrypt ? &info->vencrypt : NULL);
+        }
+        if (info->has_display) {
+            monitor_printf(mon, "  Display: %s\n", info->display);
         }
+        info2l = info2l->next;
     }
 
-out:
-    qapi_free_VncInfo(info);
+    qapi_free_VncInfo2List(info2l);
+
 }
 
 #ifdef CONFIG_SPICE
-- 
2.9.3

Re: [Qemu-devel] [PULL 2/3] hmp: Update info vnc

[Peter Maydell]

On Mon, 17 Jul 2017 at 10:40, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> The QMP query-vnc interfaces have gained a lot more information that
> the HMP interfaces hasn't got yet. Update it.
>
> Note the output format has changed, but this is HMP so that's OK.

Hi; another "ancient change Coverity has only just noticed has
a problem" email :-)   This is CID 1421932. It looks like any
"info vnc" will leak memory if there are any VNC servers to
display info about...

>  void hmp_info_vnc(Monitor *mon, const QDict *qdict)
>  {
> -    VncInfo *info;
> +    VncInfo2List *info2l;
>      Error *err = NULL;
> -    VncClientInfoList *client;
>
> -    info = qmp_query_vnc(&err);
> +    info2l = qmp_query_vnc_servers(&err);

Here we get a list of VNC servers, which is allocated memory...

>      if (err) {
>          error_report_err(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 (!info2l) {
> +        monitor_printf(mon, "None\n");
> +        return;
>      }
>
> -    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");
> +    while (info2l) {
> +        VncInfo2 *info = info2l->value;
> +        monitor_printf(mon, "%s:\n", info->id);
> +        hmp_info_vnc_servers(mon, info->server);
> +        hmp_info_vnc_clients(mon, info->clients);
> +        if (!info->server) {
> +            /* The server entry displays its auth, we only
> +             * need to display in the case of 'reverse' connections
> +             * where there's no server.
> +             */
> +            hmp_info_vnc_authcrypt(mon, "  ", info->auth,
> +                               info->has_vencrypt ? &info->vencrypt : NULL);
> +        }
> +        if (info->has_display) {
> +            monitor_printf(mon, "  Display: %s\n", info->display);
>          }
> +        info2l = info2l->next;

...but the loop iteration here updates 'info2l' as it goes along...

>      }
>
> -out:
> -    qapi_free_VncInfo(info);
> +    qapi_free_VncInfo2List(info2l);

...so here we end up passing NULL to qapi_free_VncInfo2List(),
which will do nothing, leaking the whole list.

Would somebody like to send a patch?

thanks
-- PMM

Re: [Qemu-devel] [PULL 2/3] hmp: Update info vnc

[Dr. David Alan Gilbert]

* Peter Maydell (peter.maydell@linaro.org) wrote:
> On Mon, 17 Jul 2017 at 10:40, Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >
> > The QMP query-vnc interfaces have gained a lot more information that
> > the HMP interfaces hasn't got yet. Update it.
> >
> > Note the output format has changed, but this is HMP so that's OK.
> 
> Hi; another "ancient change Coverity has only just noticed has
> a problem" email :-)   This is CID 1421932. It looks like any
> "info vnc" will leak memory if there are any VNC servers to
> display info about...
> 
> >  void hmp_info_vnc(Monitor *mon, const QDict *qdict)
> >  {
> > -    VncInfo *info;
> > +    VncInfo2List *info2l;
> >      Error *err = NULL;
> > -    VncClientInfoList *client;
> >
> > -    info = qmp_query_vnc(&err);
> > +    info2l = qmp_query_vnc_servers(&err);
> 
> Here we get a list of VNC servers, which is allocated memory...
> 
> >      if (err) {
> >          error_report_err(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 (!info2l) {
> > +        monitor_printf(mon, "None\n");
> > +        return;
> >      }
> >
> > -    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");
> > +    while (info2l) {
> > +        VncInfo2 *info = info2l->value;
> > +        monitor_printf(mon, "%s:\n", info->id);
> > +        hmp_info_vnc_servers(mon, info->server);
> > +        hmp_info_vnc_clients(mon, info->clients);
> > +        if (!info->server) {
> > +            /* The server entry displays its auth, we only
> > +             * need to display in the case of 'reverse' connections
> > +             * where there's no server.
> > +             */
> > +            hmp_info_vnc_authcrypt(mon, "  ", info->auth,
> > +                               info->has_vencrypt ? &info->vencrypt : NULL);
> > +        }
> > +        if (info->has_display) {
> > +            monitor_printf(mon, "  Display: %s\n", info->display);
> >          }
> > +        info2l = info2l->next;
> 
> ...but the loop iteration here updates 'info2l' as it goes along...
> 
> >      }
> >
> > -out:
> > -    qapi_free_VncInfo(info);
> > +    qapi_free_VncInfo2List(info2l);
> 
> ...so here we end up passing NULL to qapi_free_VncInfo2List(),
> which will do nothing, leaking the whole list.
> 
> Would somebody like to send a patch?

Oops, yes I can look at that; I guess something along the lines of an
info2l_orig  and free that at the end.

Dave

> thanks
> -- PMM
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

[Qemu-devel] [PULL 3/3] keymaps: fr-ca: add missing keys

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20170712072305.29233-1-kraxel@redhat.com
---
 pc-bios/keymaps/fr-ca | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/pc-bios/keymaps/fr-ca b/pc-bios/keymaps/fr-ca
index b645208e42..030f56a78e 100644
--- a/pc-bios/keymaps/fr-ca
+++ b/pc-bios/keymaps/fr-ca
@@ -48,3 +48,5 @@ parenleft 0xa shift
 parenright 0xb shift
 underscore 0xc shift
 plus 0xd shift
+minus 0xc
+equal 0xd
-- 
2.9.3

Re: [Qemu-devel] [PULL 0/3] Ui 20170717 patches

[Peter Maydell]

On 17 July 2017 at 10:38, Gerd Hoffmann <kraxel@redhat.com> wrote:
> The following changes since commit 6e2c46334385c7e295ac883c801c81b4925fb54f:
>
>   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20170711' into staging (2017-07-13 10:47:10 +0100)
>
> are available in the git repository at:
>
>   git://git.kraxel.org/qemu tags/ui-20170717-pull-request
>
> for you to fetch changes up to feb47cf2fac5a0b8fe8509b18edf077e065dc97f:
>
>   keymaps: fr-ca: add missing keys (2017-07-17 11:36:41 +0200)
>
> ----------------------------------------------------------------
> ui: vnc and keymap updates for 2.10
>
> ----------------------------------------------------------------
>
> Alexander Graf (1):
>   vnc: Set default kbd delay to 10ms
>
> Dr. David Alan Gilbert (1):
>   hmp: Update info vnc
>
> Gerd Hoffmann (1):
>   keymaps: fr-ca: add missing keys

Applied, thanks.

-- PMM