[Qemu-devel] Patch for ui/cocoa.m

[Qemu-devel] Patch for ui/cocoa.m

[Juan Pineda]

Hello,

This is my first time submitting a patch, so please let me know if I'm not following the correct protocol.

Under OSX Lion the boot volume dialog is not closed and it permanently obscures the emulator window since under Lion the dialog cannot be repositioned. The fix adds only to add a single line to close the dialog.

Signed-off-by: Juan Pineda <juan@logician.com>

Thanks!
-Juan


> git diff ui/cocoa.m
diff --git a/ui/cocoa.m b/ui/cocoa.m
index d9e4e3d..4b42462 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -811,6 +811,8 @@ QemuCocoaView *cocoaView;
 
         char **argv = (char**)malloc( sizeof(char*)*3 );
 
+       [sheet close];
+
         asprintf(&argv[0], "%s", bin);
         asprintf(&argv[1], "-hda");
         asprintf(&argv[2], "%s", img);

Re: [Qemu-devel] Patch for ui/cocoa.m

[Andreas Färber]

Hello Juan,

Am 18.10.2011 03:22, schrieb Juan Pineda:
> This is my first time submitting a patch, so please let me know if I'm not following the correct protocol.

Please see http://wiki.qemu.org/Contribute/SubmitAPatch

Using the git-send-mail tool assures that the patch format gets right.

Then, this patch despite its small size is not "trivial" so instead of
qemu-trivial@nongnu.org please cc andreas.faerber@web.de for Cocoa. Thanks.

> Under OSX Lion the boot volume dialog is not closed and it permanently obscures the emulator window since under Lion the dialog cannot be repositioned. The fix adds only to add a single line to close the dialog.

I don't have access to v10.7 so please describe the problem in more
details: What command line do you use? If using the right arguments you
shouldn't see a window at all. Are you maybe using -drive instead of
-hda and that is not yet handled correctly?

Apart from this issue, is it working correctly for you?

Regards,
Andreas

> Signed-off-by: Juan Pineda <juan@logician.com>
> 
> Thanks!
> -Juan
> 
> 
>> git diff ui/cocoa.m
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index d9e4e3d..4b42462 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -811,6 +811,8 @@ QemuCocoaView *cocoaView;
>  
>          char **argv = (char**)malloc( sizeof(char*)*3 );
>  
> +       [sheet close];
> +
>          asprintf(&argv[0], "%s", bin);
>          asprintf(&argv[1], "-hda");
>          asprintf(&argv[2], "%s", img);
> 
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746, AG Nürnberg

Re: [Qemu-devel] Patch for ui/cocoa.m

[Juan Pineda]

Hi Andreas,

Thanks for your quick reply. Regarding your questions:

>> What command line do you use? If using the right arguments you
>> shouldn't see a window at all. Are you maybe using -drive instead of
>> -hda and that is not yet handled correctly?

The boot volume dialog appears only when a hard disk file is not supplied on the command line. I would think the failure of the dialog to close is not unique to Lion. However in prior OS releases it probably does not present a problem as the main window can be raised above the dialog.

>> Apart from this issue, is it working correctly for you?


Yes and no. I am able to compile and run version 0.15.1 under Lion by specifying some flags in the configure phase. More information in my post to Qemu-devel here:

> http://lists.nongnu.org/archive/html/qemu-devel/2011-10/msg02020.html

With the specified flags, version 0.15.1 seems to run OK with my limited testing, although I have seen some segment violations using qemu-system-sparc trying to boot some Linux images.

However building the main branch (with flags as above) does not work. The build fails with error about redefinition of uint16 in fpu/softfloat.h. If that is fixed the build completes. But trying to run qemu-system-i386 quits:

> $ i386-softmmu/qemu-system-i386 -hda ~/Downloads/linux-0.2.img
> 
> GThread-ERROR **: GThread system may only be initialized once.
> aborting...
> Abort trap: 6


Building also displays many warning messages about redefinition of CONFIG_EMU_PREFIX. I'm running with OSX 10.7 and Xcode completely up to the latest revisions.

Thanks!
-Juan



On Oct 18, 2011, at 2:11 AM, Andreas Färber wrote:

