[Qemu-devel] [PULL 0/7] cocoa queue

[Qemu-devel] [PULL 0/7] cocoa queue

[Peter Maydell]

Cocoa queue: this just has the refactoring to ensure we
can run reliably on OSX Mojave. Thanks to everybody who helped
in reviewing and testing this.

-- PMM
 
The following changes since commit 88687719c8859bf98ed94ce91612608c99c51c1a:

  Merge remote-tracking branch 'remotes/stefanberger/tags/pull-tpm-2019-02-25-1' into staging (2019-03-04 15:56:08 +0000)

are available in the Git repository at:

  https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-cocoa-20190304

for you to fetch changes up to 5588840ff77800e839d8a34b7be10e5c3acd1721:

  ui/cocoa: Perform UI operations only on the main thread (2019-03-04 16:47:31 +0000)

----------------------------------------------------------------
cocoa tree:
 * Support OSX Mojave by ensuring that we always make Cocoa UI
   function calls from the main thread, never from any other QEMU
   thread. This was previously mostly harmless, but on Mojave
   it will cause OSX to terminate the QEMU process.

----------------------------------------------------------------
Peter Maydell (7):
      ui/cocoa: Ensure we have the iothread lock when calling into QEMU
      ui/cocoa: Use the pixman image directly in switchSurface
      ui/cocoa: Factor out initial menu creation
      ui/cocoa: Move console/device menu creation code up in file
      ui/cocoa: Don't call NSApp sendEvent directly from handleEvent
      ui/cocoa: Subclass NSApplication so we can implement sendEvent
      ui/cocoa: Perform UI operations only on the main thread

 ui/cocoa.m | 495 +++++++++++++++++++++++++++++++++++++------------------------
 1 file changed, 303 insertions(+), 192 deletions(-)

[Qemu-devel] [PULL 1/7] ui/cocoa: Ensure we have the iothread lock when calling into QEMU

[Peter Maydell]

The Cocoa UI should run on the main thread; this is enforced
in OSX Mojave. In order to be able to run on the main thread,
we need to make sure we hold the iothread lock whenever we
call into various QEMU UI midlayer functions.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>
Tested-by: Roman Bolshakov <r.bolshakov@yadro.com>
Message-id: 20190225102433.22401-2-peter.maydell@linaro.org
Message-id: 20190214102816.3393-2-peter.maydell@linaro.org
---
 ui/cocoa.m | 91 ++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 65 insertions(+), 26 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index e2567d69466..f1171c48654 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -129,6 +129,21 @@ bool stretch_video;
 NSTextField *pauseLabel;
 NSArray * supportedImageFileTypes;
 
