diff mbox

[RFC,next] ui: Split main() in two to not have Cocoa hijack it

Message ID 1338229139-94248-1-git-send-email-andreas.faerber@web.de
State New
Headers show

Commit Message

Andreas Färber May 28, 2012, 6:18 p.m. UTC 
Only call into cocoa.m when determined necessary by QEMU's option
handling. Avoids redoing all option parsing in ui/cocoa.m:main()
and constantly missing new options like -machine accel=qtest.

Move function declarations to a new ui.h header to avoid recompiling
everything when the new UI-internal function interface changes.

Handle the -psn option properly in vl.c.

Replace the faking of command line options for user-selected disk
image with proper API usage.

TODO: Clean up the main() vs. main2() split, including the naming.
This RFC simply takes the least intrusive approach by cutting
before the main loop initialization. The initialization of SPICE/VNC
may need to be moved up into main() for DT_DEFAULT handling.

RFC: Should we rather pass an opaque struct around, private to vl.c?

TODO: Eliminate remaining argv/argv checking in cocoa.m.

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
Cc: Anthony Liguori <anthony@codemonkey.ws>
---
 qemu-common.h   |    5 ----
 qemu-options.hx |    5 ++++
 ui/cocoa.m      |   71 +++++++++++++++++++++++++-----------------------------
 ui/ui.h         |   39 ++++++++++++++++++++++++++++++
 vl.c            |   56 ++++++++++++++++++++++++++++++++-----------
 5 files changed, 119 insertions(+), 57 deletions(-)
 create mode 100644 ui/ui.h

Comments