> Hello Juan,
> 
> Am 18.10.2011 03:22, schrieb Juan Pineda:
>> This is my first time submitting a patch, so please let me know if I'm not following the correct protocol.
> 
> Please see http://wiki.qemu.org/Contribute/SubmitAPatch
> 
> Using the git-send-mail tool assures that the patch format gets right.
> 
> Then, this patch despite its small size is not "trivial" so instead of
> qemu-trivial@nongnu.org please cc andreas.faerber@web.de for Cocoa. Thanks.
> 
>> Under OSX Lion the boot volume dialog is not closed and it permanently obscures the emulator window since under Lion the dialog cannot be repositioned. The fix adds only to add a single line to close the dialog.
> 
> I don't have access to v10.7 so please describe the problem in more
> details: What command line do you use? If using the right arguments you
> shouldn't see a window at all. Are you maybe using -drive instead of
> -hda and that is not yet handled correctly?
> 
> Apart from this issue, is it working correctly for you?
> 
> Regards,
> Andreas
> 
>> Signed-off-by: Juan Pineda <juan@logician.com>
>> 
>> Thanks!
>> -Juan
>> 
>> 
>>> git diff ui/cocoa.m
>> diff --git a/ui/cocoa.m b/ui/cocoa.m
>> index d9e4e3d..4b42462 100644
>> --- a/ui/cocoa.m
>> +++ b/ui/cocoa.m
>> @@ -811,6 +811,8 @@ QemuCocoaView *cocoaView;
>> 
>>         char **argv = (char**)malloc( sizeof(char*)*3 );
>> 
>> +       [sheet close];
>> +
>>         asprintf(&argv[0], "%s", bin);
>>         asprintf(&argv[1], "-hda");
>>         asprintf(&argv[2], "%s", img);
>> 
>> 
> 
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746, AG Nürnberg

Re: [Qemu-devel] Patch for ui/cocoa.m

[Andreas Färber]

Am 18.10.2011 16:35, schrieb Juan Pineda:
>>> What command line do you use? If using the right arguments you
>>> shouldn't see a window at all. Are you maybe using -drive instead of
>>> -hda and that is not yet handled correctly?
> 
> The boot volume dialog appears only when a hard disk file is not supplied on the command line. I would think the failure of the dialog to close is not unique to Lion. However in prior OS releases it probably does not present a problem as the main window can be raised above the dialog.

If that were the case I would see it as worth fixing there, too. :)

>>> Apart from this issue, is it working correctly for you?

> However building the main branch (with flags as above) does not work. The build fails with error about redefinition of uint16 in fpu/softfloat.h. If that is fixed the build completes. But trying to run qemu-system-i386 quits:
> 
>> $ i386-softmmu/qemu-system-i386 -hda ~/Downloads/linux-0.2.img
>>
>> GThread-ERROR **: GThread system may only be initialized once.
>> aborting...
>> Abort trap: 6

That's what I'm seeing, too, so I don't feel comfortable committing this
patch now. It compiles file, but it crashes either way with a message
similar to the above, so I don't see an improvement (yet).

If you could pinpoint where this error stems from (e.g., single-stepping
in gdb) that might help getting it fixed.

Thanks,
Andreas

Re: [Qemu-devel] Patch for ui/cocoa.m

[Juan Pineda]

Hi Andreas,

Thanks for your reply. You can verify that my cocoa.m patch works 100% under 0.15.1. That's where I was testing it. The fix is completely independent of the other problems in the OSX build (uint16, GThread, etc.) that have cropped up since 0.15.1, so there should be no problem merging it.

I will us GDB on that GThread error to find the source code line where it failed and post what I find.

Thanks!
-Juan



On Oct 30, 2011, at 3:45 PM, Andreas Färber wrote:

> Am 18.10.2011 16:35, schrieb Juan Pineda:
>>>> What command line do you use? If using the right arguments you
>>>> shouldn't see a window at all. Are you maybe using -drive instead of
>>>> -hda and that is not yet handled correctly?
>> 
>> The boot volume dialog appears only when a hard disk file is not supplied on the command line. I would think the failure of the dialog to close is not unique to Lion. However in prior OS releases it probably does not present a problem as the main window can be raised above the dialog.
> 
> If that were the case I would see it as worth fixing there, too. :)
> 
>>>> Apart from this issue, is it working correctly for you?
> 
>> However building the main branch (with flags as above) does not work. The build fails with error about redefinition of uint16 in fpu/softfloat.h. If that is fixed the build completes. But trying to run qemu-system-i386 quits:
>> 
>>> $ i386-softmmu/qemu-system-i386 -hda ~/Downloads/linux-0.2.img
>>> 
>>> GThread-ERROR **: GThread system may only be initialized once.
>>> aborting...
>>> Abort trap: 6
> 
> That's what I'm seeing, too, so I don't feel comfortable committing this
> patch now. It compiles file, but it crashes either way with a message
> similar to the above, so I don't see an improvement (yet).
> 
> If you could pinpoint where this error stems from (e.g., single-stepping
> in gdb) that might help getting it fixed.
> 
> Thanks,
> Andreas