+// Utility function to run specified code block with iothread lock held
+typedef void (^CodeBlock)(void);
+
+static void with_iothread_lock(CodeBlock block)
+{
+    bool locked = qemu_mutex_iothread_locked();
+    if (!locked) {
+        qemu_mutex_lock_iothread();
+    }
+    block();
+    if (!locked) {
+        qemu_mutex_unlock_iothread();
+    }
+}
+
 // Mac to QKeyCode conversion
 const int mac_to_qkeycode_map[] = {
     [kVK_ANSI_A] = Q_KEY_CODE_A,
@@ -306,6 +321,7 @@ static void handleAnyDeviceErrors(Error * err)
 - (void) toggleFullScreen:(id)sender;
 - (void) handleMonitorInput:(NSEvent *)event;
 - (void) handleEvent:(NSEvent *)event;
+- (void) handleEventLocked:(NSEvent *)event;
 - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled;
 /* The state surrounding mouse grabbing is potentially confusing.
  * isAbsoluteEnabled tracks qemu_input_is_absolute() [ie "is the emulated
@@ -649,8 +665,14 @@ QemuCocoaView *cocoaView;
 
 - (void) handleEvent:(NSEvent *)event
 {
-    COCOA_DEBUG("QemuCocoaView: handleEvent\n");
+    with_iothread_lock(^{
+        [self handleEventLocked:event];
+    });
+}
 
+- (void) handleEventLocked:(NSEvent *)event
+{
+    COCOA_DEBUG("QemuCocoaView: handleEvent\n");
     int buttons = 0;
     int keycode = 0;
     bool mouse_event = false;
@@ -945,15 +967,18 @@ QemuCocoaView *cocoaView;
  */
 - (void) raiseAllKeys
 {
-    int index;
     const int max_index = ARRAY_SIZE(modifiers_state);
 
-   for (index = 0; index < max_index; index++) {
-       if (modifiers_state[index]) {
-           modifiers_state[index] = 0;
-           qemu_input_event_send_key_qcode(dcl->con, index, false);
-       }
-   }
+    with_iothread_lock(^{
+        int index;
+
+        for (index = 0; index < max_index; index++) {
+            if (modifiers_state[index]) {
+                modifiers_state[index] = 0;
+                qemu_input_event_send_key_qcode(dcl->con, index, false);
+            }
+        }
+    });
 }
 @end
 
@@ -1178,7 +1203,9 @@ QemuCocoaView *cocoaView;
 /* Pause the guest */
 - (void)pauseQEMU:(id)sender
 {
-    qmp_stop(NULL);
+    with_iothread_lock(^{
+        qmp_stop(NULL);
+    });
     [sender setEnabled: NO];
     [[[sender menu] itemWithTitle: @"Resume"] setEnabled: YES];
     [self displayPause];
@@ -1187,7 +1214,9 @@ QemuCocoaView *cocoaView;
 /* Resume running the guest operating system */
 - (void)resumeQEMU:(id) sender
 {
-    qmp_cont(NULL);
+    with_iothread_lock(^{
+        qmp_cont(NULL);
+    });
     [sender setEnabled: NO];
     [[[sender menu] itemWithTitle: @"Pause"] setEnabled: YES];
     [self removePause];
@@ -1215,13 +1244,17 @@ QemuCocoaView *cocoaView;
 /* Restarts QEMU */
 - (void)restartQEMU:(id)sender
 {
-    qmp_system_reset(NULL);
+    with_iothread_lock(^{
+        qmp_system_reset(NULL);
+    });
 }
 
 /* Powers down QEMU */
 - (void)powerDownQEMU:(id)sender
 {
-    qmp_system_powerdown(NULL);
+    with_iothread_lock(^{
+        qmp_system_powerdown(NULL);
+    });
 }
 
 /* Ejects the media.
@@ -1237,9 +1270,11 @@ QemuCocoaView *cocoaView;
         return;
     }
 
-    Error *err = NULL;
-    qmp_eject(true, [drive cStringUsingEncoding: NSASCIIStringEncoding],
-              false, NULL, false, false, &err);
+    __block Error *err = NULL;
+    with_iothread_lock(^{
+        qmp_eject(true, [drive cStringUsingEncoding: NSASCIIStringEncoding],
+                  false, NULL, false, false, &err);
+    });
     handleAnyDeviceErrors(err);
 }
 
@@ -1271,16 +1306,18 @@ QemuCocoaView *cocoaView;
             return;
         }
 
-        Error *err = NULL;
-        qmp_blockdev_change_medium(true,
-                                   [drive cStringUsingEncoding:
-                                          NSASCIIStringEncoding],
-                                   false, NULL,
-                                   [file cStringUsingEncoding:
-                                         NSASCIIStringEncoding],
-                                   true, "raw",
-                                   false, 0,
-                                   &err);
+        __block Error *err = NULL;
+        with_iothread_lock(^{
+            qmp_blockdev_change_medium(true,
+                                       [drive cStringUsingEncoding:
+                                                  NSASCIIStringEncoding],
+                                       false, NULL,
+                                       [file cStringUsingEncoding:
+                                                 NSASCIIStringEncoding],
+                                       true, "raw",
+                                       false, 0,
+                                       &err);
+        });
         handleAnyDeviceErrors(err);
     }
 }
@@ -1419,7 +1456,9 @@ QemuCocoaView *cocoaView;
     // get the throttle percentage
     throttle_pct = [sender tag];
 
-    cpu_throttle_set(throttle_pct);
+    with_iothread_lock(^{
+        cpu_throttle_set(throttle_pct);
+    });
     COCOA_DEBUG("cpu throttling at %d%c\n", cpu_throttle_get_percentage(), '%');
 }
 
-- 
2.20.1

[Qemu-devel] [PULL 2/7] ui/cocoa: Use the pixman image directly in switchSurface

[Peter Maydell]

Currently the switchSurface method takes a DisplaySurface. We want
to change our DisplayChangeListener's dpy_gfx_switch callback
to do this work asynchronously on a different thread. The caller
of the switch callback will free the old DisplaySurface
immediately the callback returns, so to ensure that the
other thread doesn't access freed data we need to switch
to using the underlying pixman image instead. The pixman
image is reference counted, so we will be able to take
a reference to it to avoid it vanishing too early.

In this commit we only change the switchSurface method
to take a pixman image, and keep the flow of control
synchronous for now.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>
Tested-by: Roman Bolshakov <r.bolshakov@yadro.com>
Message-id: 20190225102433.22401-3-peter.maydell@linaro.org
Message-id: 20190214102816.3393-3-peter.maydell@linaro.org
---
 ui/cocoa.m | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index f1171c48654..a913a51a2d8 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -315,7 +315,7 @@ static void handleAnyDeviceErrors(Error * err)
     BOOL isAbsoluteEnabled;
     BOOL isMouseDeassociated;
 }
-- (void) switchSurface:(DisplaySurface *)surface;
+- (void) switchSurface:(pixman_image_t *)image;
 - (void) grabMouse;
 - (void) ungrabMouse;
 - (void) toggleFullScreen:(id)sender;
@@ -495,12 +495,13 @@ QemuCocoaView *cocoaView;
     }
 }
 
-- (void) switchSurface:(DisplaySurface *)surface
+- (void) switchSurface:(pixman_image_t *)image
 {
     COCOA_DEBUG("QemuCocoaView: switchSurface\n");
 
-    int w = surface_width(surface);
-    int h = surface_height(surface);
+    int w = pixman_image_get_width(image);
+    int h = pixman_image_get_height(image);
+    pixman_format_code_t image_format = pixman_image_get_format(image);
     /* cdx == 0 means this is our very first surface, in which case we need
      * to recalculate the content dimensions even if it happens to be the size
      * of the initial empty window.
@@ -522,10 +523,10 @@ QemuCocoaView *cocoaView;
         CGDataProviderRelease(dataProviderRef);
 
     //sync host window color space with guests
-    screen.bitsPerPixel = surface_bits_per_pixel(surface);
-    screen.bitsPerComponent = surface_bytes_per_pixel(surface) * 2;
+    screen.bitsPerPixel = PIXMAN_FORMAT_BPP(image_format);
+    screen.bitsPerComponent = DIV_ROUND_UP(screen.bitsPerPixel, 8) * 2;
 
-    dataProviderRef = CGDataProviderCreateWithData(NULL, surface_data(surface), w * 4 * h, NULL);
+    dataProviderRef = CGDataProviderCreateWithData(NULL, pixman_image_get_data(image), w * 4 * h, NULL);
 
     // update windows
     if (isFullscreen) {
@@ -1629,7 +1630,7 @@ static void cocoa_switch(DisplayChangeListener *dcl,
     NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
 
     COCOA_DEBUG("qemu_cocoa: cocoa_switch\n");
-    [cocoaView switchSurface:surface];
+    [cocoaView switchSurface:surface->image];
     [pool release];
 }
 
-- 
2.20.1

[Qemu-devel] [PULL 3/7] ui/cocoa: Factor out initial menu creation

[Peter Maydell]

Factor out the long code sequence in main() which creates
the initial set of menus. This will make later patches
which move initialization code around a bit clearer.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>
Tested-by: Roman Bolshakov <r.bolshakov@yadro.com>
Message-id: 20190225102433.22401-4-peter.maydell@linaro.org
Message-id: 20190214102816.3393-4-peter.maydell@linaro.org
---
 ui/cocoa.m | 78 ++++++++++++++++++++++++++++--------------------------
 1 file changed, 41 insertions(+), 37 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index a913a51a2d8..4baec0b2ff7 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -1465,43 +1465,8 @@ QemuCocoaView *cocoaView;
 
 @end
 
-
-int main (int argc, const char * argv[]) {
-
-    gArgc = argc;
-    gArgv = (char **)argv;
-    int i;
-
-    /* In case we don't need to display a window, let's not do that */
-    for (i = 1; i < argc; i++) {
-        const char *opt = argv[i];
-
-        if (opt[0] == '-') {
-            /* Treat --foo the same as -foo.  */
-            if (opt[1] == '-') {
-                opt++;
-            }
-            if (!strcmp(opt, "-h") || !strcmp(opt, "-help") ||
-                !strcmp(opt, "-vnc") ||
-                !strcmp(opt, "-nographic") ||
-                !strcmp(opt, "-version") ||
-                !strcmp(opt, "-curses") ||
-                !strcmp(opt, "-display") ||
-                !strcmp(opt, "-qtest")) {
-                return qemu_main(gArgc, gArgv, *_NSGetEnviron());
-            }
-        }
-    }
-
-    NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
-
-    // Pull this console process up to being a fully-fledged graphical
-    // app with a menubar and Dock icon
-    ProcessSerialNumber psn = { 0, kCurrentProcess };
-    TransformProcessType(&psn, kProcessTransformToForegroundApplication);
-
-    [NSApplication sharedApplication];
-
+static void create_initial_menus(void)
+{
     // Add menus
     NSMenu      *menu;
     NSMenuItem  *menuItem;
@@ -1585,6 +1550,45 @@ int main (int argc, const char * argv[]) {
     menuItem = [[[NSMenuItem alloc] initWithTitle:@"Window" action:nil keyEquivalent:@""] autorelease];
     [menuItem setSubmenu:menu];
     [[NSApp mainMenu] addItem:menuItem];
+}
+
+int main (int argc, const char * argv[]) {
+
+    gArgc = argc;
+    gArgv = (char **)argv;
+    int i;
+
+    /* In case we don't need to display a window, let's not do that */
+    for (i = 1; i < argc; i++) {
+        const char *opt = argv[i];
+
+        if (opt[0] == '-') {
+            /* Treat --foo the same as -foo.  */
+            if (opt[1] == '-') {
+                opt++;
+            }
+            if (!strcmp(opt, "-h") || !strcmp(opt, "-help") ||
+                !strcmp(opt, "-vnc") ||
+                !strcmp(opt, "-nographic") ||
+                !strcmp(opt, "-version") ||
+                !strcmp(opt, "-curses") ||
+                !strcmp(opt, "-display") ||
+                !strcmp(opt, "-qtest")) {
+                return qemu_main(gArgc, gArgv, *_NSGetEnviron());
+            }
+        }
+    }
+
+    NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
+
+    // Pull this console process up to being a fully-fledged graphical
+    // app with a menubar and Dock icon
+    ProcessSerialNumber psn = { 0, kCurrentProcess };
+    TransformProcessType(&psn, kProcessTransformToForegroundApplication);
+
+    [NSApplication sharedApplication];
+
+    create_initial_menus();
 
     // Create an Application controller
     QemuCocoaAppController *appController = [[QemuCocoaAppController alloc] init];
-- 
2.20.1

[Qemu-devel] [PULL 4/7] ui/cocoa: Move console/device menu creation code up in file

[Peter Maydell]

Move the console/device menu creation code functions
further up in the source file, next to the code which
creates the initial menus. We're going to want to
change the location we call these functions from in
the next patch.

This commit is a pure code move with no other changes.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>
Tested-by: Roman Bolshakov <r.bolshakov@yadro.com>
Message-id: 20190225102433.22401-5-peter.maydell@linaro.org
Message-id: 20190214102816.3393-5-peter.maydell@linaro.org
---
 ui/cocoa.m | 184 ++++++++++++++++++++++++++---------------------------
 1 file changed, 92 insertions(+), 92 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 4baec0b2ff7..d1fc1a6aff4 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -1552,6 +1552,98 @@ static void create_initial_menus(void)
     [[NSApp mainMenu] addItem:menuItem];
 }
 
+/* Returns a name for a given console */
+static NSString * getConsoleName(QemuConsole * console)
+{
+    return [NSString stringWithFormat: @"%s", qemu_console_get_label(console)];
+}
+
+/* Add an entry to the View menu for each console */
+static void add_console_menu_entries(void)
+{
+    NSMenu *menu;
+    NSMenuItem *menuItem;
+    int index = 0;
+
+    menu = [[[NSApp mainMenu] itemWithTitle:@"View"] submenu];
+
+    [menu addItem:[NSMenuItem separatorItem]];
+
+    while (qemu_console_lookup_by_index(index) != NULL) {
+        menuItem = [[[NSMenuItem alloc] initWithTitle: getConsoleName(qemu_console_lookup_by_index(index))
+                                               action: @selector(displayConsole:) keyEquivalent: @""] autorelease];
+        [menuItem setTag: index];
+        [menu addItem: menuItem];
+        index++;
+    }
+}
+
+/* Make menu items for all removable devices.
+ * Each device is given an 'Eject' and 'Change' menu item.
+ */
+static void addRemovableDevicesMenuItems(void)
+{
+    NSMenu *menu;
+    NSMenuItem *menuItem;
+    BlockInfoList *currentDevice, *pointerToFree;
+    NSString *deviceName;
+
+    currentDevice = qmp_query_block(NULL);
+    pointerToFree = currentDevice;
+    if(currentDevice == NULL) {
+        NSBeep();
+        QEMU_Alert(@"Failed to query for block devices!");
+        return;
+    }
+
+    menu = [[[NSApp mainMenu] itemWithTitle:@"Machine"] submenu];
+
+    // Add a separator between related groups of menu items
+    [menu addItem:[NSMenuItem separatorItem]];
+
+    // Set the attributes to the "Removable Media" menu item
+    NSString *titleString = @"Removable Media";
+    NSMutableAttributedString *attString=[[NSMutableAttributedString alloc] initWithString:titleString];
+    NSColor *newColor = [NSColor blackColor];
+    NSFontManager *fontManager = [NSFontManager sharedFontManager];
+    NSFont *font = [fontManager fontWithFamily:@"Helvetica"
+                                          traits:NSBoldFontMask|NSItalicFontMask
+                                          weight:0
+                                            size:14];
+    [attString addAttribute:NSFontAttributeName value:font range:NSMakeRange(0, [titleString length])];
+    [attString addAttribute:NSForegroundColorAttributeName value:newColor range:NSMakeRange(0, [titleString length])];
+    [attString addAttribute:NSUnderlineStyleAttributeName value:[NSNumber numberWithInt: 1] range:NSMakeRange(0, [titleString length])];
+
+    // Add the "Removable Media" menu item
+    menuItem = [NSMenuItem new];
+    [menuItem setAttributedTitle: attString];
+    [menuItem setEnabled: NO];
+    [menu addItem: menuItem];
+
+    /* Loop through all the block devices in the emulator */
+    while (currentDevice) {
+        deviceName = [[NSString stringWithFormat: @"%s", currentDevice->value->device] retain];
+
+        if(currentDevice->value->removable) {
+            menuItem = [[NSMenuItem alloc] initWithTitle: [NSString stringWithFormat: @"Change %s...", currentDevice->value->device]
+                                                  action: @selector(changeDeviceMedia:)
+                                           keyEquivalent: @""];
+            [menu addItem: menuItem];
+            [menuItem setRepresentedObject: deviceName];
+            [menuItem autorelease];
+
+            menuItem = [[NSMenuItem alloc] initWithTitle: [NSString stringWithFormat: @"Eject %s", currentDevice->value->device]
+                                                  action: @selector(ejectDeviceMedia:)
+                                           keyEquivalent: @""];
+            [menu addItem: menuItem];
+            [menuItem setRepresentedObject: deviceName];
+            [menuItem autorelease];
+        }
+        currentDevice = currentDevice->next;
+    }
+    qapi_free_BlockInfoList(pointerToFree);
+}
+
 int main (int argc, const char * argv[]) {
 
     gArgc = argc;
@@ -1680,98 +1772,6 @@ static const DisplayChangeListenerOps dcl_ops = {
     .dpy_refresh = cocoa_refresh,
 };
 
-/* Returns a name for a given console */
-static NSString * getConsoleName(QemuConsole * console)
-{
-    return [NSString stringWithFormat: @"%s", qemu_console_get_label(console)];
-}
-
-/* Add an entry to the View menu for each console */
-static void add_console_menu_entries(void)
-{
-    NSMenu *menu;
-    NSMenuItem *menuItem;
-    int index = 0;
-
-    menu = [[[NSApp mainMenu] itemWithTitle:@"View"] submenu];
-
-    [menu addItem:[NSMenuItem separatorItem]];
-
-    while (qemu_console_lookup_by_index(index) != NULL) {
-        menuItem = [[[NSMenuItem alloc] initWithTitle: getConsoleName(qemu_console_lookup_by_index(index))
-                                               action: @selector(displayConsole:) keyEquivalent: @""] autorelease];
-        [menuItem setTag: index];
-        [menu addItem: menuItem];
-        index++;
-    }
-}
-
-/* Make menu items for all removable devices.
- * Each device is given an 'Eject' and 'Change' menu item.
- */
-static void addRemovableDevicesMenuItems(void)
-{
-    NSMenu *menu;
-    NSMenuItem *menuItem;
-    BlockInfoList *currentDevice, *pointerToFree;
-    NSString *deviceName;
-
-    currentDevice = qmp_query_block(NULL);
-    pointerToFree = currentDevice;
-    if(currentDevice == NULL) {
-        NSBeep();
-        QEMU_Alert(@"Failed to query for block devices!");
-        return;
-    }
-
-    menu = [[[NSApp mainMenu] itemWithTitle:@"Machine"] submenu];
-
-    // Add a separator between related groups of menu items
-    [menu addItem:[NSMenuItem separatorItem]];
-
-    // Set the attributes to the "Removable Media" menu item
-    NSString *titleString = @"Removable Media";
-    NSMutableAttributedString *attString=[[NSMutableAttributedString alloc] initWithString:titleString];
-    NSColor *newColor = [NSColor blackColor];
-    NSFontManager *fontManager = [NSFontManager sharedFontManager];
-    NSFont *font = [fontManager fontWithFamily:@"Helvetica"
-                                          traits:NSBoldFontMask|NSItalicFontMask
-                                          weight:0
-                                            size:14];
-    [attString addAttribute:NSFontAttributeName value:font range:NSMakeRange(0, [titleString length])];
-    [attString addAttribute:NSForegroundColorAttributeName value:newColor range:NSMakeRange(0, [titleString length])];
-    [attString addAttribute:NSUnderlineStyleAttributeName value:[NSNumber numberWithInt: 1] range:NSMakeRange(0, [titleString length])];
-
-    // Add the "Removable Media" menu item
-    menuItem = [NSMenuItem new];
-    [menuItem setAttributedTitle: attString];
-    [menuItem setEnabled: NO];
-    [menu addItem: menuItem];
-
-    /* Loop through all the block devices in the emulator */
-    while (currentDevice) {
-        deviceName = [[NSString stringWithFormat: @"%s", currentDevice->value->device] retain];
-
-        if(currentDevice->value->removable) {
-            menuItem = [[NSMenuItem alloc] initWithTitle: [NSString stringWithFormat: @"Change %s...", currentDevice->value->device]
-                                                  action: @selector(changeDeviceMedia:)
-                                           keyEquivalent: @""];
-            [menu addItem: menuItem];
-            [menuItem setRepresentedObject: deviceName];
-            [menuItem autorelease];
-
-            menuItem = [[NSMenuItem alloc] initWithTitle: [NSString stringWithFormat: @"Eject %s", currentDevice->value->device]
-                                                  action: @selector(ejectDeviceMedia:)
-                                           keyEquivalent: @""];
-            [menu addItem: menuItem];
-            [menuItem setRepresentedObject: deviceName];
-            [menuItem autorelease];
-        }
-        currentDevice = currentDevice->next;
-    }
-    qapi_free_BlockInfoList(pointerToFree);
-}
-
 static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
 {
     COCOA_DEBUG("qemu_cocoa: cocoa_display_init\n");
-- 
2.20.1

[Qemu-devel] [PULL 5/7] ui/cocoa: Don't call NSApp sendEvent directly from handleEvent

[Peter Maydell]

Currently the handleEvent method will directly call the NSApp
sendEvent method for any events that we want to let OSX deal
with. When we rearrange the event handling code, the way that
we say "let OSX have this event" is going to change. Prepare
for that by refactoring so that handleEvent returns a flag
indicating whether it consumed the event.

Suggested-by: BALATON Zoltan <balaton@eik.bme.hu>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>
Tested-by: Roman Bolshakov <r.bolshakov@yadro.com>
Message-id: 20190225102433.22401-6-peter.maydell@linaro.org
Message-id: 20190214102816.3393-6-peter.maydell@linaro.org
---
 ui/cocoa.m | 49 ++++++++++++++++++++++++++++++++++---------------
 1 file changed, 34 insertions(+), 15 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index d1fc1a6aff4..1b54d42aba6 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -129,8 +129,9 @@ bool stretch_video;
 NSTextField *pauseLabel;
 NSArray * supportedImageFileTypes;
 
-// Utility function to run specified code block with iothread lock held
+// Utility functions to run specified code block with iothread lock held
 typedef void (^CodeBlock)(void);
+typedef bool (^BoolCodeBlock)(void);
 
 static void with_iothread_lock(CodeBlock block)
 {
@@ -144,6 +145,21 @@ static void with_iothread_lock(CodeBlock block)
     }
 }
 
+static bool bool_with_iothread_lock(BoolCodeBlock block)
+{
+    bool locked = qemu_mutex_iothread_locked();
+    bool val;
+
+    if (!locked) {
+        qemu_mutex_lock_iothread();
+    }
+    val = block();
+    if (!locked) {
+        qemu_mutex_unlock_iothread();
+    }
+    return val;
+}
+
 // Mac to QKeyCode conversion
 const int mac_to_qkeycode_map[] = {
     [kVK_ANSI_A] = Q_KEY_CODE_A,
@@ -320,8 +336,8 @@ static void handleAnyDeviceErrors(Error * err)
 - (void) ungrabMouse;
 - (void) toggleFullScreen:(id)sender;
 - (void) handleMonitorInput:(NSEvent *)event;
-- (void) handleEvent:(NSEvent *)event;
-- (void) handleEventLocked:(NSEvent *)event;
+- (bool) handleEvent:(NSEvent *)event;
+- (bool) handleEventLocked:(NSEvent *)event;
 - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled;
 /* The state surrounding mouse grabbing is potentially confusing.
  * isAbsoluteEnabled tracks qemu_input_is_absolute() [ie "is the emulated
@@ -664,15 +680,16 @@ QemuCocoaView *cocoaView;
     }
 }
 
-- (void) handleEvent:(NSEvent *)event
+- (bool) handleEvent:(NSEvent *)event
 {
-    with_iothread_lock(^{
-        [self handleEventLocked:event];
+    return bool_with_iothread_lock(^{
+        return [self handleEventLocked:event];
     });
 }
 
-- (void) handleEventLocked:(NSEvent *)event
+- (bool) handleEventLocked:(NSEvent *)event
 {
+    /* Return true if we handled the event, false if it should be given to OSX */
     COCOA_DEBUG("QemuCocoaView: handleEvent\n");
     int buttons = 0;
     int keycode = 0;
@@ -743,8 +760,7 @@ QemuCocoaView *cocoaView;
                 if (keycode == Q_KEY_CODE_F) {
                     switched_to_fullscreen = true;
                 }
-                [NSApp sendEvent:event];
-                return;
+                return false;
             }
 
             // default
@@ -759,12 +775,12 @@ QemuCocoaView *cocoaView;
                         // enable graphic console
                         case '1' ... '9':
                             console_select(key - '0' - 1); /* ascii math */
-                            return;
+                            return true;
 
                         // release the mouse grab
                         case 'g':
                             [self ungrabMouse];
-                            return;
+                            return true;
                     }
                 }
             }
@@ -781,7 +797,7 @@ QemuCocoaView *cocoaView;
             // don't pass the guest a spurious key-up if we treated this
             // command-key combo as a host UI action
             if (!isMouseGrabbed && ([event modifierFlags] & NSEventModifierFlagCommand)) {
-                return;
+                return true;
             }
 
             if (qemu_console_is_graphic(NULL)) {
@@ -875,7 +891,7 @@ QemuCocoaView *cocoaView;
             mouse_event = false;
             break;
         default:
-            [NSApp sendEvent:event];
+            return false;
     }
 
     if (mouse_event) {
@@ -911,10 +927,11 @@ QemuCocoaView *cocoaView;
                 qemu_input_queue_rel(dcl->con, INPUT_AXIS_Y, (int)[event deltaY]);
             }
         } else {
-            [NSApp sendEvent:event];
+            return false;
         }
         qemu_input_event_sync();
     }