Anthony Liguori May 30, 2012, 7:11 a.m. UTC | #1 
On 05/29/2012 02:18 AM, Andreas Färber wrote:
> Only call into cocoa.m when determined necessary by QEMU's option
> handling. Avoids redoing all option parsing in ui/cocoa.m:main()
> and constantly missing new options like -machine accel=qtest.
>
> Move function declarations to a new ui.h header to avoid recompiling
> everything when the new UI-internal function interface changes.
>
> Handle the -psn option properly in vl.c.
>
> Replace the faking of command line options for user-selected disk
> image with proper API usage.
>
> TODO: Clean up the main() vs. main2() split, including the naming.
> This RFC simply takes the least intrusive approach by cutting
> before the main loop initialization. The initialization of SPICE/VNC
> may need to be moved up into main() for DT_DEFAULT handling.
>
> RFC: Should we rather pass an opaque struct around, private to vl.c?
>
> TODO: Eliminate remaining argv/argv checking in cocoa.m.
>
> Signed-off-by: Andreas Färber<andreas.faerber@web.de>
> Cc: Anthony Liguori<anthony@codemonkey.ws>
> ---
>   qemu-common.h   |    5 ----
>   qemu-options.hx |    5 ++++
>   ui/cocoa.m      |   71 +++++++++++++++++++++++++-----------------------------
>   ui/ui.h         |   39 ++++++++++++++++++++++++++++++
>   vl.c            |   56 ++++++++++++++++++++++++++++++++-----------
>   5 files changed, 119 insertions(+), 57 deletions(-)
>   create mode 100644 ui/ui.h
>
> diff --git a/qemu-common.h b/qemu-common.h
> index cccfb42..f6f1a84 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -127,11 +127,6 @@ extern int use_icount;
>
>   #endif /* !defined(NEED_CPU_H) */
>
> -/* main function, renamed */
> -#if defined(CONFIG_COCOA)
> -int qemu_main(int argc, char **argv, char **envp);
> -#endif
> -
>   void qemu_get_timedate(struct tm *tm, int offset);
>   int qemu_timedate_diff(struct tm *tm);
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 8b66264..fc1caa3 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2743,6 +2743,11 @@ DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log,
>       "-qtest-log LOG  specify tracing options\n",
>       QEMU_ARCH_ALL)
>
> +#ifdef CONFIG_COCOA
> +DEF("psn", 0, QEMU_OPTION_psn,
> +    "-psn            supplied by Finder", QEMU_ARCH_ALL)
> +#endif
> +
>   HXCOMM This is the last statement. Insert new options before this line!
>   STEXI
>   @end table
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 4724d2c..6e97b7e 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -26,8 +26,10 @@
>   #include<crt_externs.h>
>
>   #include "qemu-common.h"
> +#include "ui.h"
>   #include "console.h"
>   #include "sysemu.h"
> +#include "blockdev.h"
>
>   #ifndef MAC_OS_X_VERSION_10_4
>   #define MAC_OS_X_VERSION_10_4 1040
> @@ -67,6 +69,13 @@ static DisplayChangeListener *dcl;
>
>   int gArgc;
>   char **gArgv;
> +static void *gMachine;
> +static int gSnapshot;
> +static const char *gCPUModel, *gVGAModel;
> +static char *gBootDevices;
> +static const char *gICountOption;
> +static const char *gLoadVM, *gIncoming;
> +static bool gPSN;
>
>   // keymap conversion
>   int keymap[] =
> @@ -708,7 +717,7 @@ QemuCocoaView *cocoaView;
>   @interface QemuCocoaAppController : NSObject
>   {
>   }
> -- (void)startEmulationWithArgc:(int)argc argv:(char**)argv;
> +- (void)startEmulation;
>   - (void)openPanelDidEnd:(NSOpenPanel *)sheet returnCode:(int)returnCode contextInfo:(void *)contextInfo;
>   - (void)toggleFullScreen:(id)sender;
>   - (void)showQEMUDoc:(id)sender;
> @@ -764,16 +773,16 @@ QemuCocoaView *cocoaView;
>
>       // Display an open dialog box if no argument were passed or
>       // if qemu was launched from the finder ( the Finder passes "-psn" )
> -    if( gArgc<= 1 || strncmp ((char *)gArgv[1], "-psn", 4) == 0) {
> +    if (gArgc<= 1 || gPSN) {
>           NSOpenPanel *op = [[NSOpenPanel alloc] init];
>           [op setPrompt:@"Boot image"];
>           [op setMessage:@"Select the disk image you want to boot.\n\nHit the \"Cancel\" button to quit"];
> -        [op beginSheetForDirectory:nil file:nil types:[NSArray arrayWithObjects:@"img",@"iso",@"dmg",@"qcow",@"cow",@"cloop",@"vmdk",nil]
> +        [op beginSheetForDirectory:nil file:nil types:[NSArray arrayWithObjects:@"img",@"image",@"iso",@"dmg",@"qcow",@"cow",@"cloop",@"vmdk",nil]
>                 modalForWindow:normalWindow modalDelegate:self
>                 didEndSelector:@selector(openPanelDidEnd:returnCode:contextInfo:) contextInfo:NULL];
>       } else {
> -        // or launch QEMU, with the global args
> -        [self startEmulationWithArgc:gArgc argv:(char **)gArgv];
> +        // or start the emulation
> +        [self startEmulation];
>       }
>   }
>
> @@ -790,12 +799,13 @@ QemuCocoaView *cocoaView;
>       return YES;
>   }
>
> -- (void)startEmulationWithArgc:(int)argc argv:(char**)argv
> +- (void)startEmulation
>   {
>       COCOA_DEBUG("QemuCocoaAppController: startEmulationWithArgc\n");
>
>       int status;
> -    status = qemu_main(argc, argv, *_NSGetEnviron());
> +    status = main2(gMachine, gSnapshot, gCPUModel, gVGAModel, gBootDevices,
> +                   gICountOption, gLoadVM, gIncoming);
>       exit(status);
>   }
>
> @@ -806,20 +816,13 @@ QemuCocoaView *cocoaView;
>       if(returnCode == NSCancelButton) {
>           exit(0);
>       } else if(returnCode == NSOKButton) {
> -        const char *bin = "qemu";
>           char *img = (char*)[ [ sheet filename ] cStringUsingEncoding:NSASCIIStringEncoding];
>
> -        char **argv = (char**)malloc( sizeof(char*)*3 );
> -
>           [sheet close];
>
> -        asprintf(&argv[0], "%s", bin);
> -        asprintf(&argv[1], "-hda");
> -        asprintf(&argv[2], "%s", img);
> -
> -        printf("Using argc %d argv %s -hda %s\n", 3, bin, img);
> +        drive_add(IF_DEFAULT, 0, img, "media=disk");
>
> -        [self startEmulationWithArgc:3 argv:(char**)argv];
> +        [self startEmulation];
>       }
>   }
>   - (void)toggleFullScreen:(id)sender
> @@ -859,32 +862,24 @@ OSErr CPSGetCurrentProcess( CPSProcessSerNum *psn);
>   OSErr CPSEnableForegroundOperation( CPSProcessSerNum *psn, UInt32 _arg2, UInt32 _arg3, UInt32 _arg4, UInt32 _arg5);
>   OSErr CPSSetFrontProcess( CPSProcessSerNum *psn);
>
> -int main (int argc, const char * argv[]) {
> -
> +int cocoa_main(int argc, char **argv,
> +               void *machine, int snapshot,
> +               const char *cpu_model, const char *vga_model, char *boot_devices,
> +               const char *icount_option, const char *loadvm, const char *incoming, bool psn)
> +{
>       gArgc = argc;
>       gArgv = (char **)argv;
>       CPSProcessSerNum PSN;
> -    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, "-qtest")) {
> -                return qemu_main(gArgc, gArgv, *_NSGetEnviron());
> -            }
> -        }
> -    }
> +    gMachine = machine;
> +    gSnapshot = snapshot;
> +    gCPUModel = cpu_model;
> +    gVGAModel = vga_model;
> +    gBootDevices = boot_devices;
> +    gICountOption = icount_option;
> +    gLoadVM = loadvm;
> +    gIncoming = incoming;
> +    gPSN = psn;
>
>       NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
>       [NSApplication sharedApplication];
> diff --git a/ui/ui.h b/ui/ui.h
> new file mode 100644
> index 0000000..88c0c23
> --- /dev/null
> +++ b/ui/ui.h
> @@ -0,0 +1,39 @@
> +/*
> + * QEMU UI
> + *
> + * Copyright (c) 2012 Andreas Färber
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +#ifndef QEMU_UI_H
> +#define QEMU_UI_H
> +
> +#ifdef CONFIG_COCOA
> +int cocoa_main(int argc, char **argv,
> +               void *machine, int snapshot,
> +               const char *cpu_model, const char *vga_model, char *boot_devices,
> +               const char *icount_option, const char *loadvm, const char *incoming,
> +               bool psn);
> +#endif
> +
> +int main2(void *machine, int snapshot,
> +          const char *cpu_model, const char *vga_model, char *boot_devices,
> +          const char *icount_option, const char *loadvm, const char *incoming);
> +
> +#endif
> \ No newline at end of file
> diff --git a/vl.c b/vl.c
> index 23ab3a3..0c1ab6f 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -106,11 +106,6 @@ int main(int argc, char **argv)
>   #endif
>   #endif /* CONFIG_SDL */
>
> -#ifdef CONFIG_COCOA
> -#undef main
> -#define main qemu_main
> -#endif /* CONFIG_COCOA */
> -
>   #include<glib.h>
>
>   #include "hw/hw.h"
> @@ -166,6 +161,7 @@ int main(int argc, char **argv)
>   #include "cpus.h"
>   #include "arch_init.h"
>
> +#include "ui/ui.h"
>   #include "ui/qemu-spice.h"
>
>   //#define DEBUG_NET
> @@ -2269,15 +2265,11 @@ int qemu_init_main_loop(void)
>   int main(int argc, char **argv, char **envp)
>   {
>       int i;
> -    int snapshot, linux_boot;
> +    int snapshot;
>       const char *icount_option = NULL;
> -    const char *initrd_filename;
> -    const char *kernel_filename, *kernel_cmdline;
>       char boot_devices[33] = "cad"; /* default to HD->floppy->CD-ROM */
> -    DisplayState *ds;
> -    DisplayChangeListener *dcl;
>       int cyls, heads, secs, translation;
> -    QemuOpts *hda_opts = NULL, *opts, *machine_opts;
> +    QemuOpts *hda_opts = NULL, *opts;
>       QemuOptsList *olist;
>       int optind;
>       const char *optarg;
> @@ -2287,9 +2279,6 @@ int main(int argc, char **argv, char **envp)
>       const char *vga_model = "none";
>       const char *pid_file = NULL;
>       const char *incoming = NULL;
> -#ifdef CONFIG_VNC
> -    int show_vnc_port = 0;
> -#endif
>       bool defconfig = true;
>       bool userconfig = true;
>       const char *log_mask = NULL;
> @@ -2301,6 +2290,9 @@ int main(int argc, char **argv, char **envp)
>       };
>       const char *trace_events = NULL;
>       const char *trace_file = NULL;
> +#ifdef CONFIG_COCOA
> +    bool psn = false;
> +#endif
>
>       atexit(qemu_run_exit_notifiers);
>       error_set_progname(argv[0]);
> @@ -3201,6 +3193,11 @@ int main(int argc, char **argv, char **envp)
>               case QEMU_OPTION_qtest_log:
>                   qtest_log = optarg;
>                   break;
> +#ifdef CONFIG_COCOA
> +            case QEMU_OPTION_psn:
> +                psn = true;
> +                break;
> +#endif
>               default:
>                   os_parse_cmd_args(popt->index, optarg);
>               }
> @@ -3347,6 +3344,37 @@ int main(int argc, char **argv, char **envp)
>
>       configure_accelerator();
>
> +#ifdef CONFIG_COCOA
> +    if (display_type == DT_DEFAULT || display_type == DT_SDL) {
> +        return cocoa_main(argc, argv, machine, snapshot,
> +                          cpu_model, vga_model, boot_devices,
> +                          icount_option, loadvm, incoming, psn);
> +    }
> +#endif
> +
> +    return main2(machine, snapshot,
> +                 cpu_model, vga_model, boot_devices,
> +                 icount_option, loadvm, incoming);
> +}
> +
> +/***************************************************************************/
> +
> +int main2(void *mach, int snapshot,
> +          const char *cpu_model, const char *vga_model, char *boot_devices,
> +          const char *icount_option, const char *loadvm, const char *incoming)

