[PULL 0/4] Ui 20191018 patches

[PULL 0/4] Ui 20191018 patches

[Gerd Hoffmann]

The following changes since commit f22f553efffd083ff624be116726f843a39f1148:

  Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20191013' into staging (2019-10-17 16:48:56 +0100)

are available in the Git repository at:

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

for you to fetch changes up to 707f75070a94c28889f887deef0ab4da09e25ddf:

  ui: fix keymap file search in input-barrier object (2019-10-18 10:40:46 +0200)

----------------------------------------------------------------
ui: bugfixes for cocoa, curses and input-barrier.

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

Hikaru Nishida (1):
  ui: Fix hanging up Cocoa display on macOS 10.15 (Catalina)

Laurent Vivier (1):
  ui: fix keymap file search in input-barrier object

Matthew Kilgore (2):
  curses: use the bit mask constants provided by curses
  curses: correctly pass the color pair to setcchar()

 ui/curses.c        |  8 +++++---
 ui/input-barrier.c | 14 +++++++-------
 ui/cocoa.m         | 12 ++++++++++++
 3 files changed, 24 insertions(+), 10 deletions(-)

-- 
2.18.1

[PULL 1/4] ui: Fix hanging up Cocoa display on macOS 10.15 (Catalina)

[Gerd Hoffmann]

From: Hikaru Nishida <hikarupsp@gmail.com>

macOS API documentation says that before applicationDidFinishLaunching
is called, any events will not be processed. However, some events are
fired before it is called in macOS Catalina. This causes deadlock of
iothread_lock in handleEvent while it will be released after the
app_started_sem is posted.
This patch avoids processing events before the app_started_sem is
posted to prevent this deadlock.

Buglink: https://bugs.launchpad.net/qemu/+bug/1847906
Signed-off-by: Hikaru Nishida <hikarupsp@gmail.com>
Message-id: 20191015010734.85229-1-hikarupsp@gmail.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/cocoa.m | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index f12e21df6e10..fbb5b1b45f81 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -134,6 +134,7 @@ NSArray * supportedImageFileTypes;
 
 static QemuSemaphore display_init_sem;
 static QemuSemaphore app_started_sem;
+static bool allow_events;
 
 // Utility functions to run specified code block with iothread lock held
 typedef void (^CodeBlock)(void);