+    return true;
 }
 
 - (void) grabMouse
@@ -1753,7 +1770,9 @@ static void cocoa_refresh(DisplayChangeListener *dcl)
         event = [NSApp nextEventMatchingMask:NSEventMaskAny untilDate:distantPast
                         inMode: NSDefaultRunLoopMode dequeue:YES];
         if (event != nil) {
-            [cocoaView handleEvent:event];
+            if (![cocoaView handleEvent:event]) {
+                [NSApp sendEvent:event];
+            }
         }
     } while(event != nil);
     [pool release];
-- 
2.20.1

[Qemu-devel] [PULL 6/7] ui/cocoa: Subclass NSApplication so we can implement sendEvent

[Peter Maydell]

When we switch away from our custom event handling, we still want to
be able to have first go at any events our application receives,
because in full-screen mode we want to send key events to the guest,
even if they would be menu item activation events. There are several
ways we could do that, but one simple approach is to subclass
NSApplication so we can implement a custom sendEvent method.
Do that, but for the moment have our sendEvent just invoke the
superclass method.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>
Tested-by: Roman Bolshakov <r.bolshakov@yadro.com>
Message-id: 20190225102433.22401-7-peter.maydell@linaro.org
Message-id: 20190214102816.3393-7-peter.maydell@linaro.org
---
 ui/cocoa.m | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 1b54d42aba6..00e3db69c91 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -1482,6 +1482,17 @@ QemuCocoaView *cocoaView;
 
 @end
 