Gross.

I appreciate what you're trying to achieve here but this makes the code much 
worse than it is today.  The split is arbitrary and I don't think there's a 
worse name possible than 'main2'.

Why is cocoa so special that it needs to hijack the main loop?  I suspect 
there's something wrong here that should be fixed.

Regards,

Anthony Liguori

> +{
> +    QEMUMachine *machine = mach;
> +    QemuOpts *machine_opts;
> +    const char *initrd_filename;
> +    const char *kernel_filename, *kernel_cmdline;
> +    bool linux_boot;
> +    DisplayState *ds;
> +    DisplayChangeListener *dcl;
> +#ifdef CONFIG_VNC
> +    int show_vnc_port = 0;
> +#endif
> +    int i;
> +
>       qemu_init_cpu_loop();
>       if (qemu_init_main_loop()) {
>           fprintf(stderr, "qemu_init_main_loop failed\n");
Paolo Bonzini May 30, 2012, 9 a.m. UTC | #2 
Il 28/05/2012 20:18, Andreas Färber ha scritto:
> Only call into cocoa.m when determined necessary by QEMU's option
> handling. Avoids redoing all option parsing in ui/cocoa.m:main()
> and constantly missing new options like -machine accel=qtest.
> 
> Move function declarations to a new ui.h header to avoid recompiling
> everything when the new UI-internal function interface changes.
> 
> Handle the -psn option properly in vl.c.
> 
> Replace the faking of command line options for user-selected disk
> image with proper API usage.

This is nice. :)

But the split between main/main2 is ugly.

Is it possible to run the main loop (basically everything starting at
the creation of the NSAutoreleasePool on) in a separate thread?
cocoa_main starts the thread and sits on a condition variable waiting
for applicationDidFinishLaunching: to be called.

applicationDidFinishLaunching: signals that condition variable, then
goes on a loop like

    qemu_mutex_lock(&qemu_cocoa_mutex);
    globalCons = 0;
    localCons = globalCons;

    for (;;) {
        while (!exiting && localCons == globalCons) {
            qemu_cond_wait(&qemu_cocoa_cond, &qemu_cocoa_mutex);
        }
        if (exiting) {
            break;
        }

        /* we can behave as if we held the global mutex, we
           know the iothread is waiting for us */
        ... content of cocoa_refresh ...

        globalProd++;
        qemu_cond_broadcast(&qemu_cocoa_mutex);
    }
    qemu_mutex_unlock(&qemu_cocoa_mutex);
    [NSApp stop];

and cocoa_refresh only handles the rendez-vous:

    qemu_mutex_lock(&qemu_cocoa_mutex);
    localProd = globalProd;
    globalCons++;
    qemu_cond_broadcast(&qemu_cocoa_mutex);
    while (localProd == globalProd) {
        qemu_cond_wait(&qemu_cocoa_cond, &qemu_cocoa_mutex);
    }
    qemu_mutex_unlock(&qemu_cocoa_mutex);
    vga_hw_update();

(Disclaimer, I used Cocoa exactly once).

Paolo
Andreas Färber May 30, 2012, 4:11 p.m. UTC | #3 
Am 30.05.2012 09:11, schrieb Anthony Liguori:
> On 05/29/2012 02:18 AM, Andreas Färber wrote:
>> Only call into cocoa.m when determined necessary by QEMU's option
>> handling. Avoids redoing all option parsing in ui/cocoa.m:main()
>> and constantly missing new options like -machine accel=qtest.
>>
>> Move function declarations to a new ui.h header to avoid recompiling
>> everything when the new UI-internal function interface changes.
>>
>> Handle the -psn option properly in vl.c.
>>
>> Replace the faking of command line options for user-selected disk
>> image with proper API usage.
>>
>> TODO: Clean up the main() vs. main2() split, including the naming.
>> This RFC simply takes the least intrusive approach by cutting
>> before the main loop initialization. The initialization of SPICE/VNC
>> may need to be moved up into main() for DT_DEFAULT handling.
>>
>> RFC: Should we rather pass an opaque struct around, private to vl.c?
>>
>> TODO: Eliminate remaining argv/argv checking in cocoa.m.
>>
>> Signed-off-by: Andreas Färber<andreas.faerber@web.de>
>> Cc: Anthony Liguori<anthony@codemonkey.ws>
>> ---
[...]
>>
>> +#ifdef CONFIG_COCOA
>> +    if (display_type == DT_DEFAULT || display_type == DT_SDL) {
>> +        return cocoa_main(argc, argv, machine, snapshot,
>> +                          cpu_model, vga_model, boot_devices,
>> +                          icount_option, loadvm, incoming, psn);
>> +    }
>> +#endif
>> +
>> +    return main2(machine, snapshot,
>> +                 cpu_model, vga_model, boot_devices,
>> +                 icount_option, loadvm, incoming);
>> +}
>> +
>> +/***************************************************************************/
>>
>> +
>> +int main2(void *mach, int snapshot,
>> +          const char *cpu_model, const char *vga_model, char
>> *boot_devices,
>> +          const char *icount_option, const char *loadvm, const char
>> *incoming)
> 
> Gross.
> 
> I appreciate what you're trying to achieve here but this makes the code
> much worse than it is today.  The split is arbitrary and I don't think
> there's a worse name possible than 'main2'.

