[PULL 0/5] Ui 20200831 patches

[PULL 0/5] Ui 20200831 patches

[Gerd Hoffmann]

The following changes since commit 39335fab59e11cfda9b7cf63929825db2dd3a3e0:

  Merge remote-tracking branch 'remotes/vivier2/tags/linux-user-for-5.2-pull-=
request' into staging (2020-08-28 22:30:11 +0100)

are available in the Git repository at:

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

for you to fetch changes up to e54635710f1cf667191a0a1d9df9a37d1c9b0ad0:

  ui/gtk: Update refresh interval after widget is realized (2020-08-31 10:41:=
43 +0200)

----------------------------------------------------------------
ui: memleak fixes.
gtk: refresh interval fix.
keymaps: don't require qemu-keymap (fix gitlab ci).
spice: add mouse buttons.

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

Frediano Ziglio (1):
  ui: Add more mouse buttons to SPICE

Gerd Hoffmann (1):
  meson: fix keymaps witout qemu-keymap

Pan Nengyuan (2):
  ui/gtk-gl-area: Plug memleak in gd_gl_area_create_context()
  vnc-auth-sasl: Plug memleak in vnc_socket_ip_addr_string

Philippe Mathieu-Daud=C3=A9 (1):
  ui/gtk: Update refresh interval after widget is realized

 ui/gtk-gl-area.c            | 11 ++++++++
 ui/gtk.c                    | 52 ++++++++++++++++++-------------------
 ui/spice-input.c            |  2 ++
 ui/vnc-auth-sasl.c          |  1 +
 pc-bios/keymaps/meson.build | 28 +++++++++++++-------
 5 files changed, 59 insertions(+), 35 deletions(-)

--=20
2.27.0

[PULL 1/5] ui/gtk-gl-area: Plug memleak in gd_gl_area_create_context()

[Gerd Hoffmann]

From: Pan Nengyuan <pannengyuan@huawei.com>

Receiving error in local variable err, and forgot to free it.
This patch check the return value of 'gdk_window_create_gl_context'
and 'gdk_gl_context_realize', then free err to fix it.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
Message-Id: <20200831134315.1221-6-pannengyuan@huawei.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/gtk-gl-area.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
index 85f9d14c51f1..98c22d23f501 100644
--- a/ui/gtk-gl-area.c
+++ b/ui/gtk-gl-area.c
@@ -147,10 +147,21 @@ QEMUGLContext gd_gl_area_create_context(DisplayChangeListener *dcl,
     gtk_gl_area_make_current(GTK_GL_AREA(vc->gfx.drawing_area));
     window = gtk_widget_get_window(vc->gfx.drawing_area);
     ctx = gdk_window_create_gl_context(window, &err);
+    if (err) {
+        g_printerr("Create gdk gl context failed: %s\n", err->message);
+        g_error_free(err);
+        return NULL;
+    }
     gdk_gl_context_set_required_version(ctx,
                                         params->major_ver,
                                         params->minor_ver);
     gdk_gl_context_realize(ctx, &err);
+    if (err) {
+        g_printerr("Realize gdk gl context failed: %s\n", err->message);
+        g_error_free(err);
+        g_clear_object(&ctx);
+        return NULL;
+    }
     return ctx;
 }
 
-- 
2.27.0

[PULL 2/5] vnc-auth-sasl: Plug memleak in vnc_socket_ip_addr_string

[Gerd Hoffmann]

From: Pan Nengyuan <pannengyuan@huawei.com>