+@interface QemuApplication : NSApplication
+@end
+
+@implementation QemuApplication
+- (void)sendEvent:(NSEvent *)event
+{
+    COCOA_DEBUG("QemuApplication: sendEvent\n");
+    [super sendEvent: event];
+}
+@end
+
 static void create_initial_menus(void)
 {
     // Add menus
@@ -1695,7 +1706,7 @@ int main (int argc, const char * argv[]) {
     ProcessSerialNumber psn = { 0, kCurrentProcess };
     TransformProcessType(&psn, kProcessTransformToForegroundApplication);
 
-    [NSApplication sharedApplication];
+    [QemuApplication sharedApplication];
 
     create_initial_menus();
 
-- 
2.20.1

[Qemu-devel] [PULL 7/7] ui/cocoa: Perform UI operations only on the main thread

[Peter Maydell]

The OSX Mojave release is more picky about enforcing the Cocoa API
restriction that only the main thread may perform UI calls. To
accommodate this we need to restructure the Cocoa code:
 * the special OSX main() creates a second thread and uses
   that to call the vl.c qemu_main(); the original main
   thread goes into the OSX event loop
 * the refresh, switch and update callbacks asynchronously
   tell the main thread to do the necessary work
 * the refresh callback no longer does the "get events from the
   UI event queue and handle them" loop, since we now use
   the stock OSX event loop. Instead our NSApplication sendEvent
   method will either deal with them or pass them on to OSX

All these things have to be changed in one commit, to avoid
breaking bisection.

Note that since we use dispatch_get_main_queue(), this bumps
our minimum version requirement to OSX 10.10 Yosemite (released
in 2014, unsupported by Apple since 2017).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>
Tested-by: Roman Bolshakov <r.bolshakov@yadro.com>
Message-id: 20190225102433.22401-8-peter.maydell@linaro.org
Message-id: 20190214102816.3393-8-peter.maydell@linaro.org
---
 ui/cocoa.m | 195 +++++++++++++++++++++++++++++++----------------------
 1 file changed, 116 insertions(+), 79 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 00e3db69c91..420b2411c10 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -129,6 +129,9 @@ bool stretch_video;
 NSTextField *pauseLabel;
 NSArray * supportedImageFileTypes;
 
+static QemuSemaphore display_init_sem;
+static QemuSemaphore app_started_sem;
+
 // Utility functions to run specified code block with iothread lock held
 typedef void (^CodeBlock)(void);
 typedef bool (^BoolCodeBlock)(void);
@@ -325,6 +328,7 @@ static void handleAnyDeviceErrors(Error * err)
     NSWindow *fullScreenWindow;
     float cx,cy,cw,ch,cdx,cdy;
     CGDataProviderRef dataProviderRef;
+    pixman_image_t *pixman_image;
     BOOL modifiers_state[256];
     BOOL isMouseGrabbed;
     BOOL isFullscreen;
@@ -383,8 +387,10 @@ QemuCocoaView *cocoaView;
 {
     COCOA_DEBUG("QemuCocoaView: dealloc\n");
 
-    if (dataProviderRef)
+    if (dataProviderRef) {
         CGDataProviderRelease(dataProviderRef);
+        pixman_image_unref(pixman_image);
+    }
 
     [super dealloc];
 }
@@ -535,13 +541,16 @@ QemuCocoaView *cocoaView;
     }
 
     // update screenBuffer
-    if (dataProviderRef)
+    if (dataProviderRef) {
         CGDataProviderRelease(dataProviderRef);
+        pixman_image_unref(pixman_image);
+    }
 
     //sync host window color space with guests
     screen.bitsPerPixel = PIXMAN_FORMAT_BPP(image_format);
     screen.bitsPerComponent = DIV_ROUND_UP(screen.bitsPerPixel, 8) * 2;
 
+    pixman_image = image;
     dataProviderRef = CGDataProviderCreateWithData(NULL, pixman_image_get_data(image), w * 4 * h, NULL);
 
     // update windows
@@ -1013,7 +1022,6 @@ QemuCocoaView *cocoaView;
 #endif
 {
 }
-- (void)startEmulationWithArgc:(int)argc argv:(char**)argv;
 - (void)doToggleFullScreen:(id)sender;
 - (void)toggleFullScreen:(id)sender;
 - (void)showQEMUDoc:(id)sender;
@@ -1101,8 +1109,8 @@ QemuCocoaView *cocoaView;
 - (void)applicationDidFinishLaunching: (NSNotification *) note
 {
     COCOA_DEBUG("QemuCocoaAppController: applicationDidFinishLaunching\n");
-    // launch QEMU, with the global args
-    [self startEmulationWithArgc:gArgc argv:(char **)gArgv];
+    /* Tell cocoa_display_init to proceed */
+    qemu_sem_post(&app_started_sem);
 }
 
 - (void)applicationWillTerminate:(NSNotification *)aNotification
@@ -1145,15 +1153,6 @@ QemuCocoaView *cocoaView;
     [cocoaView raiseAllKeys];
 }
 