Quote: "TODO: Clean up the main() vs. main2() split, including the
naming." :-)

Yes, least intrusive => arbitrary to some degree. Surely a patch could
be prepended to improve the point of split, moving some more stuff into
main() like the mentioned SPICE/VNC init.

The general idea is to split the option-parsing from the option-using
where it affects the threading model, to provision for calling it on a
secondary thread.

> Why is cocoa so special that it needs to hijack the main loop?  I
> suspect there's something wrong here that should be fixed.

Let me put it this way: Not every UI toolkit is designed for C or in the
same way as SDL. The same issue applies to the proposed Haiku C++
frontend as well.

For Cocoa I know from my experience with Cocoa-sharp / Cocoa-sharq that
it wants to initialize on thread 0 and does not return, and using the
Objective-C runtime functions to do so from C would get ugly. So we need
to do it from Objective-C source code and use the provided Objective-C
callbacks such as applicationDidFinishBlabla to react to events and call
back into C from there.

So no, we can't do Cocoa initialization on a separate thread spawned
from a late cocoa_display_init(), but we can do second-stage QEMU
initialization - here main2() - on a new thread, as done in mmlr's patch
set for Haiku. Last snapshot for reference:
http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/haiku-ui

And what I would find gross is reinventing QemuOpts just to decide if we
need to call the regular main() or continue with the existing ugly Cocoa
main() that I'm trying to fix here... I'm open to suggestions.

