qemu-devel
[Top][All Lists]
Advanced

[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: Anthony Liguori
Subject: Re: [Qemu-devel] [RFC next] ui: Split main() in two to not have Cocoa hijack it
Date: Wed, 30 May 2012 15:11:35 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20120329 Thunderbird/11.0.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<address@hidden>
Cc: Anthony Liguori<address@hidden>
---
  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");




reply via email to

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