-- (void)startEmulationWithArgc:(int)argc argv:(char**)argv
-{
-    COCOA_DEBUG("QemuCocoaAppController: startEmulationWithArgc\n");
-
-    int status;
-    status = qemu_main(argc, argv, *_NSGetEnviron());
-    exit(status);
-}
-
 /* We abstract the method called by the Enter Fullscreen menu item
  * because Mac OS 10.7 and higher disables it. This is because of the
  * menu item's old selector's name toggleFullScreen:
@@ -1489,7 +1488,9 @@ QemuCocoaView *cocoaView;
 - (void)sendEvent:(NSEvent *)event
 {
     COCOA_DEBUG("QemuApplication: sendEvent\n");
-    [super sendEvent: event];
+    if (![cocoaView handleEvent:event]) {
+        [super sendEvent: event];
+    }
 }
 @end
 
@@ -1672,32 +1673,59 @@ static void addRemovableDevicesMenuItems(void)
     qapi_free_BlockInfoList(pointerToFree);
 }
 
-int main (int argc, const char * argv[]) {
+/*
+ * The startup process for the OSX/Cocoa UI is complicated, because
+ * OSX insists that the UI runs on the initial main thread, and so we
+ * need to start a second thread which runs the vl.c qemu_main():
+ *
+ * Initial thread:                    2nd thread:
+ * in main():
+ *  create qemu-main thread
+ *  wait on display_init semaphore
+ *                                    call qemu_main()
+ *                                    ...
+ *                                    in cocoa_display_init():
+ *                                     post the display_init semaphore
+ *                                     wait on app_started semaphore
+ *  create application, menus, etc
+ *  enter OSX run loop
+ * in applicationDidFinishLaunching:
+ *  post app_started semaphore
+ *                                     tell main thread to fullscreen if needed
+ *                                    [...]
+ *                                    run qemu main-loop
+ *
+ * We do this in two stages so that we don't do the creation of the
+ * GUI application menus and so on for command line options like --help
+ * where we want to just print text to stdout and exit immediately.
+ */
 