Btw, SDL does have code in place for hijacking main(), too.

Regards,
Andreas

P.S. I was not able to reproduce the -psn case on v10.5, so I'm guessing
that only applies when launching the application from a bundle, which we
don't have at the moment. Maybe it leaked in from Q.
Andreas Färber May 30, 2012, 4:22 p.m. UTC | #4 
Am 30.05.2012 11:00, schrieb Paolo Bonzini:
> Il 28/05/2012 20:18, Andreas Färber ha scritto:
>> Only call into cocoa.m when determined necessary by QEMU's option
>> handling. Avoids redoing all option parsing in ui/cocoa.m:main()
>> and constantly missing new options like -machine accel=qtest.
>>
>> Move function declarations to a new ui.h header to avoid recompiling
>> everything when the new UI-internal function interface changes.
>>
>> Handle the -psn option properly in vl.c.
>>
>> Replace the faking of command line options for user-selected disk
>> image with proper API usage.
> 
> This is nice. :)
> 
> But the split between main/main2 is ugly.
> 
> Is it possible to run the main loop (basically everything starting at
> the creation of the NSAutoreleasePool on) in a separate thread?

No.

> cocoa_main starts the thread and sits on a condition variable waiting
> for applicationDidFinishLaunching: to be called.
> 
> applicationDidFinishLaunching: signals that condition variable, then
> goes on a loop like
> 
>     qemu_mutex_lock(&qemu_cocoa_mutex);
>     globalCons = 0;
>     localCons = globalCons;
> 
>     for (;;) {
>         while (!exiting && localCons == globalCons) {
>             qemu_cond_wait(&qemu_cocoa_cond, &qemu_cocoa_mutex);
>         }
>         if (exiting) {
>             break;
>         }
> 
>         /* we can behave as if we held the global mutex, we
>            know the iothread is waiting for us */
>         ... content of cocoa_refresh ...
> 
>         globalProd++;
>         qemu_cond_broadcast(&qemu_cocoa_mutex);
>     }
>     qemu_mutex_unlock(&qemu_cocoa_mutex);
>     [NSApp stop];
> 
> and cocoa_refresh only handles the rendez-vous:
> 
>     qemu_mutex_lock(&qemu_cocoa_mutex);
>     localProd = globalProd;
>     globalCons++;
>     qemu_cond_broadcast(&qemu_cocoa_mutex);
>     while (localProd == globalProd) {
>         qemu_cond_wait(&qemu_cocoa_cond, &qemu_cocoa_mutex);
>     }
>     qemu_mutex_unlock(&qemu_cocoa_mutex);
>     vga_hw_update();