'addr' is forgot to free in vnc_socket_ip_addr_string error path. Fix that.

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
Reviewed-by: Li Qiang <liq3ea@gmail.com>
Message-Id: <20200831134315.1221-11-pannengyuan@huawei.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/vnc-auth-sasl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ui/vnc-auth-sasl.c b/ui/vnc-auth-sasl.c
index 7b2b09f24276..0517b2ead9ce 100644
--- a/ui/vnc-auth-sasl.c
+++ b/ui/vnc-auth-sasl.c
@@ -522,6 +522,7 @@ vnc_socket_ip_addr_string(QIOChannelSocket *ioc,
 
     if (addr->type != SOCKET_ADDRESS_TYPE_INET) {
         error_setg(errp, "Not an inet socket type");
+        qapi_free_SocketAddress(addr);
         return NULL;
     }
     ret = g_strdup_printf("%s;%s", addr->u.inet.host, addr->u.inet.port);
-- 
2.27.0

[PULL 3/5] meson: fix keymaps witout qemu-keymap

[Gerd Hoffmann]

In case the qemu-keymap tool generating them is neither installed on the
system nor built from sources (due to xkbcommon not being available)
qemu will not find the keymaps when started directly from the build
tree,

This happens because commit ddcf607fa3d6 ("meson: drop keymaps symlink")
removed the symlink to the source tree, and the special handling for
install doesn't help in case we do not install qemu.

Lets fix that by simply copying over the file from the source tree as
fallback.

Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20200827102617.14448-1-kraxel@redhat.com
---
 pc-bios/keymaps/meson.build | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/pc-bios/keymaps/meson.build b/pc-bios/keymaps/meson.build
index b737c8223031..e102dd56b454 100644
--- a/pc-bios/keymaps/meson.build
+++ b/pc-bios/keymaps/meson.build
@@ -38,19 +38,29 @@ if meson.is_cross_build() or 'CONFIG_XKBCOMMON' not in config_host
 else
   native_qemu_keymap = qemu_keymap
 endif
+
 t = []
 foreach km, args: keymaps
-  t += custom_target(km,
-                     build_by_default: true,
-                     output: km,
-                     command: [native_qemu_keymap, '-f', '@OUTPUT@', args.split()],
-                     install_dir: config_host['qemu_datadir'] / 'keymaps')
+  if native_qemu_keymap.found()
+    # generate with qemu-kvm
+    t += custom_target(km,
+                       build_by_default: true,
+                       output: km,
+                       command: [native_qemu_keymap, '-f', '@OUTPUT@', args.split()],
+                       install_dir: config_host['qemu_datadir'] / 'keymaps')
+  else
+    # copy from source tree
+    t += custom_target(km,
+                       build_by_default: true,
+		       input: km,
+                       output: km,
+                       command: ['cp', '@INPUT@', '@OUTPUT@'],
+                       install_dir: config_host['qemu_datadir'] / 'keymaps')
+  endif
 endforeach
-if t.length() > 0
+
+if native_qemu_keymap.found()
   alias_target('update-keymaps', t)
-else
-  # install from the source tree
-  install_data(keymaps.keys(), install_dir: config_host['qemu_datadir'] / 'keymaps')
 endif
 
 install_data(['sl', 'sv'], install_dir: config_host['qemu_datadir'] / 'keymaps')
-- 
2.27.0

[PULL 4/5] ui: Add more mouse buttons to SPICE

[Gerd Hoffmann]

From: Frediano Ziglio <freddy77@gmail.com>

Add support for SIDE and EXTRA buttons.

The constants for buttons in both SPICE and QEMU are defined as
  LEFT
  MIDDLE
  RIGHT
  UP
  DOWN
  SIDE
  EXTRA
(same order).

"button_mask" contains for each bit the state of a button. Qemu currently
uses bits 0, 1, 2 respectively as LEFT, RIGHT, MIDDLE; also add bits 4
and 5 as UP and DOWN (using wheel movements). SPICE protocol uses
a bitmask based on the order above where LEFT is bit 0, MIDDLE is
bit 1 and so on till EXTRA being bit 6. To avoid clash with Qemu usage
SPICE bitmask from SIDE are move a bit more resulting respectively
in 0x40 and 0x80 values.

Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Message-id: 20200820145851.50846-1-fziglio@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/spice-input.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ui/spice-input.c b/ui/spice-input.c
index cd4bb0043fd9..d5bba231c95c 100644
--- a/ui/spice-input.c
+++ b/ui/spice-input.c
@@ -123,6 +123,8 @@ static void spice_update_buttons(QemuSpicePointer *pointer,
         [INPUT_BUTTON_RIGHT]       = 0x02,
         [INPUT_BUTTON_WHEEL_UP]    = 0x10,
         [INPUT_BUTTON_WHEEL_DOWN]  = 0x20,
+        [INPUT_BUTTON_SIDE]        = 0x40,
+        [INPUT_BUTTON_EXTRA]       = 0x80,
     };
 
     if (wheel < 0) {
-- 
2.27.0

[PULL 5/5] ui/gtk: Update refresh interval after widget is realized

[Gerd Hoffmann]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Nikola reported on Windows when gd_vc_gfx_init() is called, the
window is not yet realized, so we run gd_refresh_rate_millihz(NULL)
which returns 0 milli-Hertz.
When a Widget is realized, it fires a 'realized' event. We already
have the gd_draw_event() handler registered for this even, so simply
move the gd_refresh_rate_millihz() there. When the event fires, the
window is known to exist.
This completes commit c4c00922cc original intention.

Reported-by: Nikola Pavlica <pavlica.nikola@gmail.com>
Tested-by: Nikola Pavlica <pavlica.nikola@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20200817172331.598255-1-philmd@redhat.com
Suggested-by: Nikola Pavlica <pavlica.nikola@gmail.com>
Tested-by: Nikola Pavlica <pavlica.nikola@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/gtk.c | 52 ++++++++++++++++++++++++++--------------------------
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index b0cc08ad6da7..7a717ce8e5e4 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -744,6 +744,25 @@ static void gd_resize_event(GtkGLArea *area,
 
 #endif
 
+/*
+ * If available, return the refresh rate of the display in milli-Hertz,
+ * else return 0.
+ */
+static int gd_refresh_rate_millihz(GtkWidget *window)
+{
+#ifdef GDK_VERSION_3_22
+    GdkWindow *win = gtk_widget_get_window(window);
+
+    if (win) {
+        GdkDisplay *dpy = gtk_widget_get_display(window);
+        GdkMonitor *monitor = gdk_display_get_monitor_at_window(dpy, win);
+
+        return gdk_monitor_get_refresh_rate(monitor);
+    }
+#endif
+    return 0;
+}
+
 static gboolean gd_draw_event(GtkWidget *widget, cairo_t *cr, void *opaque)
 {
     VirtualConsole *vc = opaque;
@@ -751,6 +770,7 @@ static gboolean gd_draw_event(GtkWidget *widget, cairo_t *cr, void *opaque)
     int mx, my;
     int ww, wh;
     int fbw, fbh;
+    int refresh_rate_millihz;
 
 #if defined(CONFIG_OPENGL)
     if (vc->gfx.gls) {
@@ -771,6 +791,12 @@ static gboolean gd_draw_event(GtkWidget *widget, cairo_t *cr, void *opaque)
         return FALSE;
     }
 
+    refresh_rate_millihz = gd_refresh_rate_millihz(vc->window ?
+                                                   vc->window : s->window);
+    if (refresh_rate_millihz) {
+        vc->gfx.dcl.update_interval = MILLISEC_PER_SEC / refresh_rate_millihz;
+    }
+
     fbw = surface_width(vc->gfx.ds);
     fbh = surface_height(vc->gfx.ds);
 
@@ -1949,31 +1975,11 @@ static GtkWidget *gd_create_menu_machine(GtkDisplayState *s)
     return machine_menu;
 }
 
-/*
- * If available, return the refresh rate of the display in milli-Hertz,
- * else return 0.
- */
-static int gd_refresh_rate_millihz(GtkWidget *window)
-{
-#ifdef GDK_VERSION_3_22
-    GdkWindow *win = gtk_widget_get_window(window);
-
-    if (win) {
-        GdkDisplay *dpy = gtk_widget_get_display(window);
-        GdkMonitor *monitor = gdk_display_get_monitor_at_window(dpy, win);
-
-        return gdk_monitor_get_refresh_rate(monitor);
-    }
-#endif
-    return 0;
-}
-
 static GSList *gd_vc_gfx_init(GtkDisplayState *s, VirtualConsole *vc,
                               QemuConsole *con, int idx,
                               GSList *group, GtkWidget *view_menu)
 {
     bool zoom_to_fit = false;
-    int refresh_rate_millihz;
 
     vc->label = qemu_console_get_label(con);
     vc->s = s;
@@ -2031,12 +2037,6 @@ static GSList *gd_vc_gfx_init(GtkDisplayState *s, VirtualConsole *vc,
     vc->gfx.kbd = qkbd_state_init(con);
     vc->gfx.dcl.con = con;
 
-    refresh_rate_millihz = gd_refresh_rate_millihz(vc->window ?
-                                                   vc->window : s->window);
-    if (refresh_rate_millihz) {
-        vc->gfx.dcl.update_interval = MILLISEC_PER_SEC / refresh_rate_millihz;
-    }
-
     register_displaychangelistener(&vc->gfx.dcl);
 
     gd_connect_vc_gfx_signals(vc);
-- 
2.27.0

Re: [PULL 0/5] Ui 20200831 patches

[Peter Maydell]

On Mon, 31 Aug 2020 at 12:05, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> The following changes since commit 39335fab59e11cfda9b7cf63929825db2dd3a3e0:
>
>   Merge remote-tracking branch 'remotes/vivier2/tags/linux-user-for-5.2-pull-=
> request' into staging (2020-08-28 22:30:11 +0100)
>
> are available in the Git repository at:
>
>   git://git.kraxel.org/qemu tags/ui-20200831-pull-request
>
> for you to fetch changes up to e54635710f1cf667191a0a1d9df9a37d1c9b0ad0:
>
>   ui/gtk: Update refresh interval after widget is realized (2020-08-31 10:41:=
> 43 +0200)
>
> ----------------------------------------------------------------
> ui: memleak fixes.
> gtk: refresh interval fix.
> keymaps: don't require qemu-keymap (fix gitlab ci).
> spice: add mouse buttons.
>
> ----------------------------------------------------------------
>
> Frediano Ziglio (1):
>   ui: Add more mouse buttons to SPICE
>
> Gerd Hoffmann (1):
>   meson: fix keymaps witout qemu-keymap
>
> Pan Nengyuan (2):
>   ui/gtk-gl-area: Plug memleak in gd_gl_area_create_context()
>   vnc-auth-sasl: Plug memleak in vnc_socket_ip_addr_string
>
> Philippe Mathieu-Daud=C3=A9 (1):
>   ui/gtk: Update refresh interval after widget is realized

NB: something in your cover letter creation process is mangling UTF-8.

Fails to build, windows crossbuild:

./ninjatool -t ninja2make --omit clean dist uninstall < build.ninja >
Makefile.ninja
make[1]: Entering directory '/home/petmay01/qemu-for-merges/slirp'
make[1]: Nothing to be done for 'all'.
make[1]: Leaving directory '/home/petmay01/qemu-for-merges/slirp'
Generating qemu-version.h with a meson_exe.py custom command
Generating ar with a custom command
cp: '../../pc-bios/keymaps/ar' and 'pc-bios/keymaps/ar' are the same file
Generating bepo with a custom command
cp: '../../pc-bios/keymaps/bepo' and 'pc-bios/keymaps/bepo' are the same file
Makefile.ninja:5172: recipe for target 'pc-bios/keymaps/ar.stamp' failed
make: *** [pc-bios/keymaps/ar.stamp] Error 1
make: *** Waiting for unfinished jobs....
Makefile.ninja:5174: recipe for target 'pc-bios/keymaps/bepo.stamp' failed
make: *** [pc-bios/keymaps/bepo.stamp] Error 1
Generating cz with a custom command
cp: '../../pc-bios/keymaps/cz' and 'pc-bios/keymaps/cz' are the same file
Makefile.ninja:5176: recipe for target 'pc-bios/keymaps/cz.stamp' failed
make: *** [pc-bios/keymaps/cz.stamp] Error 1
make: Leaving directory '/home/petmay01/qemu-for-merges/build/w64'

Fails to build, x86:

[...]
Configuring 50-edk2-i386-secure.json using configuration
Configuring 50-edk2-x86_64-secure.json using configuration
Configuring 60-edk2-aarch64.json using configuration
Configuring 60-edk2-arm.json using configuration
Configuring 60-edk2-i386.json using configuration
Configuring 60-edk2-x86_64.json using configuration
Program qemu-keymap found: NO
../../pc-bios/keymaps/meson.build:58: WARNING: Custom target input
'ar' can't be converted to File object(s).
This will become a hard error in the future.
../../pc-bios/keymaps/meson.build:58: WARNING: Custom target input
'bepo' can't be converted to File object(s).
This will become a hard error in the future.
../../pc-bios/keymaps/meson.build:58: WARNING: Custom target input
'cz' can't be converted to File object(s).
This will become a hard error in the future.
../../pc-bios/keymaps/meson.build:58: WARNING: Custom target input
'da' can't be converted to File object(s).
This will become a hard error in the future.
../../pc-bios/keymaps/meson.build:58: WARNING: Custom target input
'de' can't be converted to File object(s).
This will become a hard error in the future.
[...]

Command line for building ['libcommon.fa'] is long, using a response file
Command line for building ['libqemu-aarch64-softmmu.fa'] is long,
using a response file
Command line for building ['qemu-system-aarch64'] is long, using a response file
Command line for building ['qemu-system-arm'] is long, using a response file
make: Leaving directory '/home/petmay01/linaro/qemu-for-merges/build/all'
make: Entering directory '/home/petmay01/linaro/qemu-for-merges/build/all'
/usr/bin/python3 -B
/home/petmay01/linaro/qemu-for-merges/meson/meson.py introspect
--tests | /usr/bin/python3 -B scripts/mtest2make.py > Makefile.mtest
make: Leaving directory '/home/petmay01/linaro/qemu-for-merges/build/all'
make: Entering directory '/home/petmay01/linaro/qemu-for-merges/build/all'
./ninjatool -t ninja2make --omit clean dist uninstall < build.ninja >
Makefile.ninja
make: Leaving directory '/home/petmay01/linaro/qemu-for-merges/build/all'
make: Entering directory '/home/petmay01/linaro/qemu-for-merges/build/all'
  GIT     ui/keycodemapdb tests/fp/berkeley-testfloat-3
tests/fp/berkeley-softfloat-3 meson dtc capstone slirp
make: Leaving directory '/home/petmay01/linaro/qemu-for-merges/build/all'
make[1]: Nothing to be done for 'all'.
make: *** No rule to make target '../../pc-bios/keymaps/ar', needed by
'pc-bios/keymaps/ar.stamp'. Stop.
make: *** Waiting for unfinished jobs....

thanks
-- PMM

Re: [PULL 0/5] Ui 20200831 patches

[Gerd Hoffmann]

  Hi,

> Generating ar with a custom command
> cp: '../../pc-bios/keymaps/ar' and 'pc-bios/keymaps/ar' are the same file

Hmm, strange.  No failures in gitlab ci:
	https://gitlab.com/kraxel/qemu/-/pipelines/183769485

Any chance this is an old build tree?  Specifically created before
commit ddcf607fa3d6 ("meson: drop keymaps symlink") was merged, and the
symlink is present still?

thanks,
  Gerd

Re: [PULL 0/5] Ui 20200831 patches

[Peter Maydell]

On Tue, 1 Sep 2020 at 15:16, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
>   Hi,
>
> > Generating ar with a custom command
> > cp: '../../pc-bios/keymaps/ar' and 'pc-bios/keymaps/ar' are the same file
>
> Hmm, strange.  No failures in gitlab ci:
>         https://gitlab.com/kraxel/qemu/-/pipelines/183769485
>
> Any chance this is an old build tree?  Specifically created before
> commit ddcf607fa3d6 ("meson: drop keymaps symlink") was merged, and the
> symlink is present still?

Yeah, mostly my build trees don't get regenerated. We like
incremental builds to continue to work.

thanks
-- PMM

Re: [PULL 0/5] Ui 20200831 patches

[Gerd Hoffmann]

On Tue, Sep 01, 2020 at 03:54:29PM +0100, Peter Maydell wrote:
> On Tue, 1 Sep 2020 at 15:16, Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> >   Hi,
> >
> > > Generating ar with a custom command
> > > cp: '../../pc-bios/keymaps/ar' and 'pc-bios/keymaps/ar' are the same file
> >
> > Hmm, strange.  No failures in gitlab ci:
> >         https://gitlab.com/kraxel/qemu/-/pipelines/183769485
> >
> > Any chance this is an old build tree?  Specifically created before
> > commit ddcf607fa3d6 ("meson: drop keymaps symlink") was merged, and the
> > symlink is present still?
> 
> Yeah, mostly my build trees don't get regenerated. We like
> incremental builds to continue to work.

So just not creating the symlink isn't enough, i.e. configure must
remove it if present?

Or is requiring fresh build trees ok if announced in the
cover letter / pull request?

thanks,
  Gerd