[Qemu-devel] Problem under OSX Lion: GThread-ERROR **: GThread system may only be initialized once

[Juan Pineda]

The current Qemu master branch crashes under OSX with a GThread error. Commenting out vlc.c line 2188 (call to g_thread_init) allows it to run successfully. I am running OSX Lion. Two caveats:

1) You need to apply Andreas' patch to softfloat.h to fix compile error for uint16
2) You need to compile with gcc-4.2 (non LLVM gcc) and specify --disable-user to successfully compile

See log below.

Thanks!
-Juan



$ ./configure --disable-user --enable-debug --cc=gcc-4.2 --host-cc=gcc-4.2
$ make
$ gdb ./i386-softmmu/qemu-system-i386 
...
GThread-ERROR **: GThread system may only be initialized once.
aborting...

Program received signal SIGABRT, Aborted.
0x00007fff8e50dce2 in __pthread_kill ()
(gdb) bt
#0  0x00007fff8e50dce2 in __pthread_kill ()
#1  0x00007fff94dc17d2 in pthread_kill ()
#2  0x00007fff94db2a7a in abort ()
#3  0x0000000100da28c3 in g_logv ()
#4  0x0000000100da247e in g_log ()
#5  0x0000000100d74333 in g_thread_init ()
#6  0x00000001000e6d8c in qemu_main (argc=3, argv=0x10a896710, envp=0x7fff5fbff120) at /Users/juan/Temp/build/qemu-source_code/vl.c:2188
#7  0x0000000100142e65 in -[QemuCocoaAppController startEmulationWithArgc:argv:] (self=0x1011299d0, _cmd=0x1002818e4, argc=3, argv=0x10a896710) at cocoa.m:798
#8  0x000000010014306a in -[QemuCocoaAppController openPanelDidEnd:returnCode:contextInfo:] (self=0x1011299d0, _cmd=0x1002818aa, sheet=0x10114e450, returnCode=1, contextInfo=0x0) at cocoa.m:822
#9  0x00007fff9551955e in -[NSSavePanel _didEndSheet:returnCode:contextInfo:] ()
#10 0x00007fff952987d7 in -[NSApplication endSheet:returnCode:] ()
#11 0x00007fff94a78a1d in -[NSObject performSelector:withObject:] ()
#12 0x00007fff950e7710 in -[NSApplication sendAction:to:from:] ()
#13 0x00007fff950e7642 in -[NSControl sendAction:to:] ()
#14 0x00007fff950e756d in -[NSCell _sendActionFrom:] ()
#15 0x00007fff950e6a30 in -[NSCell trackMouse:inRect:ofView:untilMouseUp:] ()
#16 0x00007fff951668e0 in -[NSButtonCell trackMouse:inRect:ofView:untilMouseUp:] ()
#17 0x00007fff950e563a in -[NSControl mouseDown:] ()
#18 0x00007fff950b00e0 in -[NSWindow sendEvent:] ()
#19 0x00007fff9504868f in -[NSApplication sendEvent:] ()
#20 0x00007fff94fde682 in -[NSApplication run] ()
#21 0x0000000100144254 in main (argc=1, argv=0x7fff5fbff110) at cocoa.m:945
(gdb) up
#1  0x00007fff94dc17d2 in pthread_kill ()
(gdb) up
#2  0x00007fff94db2a7a in abort ()
(gdb) up
#3  0x0000000100da28c3 in g_logv ()
(gdb) up
#4  0x0000000100da247e in g_log ()
(gdb) up
#5  0x0000000100d74333 in g_thread_init ()
(gdb) up
#6  0x00000001000e6d8c in qemu_main (argc=3, argv=0x10a896710, envp=0x7fff5fbff120) at /Users/juan/Temp/build/qemu-source_code/vl.c:2188
2188	    g_thread_init(NULL);
Current language:  auto; currently c
(gdb) 