I'd have to investigate this, but the main issue seemed to be that we
cannot *inline* the startup code into QEMU's main() as it is. If we
manage to split it at some sensible point, we can switch to a
multithreaded model. However from what I remember, going as low-level as
creating our own main loop would require re(verse)-engineering Cocoa
code iirc.

Andreas
diff mbox

Patch

diff --git a/qemu-common.h b/qemu-common.h
index cccfb42..f6f1a84 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -127,11 +127,6 @@  extern int use_icount;
 
 #endif /* !defined(NEED_CPU_H) */
 
-/* main function, renamed */
-#if defined(CONFIG_COCOA)
-int qemu_main(int argc, char **argv, char **envp);
-#endif
-
 void qemu_get_timedate(struct tm *tm, int offset);
 int qemu_timedate_diff(struct tm *tm);
 
diff --git a/qemu-options.hx b/qemu-options.hx
index 8b66264..fc1caa3 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2743,6 +2743,11 @@  DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log,
     "-qtest-log LOG  specify tracing options\n",
     QEMU_ARCH_ALL)
 
+#ifdef CONFIG_COCOA
+DEF("psn", 0, QEMU_OPTION_psn,
+    "-psn            supplied by Finder", QEMU_ARCH_ALL)
+#endif
+
 HXCOMM This is the last statement. Insert new options before this line!
 STEXI
 @end table
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 4724d2c..6e97b7e 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -26,8 +26,10 @@ 
 #include <crt_externs.h>
 
 #include "qemu-common.h"
+#include "ui.h"
 #include "console.h"
 #include "sysemu.h"
+#include "blockdev.h"
 
 #ifndef MAC_OS_X_VERSION_10_4
 #define MAC_OS_X_VERSION_10_4 1040
@@ -67,6 +69,13 @@  static DisplayChangeListener *dcl;
 
 int gArgc;
 char **gArgv;
+static void *gMachine;
+static int gSnapshot;
+static const char *gCPUModel, *gVGAModel;
+static char *gBootDevices;
+static const char *gICountOption;
+static const char *gLoadVM, *gIncoming;
+static bool gPSN;
 
 // keymap conversion
 int keymap[] =
@@ -708,7 +717,7 @@  QemuCocoaView *cocoaView;
 @interface QemuCocoaAppController : NSObject
 {
 }
-- (void)startEmulationWithArgc:(int)argc argv:(char**)argv;
+- (void)startEmulation;
 - (void)openPanelDidEnd:(NSOpenPanel *)sheet returnCode:(int)returnCode contextInfo:(void *)contextInfo;
 - (void)toggleFullScreen:(id)sender;
 - (void)showQEMUDoc:(id)sender;