@@ -729,6 +730,16 @@ QemuCocoaView *cocoaView;
 
 - (bool) handleEvent:(NSEvent *)event
 {
+    if(!allow_events) {
+        /*
+         * Just let OSX have all events that arrive before
+         * applicationDidFinishLaunching.
+         * This avoids a deadlock on the iothread lock, which cocoa_display_init()
+         * will not drop until after the app_started_sem is posted. (In theory
+         * there should not be any such events, but OSX Catalina now emits some.)
+         */
+        return false;
+    }
     return bool_with_iothread_lock(^{
         return [self handleEventLocked:event];
     });
@@ -1156,6 +1167,7 @@ QemuCocoaView *cocoaView;
 - (void)applicationDidFinishLaunching: (NSNotification *) note
 {
     COCOA_DEBUG("QemuCocoaAppController: applicationDidFinishLaunching\n");
+    allow_events = true;
     /* Tell cocoa_display_init to proceed */
     qemu_sem_post(&app_started_sem);
 }
-- 
2.18.1

[PULL 2/4] curses: use the bit mask constants provided by curses

[Gerd Hoffmann]

From: Matthew Kilgore <mattkilgore12@gmail.com>

The curses API provides the A_ATTRIBUTES and A_CHARTEXT bit masks for
getting the attributes and character parts of a chtype, respectively. We
should use provided constants instead of using 0xff.

Signed-off-by: Matthew Kilgore <mattkilgore12@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Tested-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Message-id: 20191004035338.25601-2-mattkilgore12@gmail.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/curses.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ui/curses.c b/ui/curses.c
index ec281125acbd..84003f56a323 100644
--- a/ui/curses.c
+++ b/ui/curses.c
@@ -75,8 +75,8 @@ static void curses_update(DisplayChangeListener *dcl,
     line = screen + y * width;
     for (h += y; y < h; y ++, line += width) {
         for (x = 0; x < width; x++) {
-            chtype ch = line[x] & 0xff;
-            chtype at = line[x] & ~0xff;
+            chtype ch = line[x] & A_CHARTEXT;
+            chtype at = line[x] & A_ATTRIBUTES;
             ret = getcchar(&vga_to_curses[ch], wch, &attrs, &colors, NULL);
             if (ret == ERR || wch[0] == 0) {
                 wch[0] = ch;
-- 
2.18.1

[PULL 3/4] curses: correctly pass the color pair to setcchar()

[Gerd Hoffmann]

From: Matthew Kilgore <mattkilgore12@gmail.com>

The current code does not correctly pass the color pair information to
setcchar(), it instead always passes zero. This results in the curses
output always being in white on black.

This patch fixes this by using PAIR_NUMBER() to retrieve the color pair
number from the chtype value, and then passes that value as an argument
to setcchar().

Signed-off-by: Matthew Kilgore <mattkilgore12@gmail.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Tested-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Message-id: 20191004035338.25601-3-mattkilgore12@gmail.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/curses.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/ui/curses.c b/ui/curses.c
index 84003f56a323..3a1b71451c93 100644
--- a/ui/curses.c
+++ b/ui/curses.c
@@ -77,12 +77,14 @@ static void curses_update(DisplayChangeListener *dcl,
         for (x = 0; x < width; x++) {
             chtype ch = line[x] & A_CHARTEXT;
             chtype at = line[x] & A_ATTRIBUTES;
+            short color_pair = PAIR_NUMBER(line[x]);
+
             ret = getcchar(&vga_to_curses[ch], wch, &attrs, &colors, NULL);
             if (ret == ERR || wch[0] == 0) {
                 wch[0] = ch;
                 wch[1] = 0;
             }
-            setcchar(&curses_line[x], wch, at, 0, NULL);
+            setcchar(&curses_line[x], wch, at, color_pair, NULL);
         }
         mvwadd_wchnstr(screenpad, y, 0, curses_line, width);
     }
-- 
2.18.1

[PULL 4/4] ui: fix keymap file search in input-barrier object

[Gerd Hoffmann]

From: Laurent Vivier <laurent@vivier.eu>

If we try to start QEMU with "-k en-us", qemu prints a message and exits
with:

    qemu-system-i386: could not read keymap file: 'en-us'

It's because this function is called way too early, before
qemu_add_data_dir() is called, and so qemu_find_file() fails.

To fix that, move init_keyboard_layout() from the class init function to the
instance init function.

Reported-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Message-id: 20190923220658.27007-1-laurent@vivier.eu
Fixes: 6105683da35b ("ui: add an embedded Barrier client")
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/input-barrier.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/ui/input-barrier.c b/ui/input-barrier.c
index a2c961f285a4..fe35049b83a2 100644
--- a/ui/input-barrier.c
+++ b/ui/input-barrier.c
@@ -682,6 +682,13 @@ static void input_barrier_instance_init(Object *obj)
 {
     InputBarrier *ib = INPUT_BARRIER(obj);
 
+    /* always use generic keymaps */
+    if (keyboard_layout && !kbd_layout) {
+        /* We use X11 key id, so use VNC name2keysym */
+        kbd_layout = init_keyboard_layout(name2keysym, keyboard_layout,
+                                          &error_fatal);
+    }
+
     ib->saddr.type = SOCKET_ADDRESS_TYPE_INET;
     ib->saddr.u.inet.host = g_strdup("localhost");
     ib->saddr.u.inet.port = g_strdup("24800");
@@ -719,13 +726,6 @@ static void input_barrier_class_init(ObjectClass *oc, void *data)
     UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
 
     ucc->complete = input_barrier_complete;
-
-    /* always use generic keymaps */
-    if (keyboard_layout) {
-        /* We use X11 key id, so use VNC name2keysym */
-        kbd_layout = init_keyboard_layout(name2keysym, keyboard_layout,
-                                          &error_fatal);
-    }
 }
 
 static const TypeInfo input_barrier_info = {
-- 
2.18.1

Re: [PULL 0/4] Ui 20191018 patches

[Peter Maydell]

On Fri, 18 Oct 2019 at 11:17, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> The following changes since commit f22f553efffd083ff624be116726f843a39f1148:
>
>   Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20191013' into staging (2019-10-17 16:48:56 +0100)
>
> are available in the Git repository at:
>
>   git://git.kraxel.org/qemu tags/ui-20191018-pull-request
>
> for you to fetch changes up to 707f75070a94c28889f887deef0ab4da09e25ddf:
>
>   ui: fix keymap file search in input-barrier object (2019-10-18 10:40:46 +0200)
>
> ----------------------------------------------------------------
> ui: bugfixes for cocoa, curses and input-barrier.
>
> ----------------------------------------------------------------
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.2
for any user-visible changes.

-- PMM

Re: [PULL 1/4] ui: Fix hanging up Cocoa display on macOS 10.15 (Catalina)

[Peter Maydell]

Oops, just noticed that this patch should have been
Cc: qemu-stable@nongnu.org.

Hopefully the stable team can pick it up anyway.

thanks
-- PMM

On Fri, 18 Oct 2019 at 11:17, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> From: Hikaru Nishida <hikarupsp@gmail.com>
>
> macOS API documentation says that before applicationDidFinishLaunching
> is called, any events will not be processed. However, some events are
> fired before it is called in macOS Catalina. This causes deadlock of
> iothread_lock in handleEvent while it will be released after the
> app_started_sem is posted.
> This patch avoids processing events before the app_started_sem is
> posted to prevent this deadlock.
>
> Buglink: https://bugs.launchpad.net/qemu/+bug/1847906
> Signed-off-by: Hikaru Nishida <hikarupsp@gmail.com>
> Message-id: 20191015010734.85229-1-hikarupsp@gmail.com
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  ui/cocoa.m | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index f12e21df6e10..fbb5b1b45f81 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -134,6 +134,7 @@ NSArray * supportedImageFileTypes;
>
>  static QemuSemaphore display_init_sem;
>  static QemuSemaphore app_started_sem;
> +static bool allow_events;
>
>  // Utility functions to run specified code block with iothread lock held
>  typedef void (^CodeBlock)(void);
> @@ -729,6 +730,16 @@ QemuCocoaView *cocoaView;
>
>  - (bool) handleEvent:(NSEvent *)event
>  {
> +    if(!allow_events) {
> +        /*
> +         * Just let OSX have all events that arrive before
> +         * applicationDidFinishLaunching.
> +         * This avoids a deadlock on the iothread lock, which cocoa_display_init()
> +         * will not drop until after the app_started_sem is posted. (In theory
> +         * there should not be any such events, but OSX Catalina now emits some.)
> +         */
> +        return false;
> +    }
>      return bool_with_iothread_lock(^{
>          return [self handleEventLocked:event];
>      });
> @@ -1156,6 +1167,7 @@ QemuCocoaView *cocoaView;
>  - (void)applicationDidFinishLaunching: (NSNotification *) note
>  {
>      COCOA_DEBUG("QemuCocoaAppController: applicationDidFinishLaunching\n");
> +    allow_events = true;
>      /* Tell cocoa_display_init to proceed */
>      qemu_sem_post(&app_started_sem);
>  }
> --
> 2.18.1

Re: [PULL 1/4] ui: Fix hanging up Cocoa display on macOS 10.15 (Catalina)

[Christian Schoenebeck]

On Freitag, 18. Oktober 2019 12:17:08 CEST Gerd Hoffmann wrote:
> From: Hikaru Nishida <hikarupsp@gmail.com>
> 
> macOS API documentation says that before applicationDidFinishLaunching
> is called, any events will not be processed. However, some events are
> fired before it is called in macOS Catalina.

Even though fixed now on qemu side, I filed a bug report with Apple to let 
them know, since this indeed looks like unintended behaviour change a.k.a. bug 
in Catalina.

Best regards,
Christian Schoenebeck

Re: [PULL 1/4] ui: Fix hanging up Cocoa display on macOS 10.15 (Catalina)

[Peter Maydell]

On Fri, 18 Oct 2019 at 14:01, Christian Schoenebeck
<qemu_oss@crudebyte.com> wrote:
>
> On Freitag, 18. Oktober 2019 12:17:08 CEST Gerd Hoffmann wrote:
> > From: Hikaru Nishida <hikarupsp@gmail.com>
> >
> > macOS API documentation says that before applicationDidFinishLaunching
> > is called, any events will not be processed. However, some events are
> > fired before it is called in macOS Catalina.
>
> Even though fixed now on qemu side, I filed a bug report with Apple to let
> them know, since this indeed looks like unintended behaviour change a.k.a. bug
> in Catalina.

Thanks -- do you have an apple bug number or something for that?

-- PMM

Re: [PULL 1/4] ui: Fix hanging up Cocoa display on macOS 10.15 (Catalina)

[Christian Schoenebeck]

On Freitag, 18. Oktober 2019 15:02:49 CEST Peter Maydell wrote:
> On Fri, 18 Oct 2019 at 14:01, Christian Schoenebeck
> 
> <qemu_oss@crudebyte.com> wrote:
> > On Freitag, 18. Oktober 2019 12:17:08 CEST Gerd Hoffmann wrote:
> > > From: Hikaru Nishida <hikarupsp@gmail.com>
> > > 
> > > macOS API documentation says that before applicationDidFinishLaunching
> > > is called, any events will not be processed. However, some events are
> > > fired before it is called in macOS Catalina.
> > 
> > Even though fixed now on qemu side, I filed a bug report with Apple to let
> > them know, since this indeed looks like unintended behaviour change a.k.a.
> > bug in Catalina.
> 
> Thanks -- do you have an apple bug number or something for that?

It's filed as FB7380815 (on October 15th).

As of May this year they changed the way how externals may report bugs. You 
now have to use "Feedback Assistant" (requires a developer account):

	https://developer.apple.com/bug-reporting/

Before that change, registered developers could use bugreport.apple.com for 
many years, which was more tightly coupled with their actual internal bug 
tracker called "Radar" (e.g. you saw their internal rdar number, internal bug 
status, duplicates) than this new Feedback Assistant.

Best regards,
Christian Schoenebeck