[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [RFC next] ui: Split main() in two to not have Cocoa hi

From: Andreas Färber
Subject: Re: [Qemu-devel] [RFC next] ui: Split main() in two to not have Cocoa hijack it
Date: Wed, 30 May 2012 18:11:29 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:12.0) Gecko/20120421 Thunderbird/12.0

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<address@hidden>
>> Cc: Anthony Liguori<address@hidden>
>> ---
>> +#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:

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.


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.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]