@@ -764,16 +773,16 @@  QemuCocoaView *cocoaView;
 
     // Display an open dialog box if no argument were passed or
     // if qemu was launched from the finder ( the Finder passes "-psn" )
-    if( gArgc <= 1 || strncmp ((char *)gArgv[1], "-psn", 4) == 0) {
+    if (gArgc <= 1 || gPSN) {
         NSOpenPanel *op = [[NSOpenPanel alloc] init];
         [op setPrompt:@"Boot image"];
         [op setMessage:@"Select the disk image you want to boot.\n\nHit the \"Cancel\" button to quit"];
-        [op beginSheetForDirectory:nil file:nil types:[NSArray arrayWithObjects:@"img",@"iso",@"dmg",@"qcow",@"cow",@"cloop",@"vmdk",nil]
+        [op beginSheetForDirectory:nil file:nil types:[NSArray arrayWithObjects:@"img",@"image",@"iso",@"dmg",@"qcow",@"cow",@"cloop",@"vmdk",nil]
               modalForWindow:normalWindow modalDelegate:self
               didEndSelector:@selector(openPanelDidEnd:returnCode:contextInfo:) contextInfo:NULL];
     } else {
-        // or launch QEMU, with the global args
-        [self startEmulationWithArgc:gArgc argv:(char **)gArgv];
+        // or start the emulation
+        [self startEmulation];
     }
 }
 
@@ -790,12 +799,13 @@  QemuCocoaView *cocoaView;
     return YES;
 }
 
-- (void)startEmulationWithArgc:(int)argc argv:(char**)argv
+- (void)startEmulation
 {
     COCOA_DEBUG("QemuCocoaAppController: startEmulationWithArgc\n");
 
     int status;
-    status = qemu_main(argc, argv, *_NSGetEnviron());
+    status = main2(gMachine, gSnapshot, gCPUModel, gVGAModel, gBootDevices,
+                   gICountOption, gLoadVM, gIncoming);
     exit(status);
 }
 
@@ -806,20 +816,13 @@  QemuCocoaView *cocoaView;
     if(returnCode == NSCancelButton) {
         exit(0);
     } else if(returnCode == NSOKButton) {
-        const char *bin = "qemu";
         char *img = (char*)[ [ sheet filename ] cStringUsingEncoding:NSASCIIStringEncoding];
 
-        char **argv = (char**)malloc( sizeof(char*)*3 );
-
         [sheet close];
 
-        asprintf(&argv[0], "%s", bin);
-        asprintf(&argv[1], "-hda");
-        asprintf(&argv[2], "%s", img);
-
-        printf("Using argc %d argv %s -hda %s\n", 3, bin, img);
+        drive_add(IF_DEFAULT, 0, img, "media=disk");
 
-        [self startEmulationWithArgc:3 argv:(char**)argv];
+        [self startEmulation];
     }
 }
 - (void)toggleFullScreen:(id)sender
@@ -859,32 +862,24 @@  OSErr CPSGetCurrentProcess( CPSProcessSerNum *psn);
 OSErr CPSEnableForegroundOperation( CPSProcessSerNum *psn, UInt32 _arg2, UInt32 _arg3, UInt32 _arg4, UInt32 _arg5);
 OSErr CPSSetFrontProcess( CPSProcessSerNum *psn);
 
-int main (int argc, const char * argv[]) {
-
+int cocoa_main(int argc, char **argv,
+               void *machine, int snapshot,
+               const char *cpu_model, const char *vga_model, char *boot_devices,
+               const char *icount_option, const char *loadvm, const char *incoming, bool psn)
+{
     gArgc = argc;
     gArgv = (char **)argv;
     CPSProcessSerNum PSN;
-    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, "-qtest")) {
-                return qemu_main(gArgc, gArgv, *_NSGetEnviron());
-            }
-        }
-    }
+    gMachine = machine;
+    gSnapshot = snapshot;
+    gCPUModel = cpu_model;
+    gVGAModel = vga_model;
+    gBootDevices = boot_devices;
+    gICountOption = icount_option;
+    gLoadVM = loadvm;
+    gIncoming = incoming;
+    gPSN = psn;
 
     NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
     [NSApplication sharedApplication];
