|
From: | Akihiko Odaki |
Subject: | Re: [RFC PATCH-for-7.0 v4 0/2] cocoa: run qemu_init in the main thread |
Date: | Sat, 19 Mar 2022 23:15:48 +0900 |
User-agent: | Mozilla/5.0 (X11; Linux aarch64; rv:91.0) Gecko/20100101 Thunderbird/91.7.0 |
On 2022/03/19 22:56, Philippe Mathieu-Daudé wrote:
Hi Akihiko, Paolo, Peter. On 17/3/22 13:55, Philippe Mathieu-Daudé wrote:From: Philippe Mathieu-Daudé <f4bug@amsat.org> Posting v4 in case someone want to iterate. Pending issue raised by Akihiko Odaki: * this actually breaks the "runas" option with ui/cocoa. [+NSApplication sharedApplication] calls issetugid() to see if setgid() or setuid() is called before and calls exit() if it evaluates true. It does not evaluate true without this patch since setgid() and setuid() are called after [+NSApplication sharedApplication]. This patch, however, changes the order and triggers the check. There are two options to solve the problem: 1. Move setgid and setuid calls after [+NSApplication sharedApplication] to let NSApplication initialize as the original user.Akihiko, could you send a patch?
Do you mean a patch for option 1?
2. Do: [[NSUserDefaults standardUserDefaults] setBool:YES forKey:@"_NSAppAllowsNonTrustedUGID"] Option 2 would be preferred in terms of practicality since nobody would want to initialize NSApplication as the original user (usually superuser). However, _NSAppAllowsNonTrustedUGID is not documented by Apple.What are your views on this problem for 7.0-rc1? Keep modifying cocoa UI? Disable block layer assertions? Only disable them for Darwin?
I think we should disable block layer assertions. It is not a change visible to user and its value is more apparent in development. We can preserve most of its benefit if we restore the assertions immediately after 7.0 release and let them work during the next development cycle.
If it is not preferred, we can apply the following change as a less-intrusive alternative:
20220307134946.61407-1-akihiko.odaki@gmail.com/">https://patchew.org/QEMU/20220307134946.61407-1-akihiko.odaki@gmail.com/The change faced objections as it uses Cocoa APIs from iothread. It is still in accordance with Cocoa's API convention and the only negative effect is that it can confuse developers. It is not affecting users and we can also minimize the possibility of confusion by immediately following with this "qemu_init in the main thread" patch after 7.0 release.
Regards, Akihiko Odaki
* Oudated comment in main(): 1970 /*1971 * Create the menu entries which depend on QEMU state (for consoles 1972 * and removeable devices). These make calls back into QEMU functions, 1973 * which is OK because at this point we know that the second thread1974 * holds the iothread lock and is synchronously waiting for us to 1975 * finish. 1976 */ (https://marc.info/?l=qemu-devel&m=164752136410805) Since v3: - Move qemu_event_init before cbowner alloc - Reduce main_thread scope to applicationDidFinishLaunching - Updated updateUIInfo() comment (s/cocoa_display_init/applicationDidFinishLaunching) Since v2: - Extracted code movement in preliminary patchv3: 20220317115644.37276-1-philippe.mathieu.daude@gmail.com/">https://lore.kernel.org/qemu-devel/20220317115644.37276-1-philippe.mathieu.daude@gmail.com/ v2: 20220316160300.85438-1-philippe.mathieu.daude@gmail.com/">https://lore.kernel.org/qemu-devel/20220316160300.85438-1-philippe.mathieu.daude@gmail.com/ v1: 20220307151004.578069-1-pbonzini@redhat.com/">https://lore.kernel.org/qemu-devel/20220307151004.578069-1-pbonzini@redhat.com/Paolo Bonzini (1): ui/cocoa: run qemu_init in the main thread Philippe Mathieu-Daudé (1): ui/cocoa: Code movement softmmu/main.c | 12 ++-- ui/cocoa.m | 161 ++++++++++++++++++++++--------------------------- 2 files changed, 79 insertions(+), 94 deletions(-)
[Prev in Thread] | Current Thread | [Next in Thread] |