+static void *call_qemu_main(void *opaque)
+{
+    int status;
+
+    COCOA_DEBUG("Second thread: calling qemu_main()\n");
+    status = qemu_main(gArgc, gArgv, *_NSGetEnviron());
+    COCOA_DEBUG("Second thread: qemu_main() returned, exiting\n");
+    exit(status);
+}
+
+int main (int argc, const char * argv[]) {
+    QemuThread thread;
+
+    COCOA_DEBUG("Entered main()\n");
     gArgc = argc;
     gArgv = (char **)argv;
-    int i;
 
-    /* In case we don't need to display a window, let's not do that */
-    for (i = 1; i < argc; i++) {
-        const char *opt = argv[i];
+    qemu_sem_init(&display_init_sem, 0);
+    qemu_sem_init(&app_started_sem, 0);
 
-        if (opt[0] == '-') {
-            /* Treat --foo the same as -foo.  */
-            if (opt[1] == '-') {
-                opt++;
-            }
-            if (!strcmp(opt, "-h") || !strcmp(opt, "-help") ||
-                !strcmp(opt, "-vnc") ||
-                !strcmp(opt, "-nographic") ||
-                !strcmp(opt, "-version") ||
-                !strcmp(opt, "-curses") ||
-                !strcmp(opt, "-display") ||
-                !strcmp(opt, "-qtest")) {
-                return qemu_main(gArgc, gArgv, *_NSGetEnviron());
-            }
-        }
-    }
+    qemu_thread_create(&thread, "qemu_main", call_qemu_main,
+                       NULL, QEMU_THREAD_DETACHED);
+
+    COCOA_DEBUG("Main thread: waiting for display_init_sem\n");
+    qemu_sem_wait(&display_init_sem);
+    COCOA_DEBUG("Main thread: initializing app\n");
 
     NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
 
@@ -1710,12 +1738,24 @@ int main (int argc, const char * argv[]) {
 
     create_initial_menus();
 
+    /*
+     * Create the menu entries which depend on QEMU state (for consoles
+     * and removeable devices). These make calls back into QEMU functions,
+     * which is OK because at this point we know that the second thread
+     * holds the iothread lock and is synchronously waiting for us to
+     * finish.
+     */
+    add_console_menu_entries();
+    addRemovableDevicesMenuItems();
+
     // Create an Application controller
     QemuCocoaAppController *appController = [[QemuCocoaAppController alloc] init];
     [NSApp setDelegate:appController];
 
     // Start the main event loop
+    COCOA_DEBUG("Main thread: entering OSX run loop\n");
     [NSApp run];
+    COCOA_DEBUG("Main thread: left OSX run loop, exiting\n");
 
     [appController release];
     [pool release];
@@ -1733,17 +1773,19 @@ static void cocoa_update(DisplayChangeListener *dcl,
 
     COCOA_DEBUG("qemu_cocoa: cocoa_update\n");
 
-    NSRect rect;
-    if ([cocoaView cdx] == 1.0) {
-        rect = NSMakeRect(x, [cocoaView gscreen].height - y - h, w, h);
-    } else {
-        rect = NSMakeRect(
-            x * [cocoaView cdx],
-            ([cocoaView gscreen].height - y - h) * [cocoaView cdy],
-            w * [cocoaView cdx],
-            h * [cocoaView cdy]);
-    }
-    [cocoaView setNeedsDisplayInRect:rect];
+    dispatch_async(dispatch_get_main_queue(), ^{
+        NSRect rect;
+        if ([cocoaView cdx] == 1.0) {
+            rect = NSMakeRect(x, [cocoaView gscreen].height - y - h, w, h);
+        } else {
+            rect = NSMakeRect(
+                x * [cocoaView cdx],
+                ([cocoaView gscreen].height - y - h) * [cocoaView cdy],
+                w * [cocoaView cdx],
+                h * [cocoaView cdy]);
+        }
+        [cocoaView setNeedsDisplayInRect:rect];
+    });
 
     [pool release];
 }
@@ -1752,9 +1794,19 @@ static void cocoa_switch(DisplayChangeListener *dcl,
                          DisplaySurface *surface)
 {
     NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
+    pixman_image_t *image = surface->image;
 
     COCOA_DEBUG("qemu_cocoa: cocoa_switch\n");
-    [cocoaView switchSurface:surface->image];
+
+    // The DisplaySurface will be freed as soon as this callback returns.
+    // We take a reference to the underlying pixman image here so it does
+    // not disappear from under our feet; the switchSurface method will
+    // deref the old image when it is done with it.
+    pixman_image_ref(image);
+
+    dispatch_async(dispatch_get_main_queue(), ^{
+        [cocoaView switchSurface:image];
+    });
     [pool release];
 }
 
@@ -1766,26 +1818,15 @@ static void cocoa_refresh(DisplayChangeListener *dcl)
     graphic_hw_update(NULL);
 
     if (qemu_input_is_absolute()) {
-        if (![cocoaView isAbsoluteEnabled]) {
-            if ([cocoaView isMouseGrabbed]) {
-                [cocoaView ungrabMouse];
+        dispatch_async(dispatch_get_main_queue(), ^{
+            if (![cocoaView isAbsoluteEnabled]) {
+                if ([cocoaView isMouseGrabbed]) {
+                    [cocoaView ungrabMouse];
+                }
             }
-        }
-        [cocoaView setAbsoluteEnabled:YES];
+            [cocoaView setAbsoluteEnabled:YES];
+        });
     }
-
-    NSDate *distantPast;
-    NSEvent *event;
-    distantPast = [NSDate distantPast];
-    do {
-        event = [NSApp nextEventMatchingMask:NSEventMaskAny untilDate:distantPast
-                        inMode: NSDefaultRunLoopMode dequeue:YES];
-        if (event != nil) {
-            if (![cocoaView handleEvent:event]) {
-                [NSApp sendEvent:event];
-            }
-        }
-    } while(event != nil);
     [pool release];
 }
 
@@ -1806,10 +1847,17 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
 {
     COCOA_DEBUG("qemu_cocoa: cocoa_display_init\n");
 
+    /* Tell main thread to go ahead and create the app and enter the run loop */
+    qemu_sem_post(&display_init_sem);
+    qemu_sem_wait(&app_started_sem);
+    COCOA_DEBUG("cocoa_display_init: app start completed\n");
+
     /* if fullscreen mode is to be used */
     if (opts->has_full_screen && opts->full_screen) {
-        [NSApp activateIgnoringOtherApps: YES];
-        [(QemuCocoaAppController *)[[NSApplication sharedApplication] delegate] toggleFullScreen: nil];
+        dispatch_async(dispatch_get_main_queue(), ^{
+            [NSApp activateIgnoringOtherApps: YES];
+            [(QemuCocoaAppController *)[[NSApplication sharedApplication] delegate] toggleFullScreen: nil];
+        });
     }
 
     dcl = g_malloc0(sizeof(DisplayChangeListener));
@@ -1820,17 +1868,6 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
 
     // register cleanup function
     atexit(cocoa_cleanup);
-
-    /* At this point QEMU has created all the consoles, so we can add View
-     * menu entries for them.
-     */
-    add_console_menu_entries();
-
-    /* Give all removable devices a menu item.
-     * Has to be called after QEMU has started to
-     * find out what removable devices it has.
-     */
-    addRemovableDevicesMenuItems();
 }
 
 static QemuDisplay qemu_display_cocoa = {
-- 
2.20.1

Re: [Qemu-devel] [PULL 1/7] ui/cocoa: Ensure we have the iothread lock when calling into QEMU

[Philippe Mathieu-Daudé]

Hi Peter,

On 3/4/19 5:49 PM, Peter Maydell wrote:
> The Cocoa UI should run on the main thread; this is enforced
> in OSX Mojave. In order to be able to run on the main thread,
> we need to make sure we hold the iothread lock whenever we
> call into various QEMU UI midlayer functions.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>
> Tested-by: Roman Bolshakov <r.bolshakov@yadro.com>

I got surprised by your 2 Message-id lines (same with other patches from
this pull request):

> Message-id: 20190225102433.22401-2-peter.maydell@linaro.org

^ v3

> Message-id: 20190214102816.3393-2-peter.maydell@linaro.org

^ v2

> ---
>  ui/cocoa.m | 91 ++++++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 65 insertions(+), 26 deletions(-)
> 
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index e2567d69466..f1171c48654 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -129,6 +129,21 @@ bool stretch_video;
>  NSTextField *pauseLabel;
>  NSArray * supportedImageFileTypes;
>  
> +// Utility function to run specified code block with iothread lock held
> +typedef void (^CodeBlock)(void);
> +
> +static void with_iothread_lock(CodeBlock block)
> +{
> +    bool locked = qemu_mutex_iothread_locked();
> +    if (!locked) {
> +        qemu_mutex_lock_iothread();
> +    }
> +    block();
> +    if (!locked) {
> +        qemu_mutex_unlock_iothread();
> +    }
> +}
> +
>  // Mac to QKeyCode conversion
>  const int mac_to_qkeycode_map[] = {
>      [kVK_ANSI_A] = Q_KEY_CODE_A,
> @@ -306,6 +321,7 @@ static void handleAnyDeviceErrors(Error * err)
>  - (void) toggleFullScreen:(id)sender;
>  - (void) handleMonitorInput:(NSEvent *)event;
>  - (void) handleEvent:(NSEvent *)event;
> +- (void) handleEventLocked:(NSEvent *)event;
>  - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled;
>  /* The state surrounding mouse grabbing is potentially confusing.
>   * isAbsoluteEnabled tracks qemu_input_is_absolute() [ie "is the emulated
> @@ -649,8 +665,14 @@ QemuCocoaView *cocoaView;
>  
>  - (void) handleEvent:(NSEvent *)event
>  {
> -    COCOA_DEBUG("QemuCocoaView: handleEvent\n");
> +    with_iothread_lock(^{
> +        [self handleEventLocked:event];
> +    });
> +}
>  
> +- (void) handleEventLocked:(NSEvent *)event
> +{
> +    COCOA_DEBUG("QemuCocoaView: handleEvent\n");
>      int buttons = 0;
>      int keycode = 0;
>      bool mouse_event = false;
> @@ -945,15 +967,18 @@ QemuCocoaView *cocoaView;
>   */
>  - (void) raiseAllKeys
>  {
> -    int index;
>      const int max_index = ARRAY_SIZE(modifiers_state);
>  
> -   for (index = 0; index < max_index; index++) {
> -       if (modifiers_state[index]) {
> -           modifiers_state[index] = 0;
> -           qemu_input_event_send_key_qcode(dcl->con, index, false);
> -       }
> -   }
> +    with_iothread_lock(^{
> +        int index;
> +
> +        for (index = 0; index < max_index; index++) {
> +            if (modifiers_state[index]) {
> +                modifiers_state[index] = 0;
> +                qemu_input_event_send_key_qcode(dcl->con, index, false);
> +            }
> +        }
> +    });
>  }
>  @end
>  
> @@ -1178,7 +1203,9 @@ QemuCocoaView *cocoaView;
>  /* Pause the guest */
>  - (void)pauseQEMU:(id)sender
>  {
> -    qmp_stop(NULL);
> +    with_iothread_lock(^{
> +        qmp_stop(NULL);
> +    });
>      [sender setEnabled: NO];
>      [[[sender menu] itemWithTitle: @"Resume"] setEnabled: YES];
>      [self displayPause];
> @@ -1187,7 +1214,9 @@ QemuCocoaView *cocoaView;
>  /* Resume running the guest operating system */
>  - (void)resumeQEMU:(id) sender
>  {
> -    qmp_cont(NULL);
> +    with_iothread_lock(^{
> +        qmp_cont(NULL);
> +    });
>      [sender setEnabled: NO];
>      [[[sender menu] itemWithTitle: @"Pause"] setEnabled: YES];
>      [self removePause];
> @@ -1215,13 +1244,17 @@ QemuCocoaView *cocoaView;
>  /* Restarts QEMU */
>  - (void)restartQEMU:(id)sender
>  {
> -    qmp_system_reset(NULL);
> +    with_iothread_lock(^{
> +        qmp_system_reset(NULL);
> +    });
>  }
>  
>  /* Powers down QEMU */
>  - (void)powerDownQEMU:(id)sender
>  {
> -    qmp_system_powerdown(NULL);
> +    with_iothread_lock(^{
> +        qmp_system_powerdown(NULL);
> +    });
>  }
>  
>  /* Ejects the media.
> @@ -1237,9 +1270,11 @@ QemuCocoaView *cocoaView;
>          return;
>      }
>  
> -    Error *err = NULL;
> -    qmp_eject(true, [drive cStringUsingEncoding: NSASCIIStringEncoding],
> -              false, NULL, false, false, &err);
> +    __block Error *err = NULL;
> +    with_iothread_lock(^{
> +        qmp_eject(true, [drive cStringUsingEncoding: NSASCIIStringEncoding],
> +                  false, NULL, false, false, &err);
> +    });
>      handleAnyDeviceErrors(err);
>  }
>  
> @@ -1271,16 +1306,18 @@ QemuCocoaView *cocoaView;
>              return;
>          }
>  
> -        Error *err = NULL;
> -        qmp_blockdev_change_medium(true,
> -                                   [drive cStringUsingEncoding:
> -                                          NSASCIIStringEncoding],
> -                                   false, NULL,
> -                                   [file cStringUsingEncoding:
> -                                         NSASCIIStringEncoding],
> -                                   true, "raw",
> -                                   false, 0,
> -                                   &err);
> +        __block Error *err = NULL;
> +        with_iothread_lock(^{
> +            qmp_blockdev_change_medium(true,
> +                                       [drive cStringUsingEncoding:
> +                                                  NSASCIIStringEncoding],
> +                                       false, NULL,
> +                                       [file cStringUsingEncoding:
> +                                                 NSASCIIStringEncoding],
> +                                       true, "raw",
> +                                       false, 0,
> +                                       &err);
> +        });
>          handleAnyDeviceErrors(err);
>      }
>  }
> @@ -1419,7 +1456,9 @@ QemuCocoaView *cocoaView;
>      // get the throttle percentage
>      throttle_pct = [sender tag];
>  
> -    cpu_throttle_set(throttle_pct);
> +    with_iothread_lock(^{
> +        cpu_throttle_set(throttle_pct);
> +    });
>      COCOA_DEBUG("cpu throttling at %d%c\n", cpu_throttle_get_percentage(), '%');
>  }
>  
> 

Re: [Qemu-devel] [PULL 1/7] ui/cocoa: Ensure we have the iothread lock when calling into QEMU

[Peter Maydell]

On Mon, 4 Mar 2019 at 17:45, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> Hi Peter,
>
> On 3/4/19 5:49 PM, Peter Maydell wrote:
> > The Cocoa UI should run on the main thread; this is enforced
> > in OSX Mojave. In order to be able to run on the main thread,
> > we need to make sure we hold the iothread lock whenever we
> > call into various QEMU UI midlayer functions.
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>
> > Tested-by: Roman Bolshakov <r.bolshakov@yadro.com>
>
> I got surprised by your 2 Message-id lines (same with other patches from
> this pull request):
>
> > Message-id: 20190225102433.22401-2-peter.maydell@linaro.org
>
> ^ v3
>
> > Message-id: 20190214102816.3393-2-peter.maydell@linaro.org

I think the 'patches' program tends to add an extra
Message-id: line even if one was already present. I treat
this header as pure noise and do not attempt to either add
it or remove it by hand.

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/7] cocoa queue

[Peter Maydell]

On Mon, 4 Mar 2019 at 16:49, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> Cocoa queue: this just has the refactoring to ensure we
> can run reliably on OSX Mojave. Thanks to everybody who helped
> in reviewing and testing this.
>
> -- PMM
>
> The following changes since commit 88687719c8859bf98ed94ce91612608c99c51c1a:
>
>   Merge remote-tracking branch 'remotes/stefanberger/tags/pull-tpm-2019-02-25-1' into staging (2019-03-04 15:56:08 +0000)
>
> are available in the Git repository at:
>
>   https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-cocoa-20190304
>
> for you to fetch changes up to 5588840ff77800e839d8a34b7be10e5c3acd1721:
>
>   ui/cocoa: Perform UI operations only on the main thread (2019-03-04 16:47:31 +0000)
>
> ----------------------------------------------------------------
> cocoa tree:
>  * Support OSX Mojave by ensuring that we always make Cocoa UI
>    function calls from the main thread, never from any other QEMU
>    thread. This was previously mostly harmless, but on Mojave
>    it will cause OSX to terminate the QEMU process.
>

Applied, thanks.

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

-- PMM