diff --git a/ui/ui.h b/ui/ui.h
new file mode 100644
index 0000000..88c0c23
--- /dev/null
+++ b/ui/ui.h
@@ -0,0 +1,39 @@ 
+/*
+ * QEMU UI
+ *
+ * Copyright (c) 2012 Andreas Färber
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#ifndef QEMU_UI_H
+#define QEMU_UI_H
+
+#ifdef CONFIG_COCOA
+int cocoa_main(int argc, char **argv,
+               void *machine, int snapshot,
+               const char *cpu_model, const char *vga_model, char *boot_devices,
+               const char *icount_option, const char *loadvm, const char *incoming,
+               bool psn);
+#endif
+
+int main2(void *machine, int snapshot,
+          const char *cpu_model, const char *vga_model, char *boot_devices,
+          const char *icount_option, const char *loadvm, const char *incoming);
+
+#endif
\ No newline at end of file
diff --git a/vl.c b/vl.c
index 23ab3a3..0c1ab6f 100644
--- a/vl.c
+++ b/vl.c
@@ -106,11 +106,6 @@  int main(int argc, char **argv)
 #endif
 #endif /* CONFIG_SDL */
 
-#ifdef CONFIG_COCOA
-#undef main
-#define main qemu_main
-#endif /* CONFIG_COCOA */
-
 #include <glib.h>
 
 #include "hw/hw.h"
@@ -166,6 +161,7 @@  int main(int argc, char **argv)
 #include "cpus.h"
 #include "arch_init.h"
 
+#include "ui/ui.h"
 #include "ui/qemu-spice.h"
 
 //#define DEBUG_NET
@@ -2269,15 +2265,11 @@  int qemu_init_main_loop(void)
 int main(int argc, char **argv, char **envp)
 {
     int i;
-    int snapshot, linux_boot;
+    int snapshot;
     const char *icount_option = NULL;
-    const char *initrd_filename;
-    const char *kernel_filename, *kernel_cmdline;
     char boot_devices[33] = "cad"; /* default to HD->floppy->CD-ROM */
-    DisplayState *ds;
-    DisplayChangeListener *dcl;
     int cyls, heads, secs, translation;
-    QemuOpts *hda_opts = NULL, *opts, *machine_opts;
+    QemuOpts *hda_opts = NULL, *opts;
     QemuOptsList *olist;
     int optind;
     const char *optarg;
@@ -2287,9 +2279,6 @@  int main(int argc, char **argv, char **envp)
     const char *vga_model = "none";
     const char *pid_file = NULL;
     const char *incoming = NULL;
-#ifdef CONFIG_VNC
-    int show_vnc_port = 0;
-#endif
     bool defconfig = true;
     bool userconfig = true;
     const char *log_mask = NULL;
@@ -2301,6 +2290,9 @@  int main(int argc, char **argv, char **envp)
     };
     const char *trace_events = NULL;
     const char *trace_file = NULL;
+#ifdef CONFIG_COCOA
+    bool psn = false;
+#endif
 
     atexit(qemu_run_exit_notifiers);
     error_set_progname(argv[0]);
@@ -3201,6 +3193,11 @@  int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_qtest_log:
                 qtest_log = optarg;
                 break;
+#ifdef CONFIG_COCOA
+            case QEMU_OPTION_psn:
+                psn = true;
+                break;
+#endif
             default:
                 os_parse_cmd_args(popt->index, optarg);
             }
@@ -3347,6 +3344,37 @@  int main(int argc, char **argv, char **envp)
 
     configure_accelerator();
 
+#ifdef CONFIG_COCOA
+    if (display_type == DT_DEFAULT || display_type == DT_SDL) {
+        return cocoa_main(argc, argv, machine, snapshot,
+                          cpu_model, vga_model, boot_devices,
+                          icount_option, loadvm, incoming, psn);
+    }
+#endif
+
+    return main2(machine, snapshot,
+                 cpu_model, vga_model, boot_devices,
+                 icount_option, loadvm, incoming);
+}
+
+/***************************************************************************/
+
+int main2(void *mach, int snapshot,
+          const char *cpu_model, const char *vga_model, char *boot_devices,
+          const char *icount_option, const char *loadvm, const char *incoming)
+{
+    QEMUMachine *machine = mach;
+    QemuOpts *machine_opts;
+    const char *initrd_filename;
+    const char *kernel_filename, *kernel_cmdline;
+    bool linux_boot;
+    DisplayState *ds;
+    DisplayChangeListener *dcl;
+#ifdef CONFIG_VNC
+    int show_vnc_port = 0;
+#endif
+    int i;
+
     qemu_init_cpu_loop();
     if (qemu_init_main_loop()) {
         fprintf(stderr, "qemu_init_main_loop failed\n");