On Oct 31, 2011, at 8:32 AM, Juan Pineda wrote:

> Hi Andreas,
> 
> Thanks for your reply. You can verify that my cocoa.m patch works 100% under 0.15.1. That's where I was testing it. The fix is completely independent of the other problems in the OSX build (uint16, GThread, etc.) that have cropped up since 0.15.1, so there should be no problem merging it.
> 
> I will us GDB on that GThread error to find the source code line where it failed and post what I find.
> 
> Thanks!
> -Juan
> 
> 
> 
> On Oct 30, 2011, at 3:45 PM, Andreas Färber wrote:
> 
>> Am 18.10.2011 16:35, schrieb Juan Pineda:
>>>>> What command line do you use? If using the right arguments you
>>>>> shouldn't see a window at all. Are you maybe using -drive instead of
>>>>> -hda and that is not yet handled correctly?
>>> 
>>> The boot volume dialog appears only when a hard disk file is not supplied on the command line. I would think the failure of the dialog to close is not unique to Lion. However in prior OS releases it probably does not present a problem as the main window can be raised above the dialog.
>> 
>> If that were the case I would see it as worth fixing there, too. :)
>> 
>>>>> Apart from this issue, is it working correctly for you?
>> 
>>> However building the main branch (with flags as above) does not work. The build fails with error about redefinition of uint16 in fpu/softfloat.h. If that is fixed the build completes. But trying to run qemu-system-i386 quits:
>>> 
>>>> $ i386-softmmu/qemu-system-i386 -hda ~/Downloads/linux-0.2.img
>>>> 
>>>> GThread-ERROR **: GThread system may only be initialized once.
>>>> aborting...
>>>> Abort trap: 6
>> 
>> That's what I'm seeing, too, so I don't feel comfortable committing this
>> patch now. It compiles file, but it crashes either way with a message
>> similar to the above, so I don't see an improvement (yet).
>> 
>> If you could pinpoint where this error stems from (e.g., single-stepping
>> in gdb) that might help getting it fixed.
>> 
>> Thanks,
>> Andreas
> 

Re: [Qemu-devel] Problem under OSX Lion: GThread-ERROR **: GThread system may only be initialized once

[Daniel P. Berrange]

On Mon, Oct 31, 2011 at 09:25:26AM -0700, Juan Pineda wrote:
> The current Qemu master branch crashes under OSX with a GThread error.
> Commenting out vlc.c line 2188 (call to g_thread_init) allows it to run
> successfully.

As a general rule all calls to 'g_thread_init(NULL)' should be protected
by a conditional thus:

        if (!g_thread_supported())
            g_thread_init(NULL);

Even though QEMU calls g_thread_init() very early in main(), there is
always the possibility that some library QEMU links to, has got an ELF
initializer, triggering before main(), which calls g_thread_init().

NB on GLib versions >= 2.24, g_thread_init() is a no-op if called more
than once[1], but for sake of compatibility QEMU ought to use the conditonal
check.

Regards,
Daniel

[1] The reason for this change is that GObject itself actually calls
    g_thread_init() secretly too.
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] Problem under OSX Lion: GThread-ERROR **: GThread system may only be initialized once

[Andreas Färber]

Am 31.10.2011 17:36, schrieb Daniel P. Berrange:
> On Mon, Oct 31, 2011 at 09:25:26AM -0700, Juan Pineda wrote:
>> The current Qemu master branch crashes under OSX with a GThread error.
>> Commenting out vlc.c line 2188 (call to g_thread_init) allows it to run
>> successfully.
> 
> As a general rule all calls to 'g_thread_init(NULL)' should be protected
> by a conditional thus:
> 
>         if (!g_thread_supported())
>             g_thread_init(NULL);
> 
> Even though QEMU calls g_thread_init() very early in main(), there is
> always the possibility that some library QEMU links to, has got an ELF
> initializer, triggering before main(), which calls g_thread_init().

Thanks to both of you! I checked: vl.c is the only use without such
condition. So it seems this regression was introduced through
coroutine-gthread.c and is not specific to Darwin. I'll post a patch
shortly.

Andreas