qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 1/3] Modular command line options
@ 2008-05-20 23:51 Jan Kiszka
  2008-05-21 19:17 ` Glauber Costa
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Jan Kiszka @ 2008-05-20 23:51 UTC (permalink / raw)
  To: qemu-devel

Following up on my earlier proposal to introduce per-machine command
line options, this version provides a more generic approach. It should
also be usable for scenarios like per-arch or per-accelerator.

To summarize the approach: Command line options are now organized in
sets. Each set consists of a list of options, a help text, and a pointer
the a parse handler that can process the options. The base option set of
QEMU is automatically registered, more sets can be added via
qemu_register_option_set().

The patch includes quite some refactoring in order to move the basic
switches over the new scheme. So you may have a look at the data
structures in qemu-common.h, but otherwise it is better to apply it and
then study vl.c. More refactoring of existing switches is likely
feasible, but first this needs and an OK from (or merging by) the
maintainers.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 qemu-common.h |   19 
 vl.c          | 1690 +++++++++++++++++++++++++++++-----------------------------
 2 files changed, 867 insertions(+), 842 deletions(-)

Index: b/qemu-common.h
===================================================================
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -132,4 +132,23 @@ typedef struct SerialState SerialState;
 typedef struct IRQState *qemu_irq;
 struct pcmcia_card_s;
 
+#define HAS_ARG 0x0001
+
+typedef struct QEMUOption {
+    const char *name;
+    int flags;
+    int index;
+} QEMUOption;
+
+typedef void QEMUOptionParser(int index, const char *optarg);
+
+typedef struct QEMUOptionSet {
+    const QEMUOption *options;
+    const char *help_string;
+    QEMUOptionParser *parse_handler;
+    struct QEMUOptionSet *next;
+} QEMUOptionSet;
+
+void qemu_register_option_set(QEMUOptionSet *option_set);
+
 #endif
Index: b/vl.c
===================================================================
--- a/vl.c
+++ b/vl.c
@@ -143,9 +143,11 @@ int inet_aton(const char *cp, struct in_
 //#define DEBUG_IOPORT
 
 #ifdef TARGET_PPC
-#define DEFAULT_RAM_SIZE 144
+#define DEFAULT_RAM_SIZE        144
+#define DEFAULT_RAM_SIZE_STR    "144"
 #else
-#define DEFAULT_RAM_SIZE 128
+#define DEFAULT_RAM_SIZE        128
+#define DEFAULT_RAM_SIZE_STR    "128"
 #endif
 /* in ms */
 #define GUI_REFRESH_INTERVAL 30
@@ -156,6 +158,7 @@ int inet_aton(const char *cp, struct in_
 /* XXX: use a two level table to limit memory usage */
 #define MAX_IOPORTS 65536
 
+const char *qemu_bin_name;
 const char *bios_dir = CONFIG_QEMU_SHAREDIR;
 const char *bios_name = NULL;
 void *ioport_opaque[MAX_IOPORTS];
@@ -240,6 +243,35 @@ static CPUState *cur_cpu;
 static CPUState *next_cpu;
 static int event_pending = 1;
 
+/* command line parameters */
+static QEMUMachine *machine;
+static const char *cpu_model;
+static const char *initrd_filename;
+static const char *kernel_filename;
+static const char *kernel_cmdline = "";
+static const char *boot_devices = "";
+static int hda_cyls, hda_heads, hda_secs;
+static int hda_translation = BIOS_ATA_TRANSLATION_AUTO;
+static int hda_index = -1;
+static int snapshot;
+static const char *serial_devices[MAX_SERIAL_PORTS];
+static int serial_device_index;
+static const char *parallel_devices[MAX_PARALLEL_PORTS];
+static int parallel_device_index;
+static const char *monitor_device;
+static uint32_t boot_devices_bitmap;
+#define MAX_NET_CLIENTS 32
+static const char *net_clients[MAX_NET_CLIENTS];
+static int nb_net_clients;
+static const char *usb_devices[MAX_USB_CMDLINE];
+static int usb_devices_index;
+static const char *loadvm;
+static const char *pid_file;
+#ifdef CONFIG_GDBSTUB
+static int use_gdbstub;
+static const char *gdbstub_port = DEFAULT_GDBSTUB_PORT;
+#endif
+
 #define TFR(expr) do { if ((expr) != -1) break; } while (errno == EINTR)
 
 /***********************************************************/
@@ -5004,7 +5036,7 @@ static void bdrv_format_print(void *opaq
     fprintf(stderr, " %s", name);
 }
 
-static int drive_init(struct drive_opt *arg, int snapshot,
+static int drive_init(struct drive_opt *arg, int use_snapshot,
                       QEMUMachine *machine)
 {
     char buf[128];
@@ -5172,9 +5204,9 @@ static int drive_init(struct drive_opt *
 
     if (get_param_value(buf, sizeof(buf), "snapshot", str)) {
         if (!strcmp(buf, "on"))
-	    snapshot = 1;
+	    use_snapshot = 1;
         else if (!strcmp(buf, "off"))
-	    snapshot = 0;
+	    use_snapshot = 0;
 	else {
 	    fprintf(stderr, "qemu: '%s' invalid snapshot option\n", str);
 	    return -1;
@@ -5304,7 +5336,7 @@ static int drive_init(struct drive_opt *
     if (!file[0])
         return 0;
     bdrv_flags = 0;
-    if (snapshot)
+    if (use_snapshot)
         bdrv_flags |= BDRV_O_SNAPSHOT;
     if (!cache)
         bdrv_flags |= BDRV_O_DIRECT;
@@ -7139,162 +7171,177 @@ static int main_loop(void)
     return ret;
 }
 
-static void help(int exitcode)
+/* password input */
+
+int qemu_key_check(BlockDriverState *bs, const char *name)
 {
-    printf("QEMU PC emulator version " QEMU_VERSION ", Copyright (c) 2003-2008 Fabrice Bellard\n"
-           "usage: %s [options] [disk_image]\n"
-           "\n"
-           "'disk_image' is a raw hard image image for IDE hard disk 0\n"
-           "\n"
-           "Standard options:\n"
-           "-M machine      select emulated machine (-M ? for list)\n"
-           "-cpu cpu        select CPU (-cpu ? for list)\n"
-           "-fda/-fdb file  use 'file' as floppy disk 0/1 image\n"
-           "-hda/-hdb file  use 'file' as IDE hard disk 0/1 image\n"
-           "-hdc/-hdd file  use 'file' as IDE hard disk 2/3 image\n"
-           "-cdrom file     use 'file' as IDE cdrom image (cdrom is ide1 master)\n"
-	   "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n"
-           "       [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n"
-           "       [,cache=on|off][,format=f]\n"
-	   "                use 'file' as a drive image\n"
-           "-mtdblock file  use 'file' as on-board Flash memory image\n"
-           "-sd file        use 'file' as SecureDigital card image\n"
-           "-pflash file    use 'file' as a parallel flash image\n"
-           "-boot [a|c|d|n] boot on floppy (a), hard disk (c), CD-ROM (d), or network (n)\n"
-           "-snapshot       write to temporary files instead of disk image files\n"
-#ifdef CONFIG_SDL
-           "-no-frame       open SDL window without a frame and window decorations\n"
-           "-alt-grab       use Ctrl-Alt-Shift to grab mouse (instead of Ctrl-Alt)\n"
-           "-no-quit        disable SDL window close capability\n"
-#endif
-#ifdef TARGET_I386
-           "-no-fd-bootchk  disable boot signature checking for floppy disks\n"
-#endif
-           "-m megs         set virtual RAM size to megs MB [default=%d]\n"
-           "-smp n          set the number of CPUs to 'n' [default=1]\n"
-           "-nographic      disable graphical output and redirect serial I/Os to console\n"
-           "-portrait       rotate graphical output 90 deg left (only PXA LCD)\n"
-#ifndef _WIN32
-           "-k language     use keyboard layout (for example \"fr\" for French)\n"
-#endif
+    char password[256];
+    int i;
+
+    if (!bdrv_is_encrypted(bs))
+        return 0;
+
+    term_printf("%s is encrypted.\n", name);
+    for(i = 0; i < 3; i++) {
+        monitor_readline("Password: ", 1, password, sizeof(password));
+        if (bdrv_set_key(bs, password) == 0)
+            return 0;
+        term_printf("invalid password\n");
+    }
+    return -EPERM;
+}
+
+static BlockDriverState *get_bdrv(int index)
+{
+    if (index > nb_drives)
+        return NULL;
+    return drives_table[index].bdrv;
+}
+
+static void read_passwords(void)
+{
+    BlockDriverState *bs;
+    int i;
+
+    for(i = 0; i < 6; i++) {
+        bs = get_bdrv(i);
+        if (bs)
+            qemu_key_check(bs, bdrv_get_device_name(bs));
+    }
+}
+
 #ifdef HAS_AUDIO
-           "-audio-help     print list of audio drivers and their options\n"
-           "-soundhw c1,... enable audio support\n"
-           "                and only specified sound cards (comma separated list)\n"
-           "                use -soundhw ? to get the list of supported cards\n"
-           "                use -soundhw all to enable all of them\n"
-#endif
-           "-localtime      set the real time clock to local time [default=utc]\n"
-           "-full-screen    start in full screen\n"
-#ifdef TARGET_I386
-           "-win2k-hack     use it when installing Windows 2000 to avoid a disk full bug\n"
-#endif
-           "-usb            enable the USB driver (will be the default soon)\n"
-           "-usbdevice name add the host or guest USB device 'name'\n"
-#if defined(TARGET_PPC) || defined(TARGET_SPARC)
-           "-g WxH[xDEPTH]  Set the initial graphical resolution and depth\n"
-#endif
-           "-name string    set the name of the guest\n"
-           "\n"
-           "Network options:\n"
-           "-net nic[,vlan=n][,macaddr=addr][,model=type]\n"
-           "                create a new Network Interface Card and connect it to VLAN 'n'\n"
-#ifdef CONFIG_SLIRP
-           "-net user[,vlan=n][,hostname=host]\n"
-           "                connect the user mode network stack to VLAN 'n' and send\n"
-           "                hostname 'host' to DHCP clients\n"
+struct soundhw soundhw[] = {
+#ifdef HAS_AUDIO_CHOICE
+#if defined(TARGET_I386) || defined(TARGET_MIPS)
+    {
+        "pcspk",
+        "PC speaker",
+        0,
+        1,
+        { .init_isa = pcspk_audio_init }
+    },
 #endif
-#ifdef _WIN32
-           "-net tap[,vlan=n],ifname=name\n"
-           "                connect the host TAP network interface to VLAN 'n'\n"
+    {
+        "sb16",
+        "Creative Sound Blaster 16",
+        0,
+        1,
+        { .init_isa = SB16_init }
+    },
+
+#ifdef CONFIG_ADLIB
+    {
+        "adlib",
+#ifdef HAS_YMF262
+        "Yamaha YMF262 (OPL3)",
 #else
-           "-net tap[,vlan=n][,fd=h][,ifname=name][,script=file][,downscript=dfile]\n"
-           "                connect the host TAP network interface to VLAN 'n' and use the\n"
-           "                network scripts 'file' (default=%s)\n"
-           "                and 'dfile' (default=%s);\n"
-           "                use '[down]script=no' to disable script execution;\n"
-           "                use 'fd=h' to connect to an already opened TAP interface\n"
-#endif
-           "-net socket[,vlan=n][,fd=h][,listen=[host]:port][,connect=host:port]\n"
-           "                connect the vlan 'n' to another VLAN using a socket connection\n"
-           "-net socket[,vlan=n][,fd=h][,mcast=maddr:port]\n"
-           "                connect the vlan 'n' to multicast maddr and port\n"
-           "-net none       use it alone to have zero network devices; if no -net option\n"
-           "                is provided, the default is '-net nic -net user'\n"
-           "\n"
-#ifdef CONFIG_SLIRP
-           "-tftp dir       allow tftp access to files in dir [-net user]\n"
-           "-bootp file     advertise file in BOOTP replies\n"
-#ifndef _WIN32
-           "-smb dir        allow SMB access to files in 'dir' [-net user]\n"
-#endif
-           "-redir [tcp|udp]:host-port:[guest-host]:guest-port\n"
-           "                redirect TCP or UDP connections from host to guest [-net user]\n"
-#endif
-           "\n"
-           "Linux boot specific:\n"
-           "-kernel bzImage use 'bzImage' as kernel image\n"
-           "-append cmdline use 'cmdline' as kernel command line\n"
-           "-initrd file    use 'file' as initial ram disk\n"
-           "\n"
-           "Debug/Expert options:\n"
-           "-monitor dev    redirect the monitor to char device 'dev'\n"
-           "-serial dev     redirect the serial port to char device 'dev'\n"
-           "-parallel dev   redirect the parallel port to char device 'dev'\n"
-           "-pidfile file   Write PID to 'file'\n"
-           "-S              freeze CPU at startup (use 'c' to start execution)\n"
-           "-s              wait gdb connection to port\n"
-           "-p port         set gdb connection port [default=%s]\n"
-           "-d item1,...    output log to %s (use -d ? for a list of log items)\n"
-           "-hdachs c,h,s[,t]  force hard disk 0 physical geometry and the optional BIOS\n"
-           "                translation (t=none or lba) (usually qemu can guess them)\n"
-           "-L path         set the directory for the BIOS, VGA BIOS and keymaps\n"
-#ifdef USE_KQEMU
-           "-kernel-kqemu   enable KQEMU full virtualization (default is user mode only)\n"
-           "-no-kqemu       disable KQEMU kernel module usage\n"
-#endif
-#ifdef TARGET_I386
-           "-std-vga        simulate a standard VGA card with VESA Bochs Extensions\n"
-           "                (default is CL-GD5446 PCI VGA)\n"
-           "-no-acpi        disable ACPI\n"
+        "Yamaha YM3812 (OPL2)",
 #endif
-#ifdef CONFIG_CURSES
-           "-curses         use a curses/ncurses interface instead of SDL\n"
+        0,
+        1,
+        { .init_isa = Adlib_init }
+    },
 #endif
-           "-no-reboot      exit instead of rebooting\n"
-           "-no-shutdown    stop before shutdown\n"
-           "-loadvm [tag|id]  start right away with a saved state (loadvm in monitor)\n"
-	   "-vnc display    start a VNC server on display\n"
-#ifndef _WIN32
-	   "-daemonize      daemonize QEMU after initializing\n"
+
+#ifdef CONFIG_GUS
+    {
+        "gus",
+        "Gravis Ultrasound GF1",
+        0,
+        1,
+        { .init_isa = GUS_init }
+    },
 #endif
-	   "-option-rom rom load a file, rom, into the option ROM space\n"
-#ifdef TARGET_SPARC
-           "-prom-env variable=value  set OpenBIOS nvram variables\n"
+
+#ifdef CONFIG_AC97
+    {
+        "ac97",
+        "Intel 82801AA AC97 Audio",
+        0,
+        0,
+        { .init_pci = ac97_init }
+    },
 #endif
-           "-clock          force the use of the given methods for timer alarm.\n"
-           "                To see what timers are available use -clock ?\n"
-           "-startdate      select initial date of the clock\n"
-           "\n"
-           "During emulation, the following keys are useful:\n"
-           "ctrl-alt-f      toggle full screen\n"
-           "ctrl-alt-n      switch to virtual console 'n'\n"
-           "ctrl-alt        toggle mouse and keyboard grab\n"
-           "\n"
-           "When using -nographic, press 'ctrl-a h' to get some help.\n"
-           ,
-           "qemu",
-           DEFAULT_RAM_SIZE,
-#ifndef _WIN32
-           DEFAULT_NETWORK_SCRIPT,
-           DEFAULT_NETWORK_DOWN_SCRIPT,
+
+    {
+        "es1370",
+        "ENSONIQ AudioPCI ES1370",
+        0,
+        0,
+        { .init_pci = es1370_init }
+    },
 #endif
-           DEFAULT_GDBSTUB_PORT,
-           "/tmp/qemu.log");
-    exit(exitcode);
+
+    { NULL, NULL, 0, 0, { NULL } }
+};
+
+static void select_soundhw (const char *optarg)
+{
+    struct soundhw *c;
+
+    if (*optarg == '?') {
+    show_valid_cards:
+
+        printf ("Valid sound card names (comma separated):\n");
+        for (c = soundhw; c->name; ++c) {
+            printf ("%-11s %s\n", c->name, c->descr);
+        }
+        printf ("\n-soundhw all will enable all of the above\n");
+        exit (*optarg != '?');
+    }
+    else {
+        size_t l;
+        const char *p;
+        char *e;
+        int bad_card = 0;
+
+        if (!strcmp (optarg, "all")) {
+            for (c = soundhw; c->name; ++c) {
+                c->enabled = 1;
+            }
+            return;
+        }
+
+        p = optarg;
+        while (*p) {
+            e = strchr (p, ',');
+            l = !e ? strlen (p) : (size_t) (e - p);
+
+            for (c = soundhw; c->name; ++c) {
+                if (!strncmp (c->name, p, l)) {
+                    c->enabled = 1;
+                    break;
+                }
+            }
+
+            if (!c->name) {
+                if (l > 80) {
+                    fprintf (stderr,
+                             "Unknown sound card name (too big to show)\n");
+                }
+                else {
+                    fprintf (stderr, "Unknown sound card name `%.*s'\n",
+                             (int) l, p);
+                }
+                bad_card = 1;
+            }
+            p += l + (e != NULL);
+        }
+
+        if (bad_card)
+            goto show_valid_cards;
+    }
 }
+#endif
 
-#define HAS_ARG 0x0001
+#ifdef _WIN32
+static BOOL WINAPI qemu_ctrl_handler(DWORD type)
+{
+    exit(STATUS_CONTROL_C_EXIT);
+    return TRUE;
+}
+#endif
 
 enum {
     QEMU_OPTION_h,
@@ -7380,13 +7427,7 @@ enum {
     QEMU_OPTION_startdate,
 };
 
-typedef struct QEMUOption {
-    const char *name;
-    int flags;
-    int index;
-} QEMUOption;
-
-const QEMUOption qemu_options[] = {
+static const QEMUOption qemu_base_options[] = {
     { "h", 0, QEMU_OPTION_h },
     { "help", 0, QEMU_OPTION_h },
 
@@ -7492,212 +7533,681 @@ const QEMUOption qemu_options[] = {
     { NULL },
 };
 
-/* password input */
+static void qemu_parse_base_options(int index, const char *optarg);
 
-int qemu_key_check(BlockDriverState *bs, const char *name)
-{
-    char password[256];
-    int i;
+static QEMUOptionSet qemu_options = {
+    .options = qemu_base_options,
+    .help_string =
+        "Standard options:\n"
+        "-M machine      select emulated machine (-M ? for list)\n"
+        "-cpu cpu        select CPU (-cpu ? for list)\n"
+        "-fda/-fdb file  use 'file' as floppy disk 0/1 image\n"
+        "-hda/-hdb file  use 'file' as IDE hard disk 0/1 image\n"
+        "-hdc/-hdd file  use 'file' as IDE hard disk 2/3 image\n"
+        "-cdrom file     use 'file' as IDE cdrom image (cdrom is ide1 master)\n"
+        "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n"
+        "       [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n"
+        "       [,cache=on|off][,format=f]\n"
+        "                use 'file' as a drive image\n"
+        "-mtdblock file  use 'file' as on-board Flash memory image\n"
+        "-sd file        use 'file' as SecureDigital card image\n"
+        "-pflash file    use 'file' as a parallel flash image\n"
+        "-boot [a|c|d|n] boot on floppy (a), hard disk (c), CD-ROM (d), or network (n)\n"
+        "-snapshot       write to temporary files instead of disk image files\n"
+#ifdef CONFIG_SDL
+        "-no-frame       open SDL window without a frame and window decorations\n"
+        "-alt-grab       use Ctrl-Alt-Shift to grab mouse (instead of Ctrl-Alt)\n"
+        "-no-quit        disable SDL window close capability\n"
+#endif
+#ifdef TARGET_I386
+        "-no-fd-bootchk  disable boot signature checking for floppy disks\n"
+#endif
+        "-m megs         set virtual RAM size to megs MB [default=" DEFAULT_RAM_SIZE_STR "]\n"
+        "-smp n          set the number of CPUs to 'n' [default=1]\n"
+        "-nographic      disable graphical output and redirect serial I/Os to console\n"
+        "-portrait       rotate graphical output 90 deg left (only PXA LCD)\n"
+#ifndef _WIN32
+        "-k language     use keyboard layout (for example \"fr\" for French)\n"
+#endif
+#ifdef HAS_AUDIO
+        "-audio-help     print list of audio drivers and their options\n"
+        "-soundhw c1,... enable audio support\n"
+        "                and only specified sound cards (comma separated list)\n"
+        "                use -soundhw ? to get the list of supported cards\n"
+        "                use -soundhw all to enable all of them\n"
+#endif
+        "-localtime      set the real time clock to local time [default=utc]\n"
+        "-full-screen    start in full screen\n"
+#ifdef TARGET_I386
+        "-win2k-hack     use it when installing Windows 2000 to avoid a disk full bug\n"
+#endif
+        "-usb            enable the USB driver (will be the default soon)\n"
+        "-usbdevice name add the host or guest USB device 'name'\n"
+#if defined(TARGET_PPC) || defined(TARGET_SPARC)
+        "-g WxH[xDEPTH]  Set the initial graphical resolution and depth\n"
+#endif
+        "-name string    set the name of the guest\n"
+        "\n"
+        "Network options:\n"
+        "-net nic[,vlan=n][,macaddr=addr][,model=type]\n"
+        "                create a new Network Interface Card and connect it to VLAN 'n'\n"
+#ifdef CONFIG_SLIRP
+        "-net user[,vlan=n][,hostname=host]\n"
+        "                connect the user mode network stack to VLAN 'n' and send\n"
+        "                hostname 'host' to DHCP clients\n"
+#endif
+#ifdef _WIN32
+        "-net tap[,vlan=n],ifname=name\n"
+        "                connect the host TAP network interface to VLAN 'n'\n"
+#else
+        "-net tap[,vlan=n][,fd=h][,ifname=name][,script=file][,downscript=dfile]\n"
+        "                connect the host TAP network interface to VLAN 'n' and use the\n"
+        "                network scripts 'file' (default=" DEFAULT_NETWORK_SCRIPT ")\n"
+        "                and 'dfile' (default=" DEFAULT_NETWORK_DOWN_SCRIPT ");\n"
+        "                use '[down]script=no' to disable script execution;\n"
+        "                use 'fd=h' to connect to an already opened TAP interface\n"
+#endif
+        "-net socket[,vlan=n][,fd=h][,listen=[host]:port][,connect=host:port]\n"
+        "                connect the vlan 'n' to another VLAN using a socket connection\n"
+        "-net socket[,vlan=n][,fd=h][,mcast=maddr:port]\n"
+        "                connect the vlan 'n' to multicast maddr and port\n"
+        "-net none       use it alone to have zero network devices; if no -net option\n"
+        "                is provided, the default is '-net nic -net user'\n"
+        "\n"
+#ifdef CONFIG_SLIRP
+        "-tftp dir       allow tftp access to files in dir [-net user]\n"
+        "-bootp file     advertise file in BOOTP replies\n"
+#ifndef _WIN32
+        "-smb dir        allow SMB access to files in 'dir' [-net user]\n"
+#endif
+        "-redir [tcp|udp]:host-port:[guest-host]:guest-port\n"
+        "                redirect TCP or UDP connections from host to guest [-net user]\n"
+#endif
+        "\n"
+        "Linux boot specific:\n"
+        "-kernel bzImage use 'bzImage' as kernel image\n"
+        "-append cmdline use 'cmdline' as kernel command line\n"
+        "-initrd file    use 'file' as initial ram disk\n"
+        "\n"
+        "Debug/Expert options:\n"
+        "-monitor dev    redirect the monitor to char device 'dev'\n"
+        "-serial dev     redirect the serial port to char device 'dev'\n"
+        "-parallel dev   redirect the parallel port to char device 'dev'\n"
+        "-pidfile file   Write PID to 'file'\n"
+        "-S              freeze CPU at startup (use 'c' to start execution)\n"
+        "-s              wait gdb connection to port\n"
+        "-p port         set gdb connection port [default=" DEFAULT_GDBSTUB_PORT "]\n"
+        "-d item1,...    output log to /tmp/qemu.log (use -d ? for a list of log items)\n"
+        "-hdachs c,h,s[,t]  force hard disk 0 physical geometry and the optional BIOS\n"
+        "                translation (t=none or lba) (usually qemu can guess them)\n"
+        "-L path         set the directory for the BIOS, VGA BIOS and keymaps\n"
+#ifdef USE_KQEMU
+        "-kernel-kqemu   enable KQEMU full virtualization (default is user mode only)\n"
+        "-no-kqemu       disable KQEMU kernel module usage\n"
+#endif
+#ifdef TARGET_I386
+        "-std-vga        simulate a standard VGA card with VESA Bochs Extensions\n"
+        "                (default is CL-GD5446 PCI VGA)\n"
+        "-no-acpi        disable ACPI\n"
+#endif
+#ifdef CONFIG_CURSES
+        "-curses         use a curses/ncurses interface instead of SDL\n"
+#endif
+        "-no-reboot      exit instead of rebooting\n"
+        "-no-shutdown    stop before shutdown\n"
+        "-loadvm [tag|id]  start right away with a saved state (loadvm in monitor)\n"
+        "-vnc display    start a VNC server on display\n"
+#ifndef _WIN32
+        "-daemonize      daemonize QEMU after initializing\n"
+#endif
+        "-option-rom rom load a file, rom, into the option ROM space\n"
+#ifdef TARGET_SPARC
+        "-prom-env variable=value  set OpenBIOS nvram variables\n"
+#endif
+        "-clock          force the use of the given methods for timer alarm.\n"
+        "                To see what timers are available use -clock ?\n"
+        "-startdate      select initial date of the clock\n"
+        "\n",
+    .parse_handler = qemu_parse_base_options,
+    .next = NULL
+};
 
-    if (!bdrv_is_encrypted(bs))
-        return 0;
+static void help(int exitcode)
+{
+    QEMUOptionSet *optset = &qemu_options;
 
-    term_printf("%s is encrypted.\n", name);
-    for(i = 0; i < 3; i++) {
-        monitor_readline("Password: ", 1, password, sizeof(password));
-        if (bdrv_set_key(bs, password) == 0)
-            return 0;
-        term_printf("invalid password\n");
+    printf("QEMU PC emulator version " QEMU_VERSION ", Copyright (c) 2003-2008 Fabrice Bellard\n"
+           "usage: %s [options] [disk_image]\n"
+           "\n"
+           "'disk_image' is a raw hard image image for IDE hard disk 0\n"
+           "\n",
+           qemu_bin_name);
+    while (optset) {
+        printf(optset->help_string);
+        optset = optset->next;
     }
-    return -EPERM;
-}
-
-static BlockDriverState *get_bdrv(int index)
-{
-    if (index > nb_drives)
-        return NULL;
-    return drives_table[index].bdrv;
+    printf("During emulation, the following keys are useful:\n"
+           "ctrl-alt-f      toggle full screen\n"
+           "ctrl-alt-n      switch to virtual console 'n'\n"
+           "ctrl-alt        toggle mouse and keyboard grab\n"
+           "\n"
+           "When using -nographic, press 'ctrl-a h' to get some help.\n");
+    exit(exitcode);
 }
 
-static void read_passwords(void)
+static void qemu_parse_base_options(int index, const char *optarg)
 {
-    BlockDriverState *bs;
-    int i;
-
-    for(i = 0; i < 6; i++) {
-        bs = get_bdrv(i);
-        if (bs)
-            qemu_key_check(bs, bdrv_get_device_name(bs));
-    }
-}
+    const char *p;
 
-#ifdef HAS_AUDIO
-struct soundhw soundhw[] = {
-#ifdef HAS_AUDIO_CHOICE
-#if defined(TARGET_I386) || defined(TARGET_MIPS)
-    {
-        "pcspk",
-        "PC speaker",
-        0,
-        1,
-        { .init_isa = pcspk_audio_init }
-    },
+    switch(index) {
+    case QEMU_OPTION_M:
+        machine = find_machine(optarg);
+        if (!machine) {
+            QEMUMachine *m;
+            printf("Supported machines are:\n");
+            for(m = first_machine; m != NULL; m = m->next) {
+                printf("%-10s %s%s\n",
+                    m->name, m->desc,
+                    m == first_machine ? " (default)" : "");
+            }
+            exit(*optarg != '?');
+        }
+        break;
+    case QEMU_OPTION_cpu:
+        /* hw initialization will check this */
+        if (*optarg == '?') {
+/* XXX: implement xxx_cpu_list for targets that still miss it */
+#if defined(cpu_list)
+            cpu_list(stdout, &fprintf);
 #endif
-    {
-        "sb16",
-        "Creative Sound Blaster 16",
-        0,
-        1,
-        { .init_isa = SB16_init }
-    },
-
-#ifdef CONFIG_ADLIB
-    {
-        "adlib",
-#ifdef HAS_YMF262
-        "Yamaha YMF262 (OPL3)",
-#else
-        "Yamaha YM3812 (OPL2)",
+            exit(0);
+        } else {
+            cpu_model = optarg;
+        }
+        break;
+    case QEMU_OPTION_initrd:
+        initrd_filename = optarg;
+        break;
+    case QEMU_OPTION_hda:
+        if (hda_cyls == 0)
+            hda_index = drive_add(optarg, HD_ALIAS, 0);
+        else
+            hda_index =
+                drive_add(optarg, HD_ALIAS ",cyls=%d,heads=%d,secs=%d%s",
+                          0, hda_cyls, hda_heads, hda_secs,
+                          hda_translation == BIOS_ATA_TRANSLATION_LBA ?
+                              ",trans=lba" :
+                                  hda_translation == BIOS_ATA_TRANSLATION_NONE ?
+                                      ",trans=none" : "");
+        break;
+    case QEMU_OPTION_hdb:
+    case QEMU_OPTION_hdc:
+    case QEMU_OPTION_hdd:
+        drive_add(optarg, HD_ALIAS, index - QEMU_OPTION_hda);
+        break;
+    case QEMU_OPTION_drive:
+        drive_add(NULL, "%s", optarg);
+        break;
+    case QEMU_OPTION_mtdblock:
+        drive_add(optarg, MTD_ALIAS);
+        break;
+    case QEMU_OPTION_sd:
+        drive_add(optarg, SD_ALIAS);
+        break;
+    case QEMU_OPTION_pflash:
+        drive_add(optarg, PFLASH_ALIAS);
+        break;
+    case QEMU_OPTION_snapshot:
+        snapshot = 1;
+        break;
+    case QEMU_OPTION_hdachs:
+        p = optarg;
+        hda_cyls = strtol(p, (char **)&p, 0);
+        if (hda_cyls < 1 || hda_cyls > 16383)
+            goto chs_fail;
+        if (*p != ',')
+            goto chs_fail;
+        p++;
+        hda_heads = strtol(p, (char **)&p, 0);
+        if (hda_heads < 1 || hda_heads > 16)
+            goto chs_fail;
+        if (*p != ',')
+            goto chs_fail;
+        p++;
+        hda_secs = strtol(p, (char **)&p, 0);
+        if (hda_secs < 1 || hda_secs > 63)
+            goto chs_fail;
+        if (*p == ',') {
+            p++;
+            if (!strcmp(p, "none"))
+                hda_translation = BIOS_ATA_TRANSLATION_NONE;
+            else if (!strcmp(p, "lba"))
+                hda_translation = BIOS_ATA_TRANSLATION_LBA;
+            else if (!strcmp(p, "auto"))
+                hda_translation = BIOS_ATA_TRANSLATION_AUTO;
+            else
+                goto chs_fail;
+        } else if (*p != '\0') {
+        chs_fail:
+            fprintf(stderr, "qemu: invalid physical CHS format\n");
+            exit(1);
+        }
+        if (hda_index != -1)
+            snprintf(drives_opt[hda_index].opt,
+                     sizeof(drives_opt[hda_index].opt),
+                     HD_ALIAS ",cyls=%d,heads=%d,secs=%d%s",
+                     0, hda_cyls, hda_heads, hda_secs,
+                     hda_translation == BIOS_ATA_TRANSLATION_LBA ?
+                         ",trans=lba" :
+                             hda_translation == BIOS_ATA_TRANSLATION_NONE ?
+                                 ",trans=none" : "");
+        break;
+    case QEMU_OPTION_nographic:
+        serial_devices[0] = "stdio";
+        parallel_devices[0] = "null";
+        monitor_device = "stdio";
+        nographic = 1;
+        break;
+#ifdef CONFIG_CURSES
+    case QEMU_OPTION_curses:
+        curses = 1;
+        break;
 #endif
-        0,
-        1,
-        { .init_isa = Adlib_init }
-    },
+    case QEMU_OPTION_portrait:
+        graphic_rotate = 1;
+        break;
+    case QEMU_OPTION_kernel:
+        kernel_filename = optarg;
+        break;
+    case QEMU_OPTION_append:
+        kernel_cmdline = optarg;
+        break;
+    case QEMU_OPTION_cdrom:
+        drive_add(optarg, CDROM_ALIAS);
+        break;
+    case QEMU_OPTION_boot:
+        boot_devices = optarg;
+        /* We just do some generic consistency checks.
+         * Could easily be extended to 64 devices if needed */
+        boot_devices_bitmap = 0;
+        for (p = boot_devices; *p != '\0'; p++) {
+            /* Allowed boot devices are:
+             * a b     : floppy disk drives
+             * c ... f : IDE disk drives
+             * g ... m : machine implementation dependant drives
+             * n ... p : network devices
+             * It's up to each machine implementation to check
+             * if the given boot devices match the actual hardware
+             * implementation and firmware features.
+             */
+            if (*p < 'a' || *p > 'q') {
+                fprintf(stderr, "Invalid boot device '%c'\n", *p);
+                exit(1);
+            }
+            if (boot_devices_bitmap & (1 << (*p - 'a'))) {
+                fprintf(stderr,
+                        "Boot device '%c' was given twice\n",*p);
+                exit(1);
+            }
+            boot_devices_bitmap |= 1 << (*p - 'a');
+        }
+        break;
+    case QEMU_OPTION_fda:
+    case QEMU_OPTION_fdb:
+        drive_add(optarg, FD_ALIAS, index - QEMU_OPTION_fda);
+        break;
+#ifdef TARGET_I386
+    case QEMU_OPTION_no_fd_bootchk:
+        fd_bootchk = 0;
+        break;
 #endif
-
-#ifdef CONFIG_GUS
-    {
-        "gus",
-        "Gravis Ultrasound GF1",
-        0,
-        1,
-        { .init_isa = GUS_init }
-    },
+    case QEMU_OPTION_net:
+        if (nb_net_clients >= MAX_NET_CLIENTS) {
+            fprintf(stderr, "qemu: too many network clients\n");
+            exit(1);
+        }
+        net_clients[nb_net_clients] = optarg;
+        nb_net_clients++;
+        break;
+#ifdef CONFIG_SLIRP
+    case QEMU_OPTION_tftp:
+        tftp_prefix = optarg;
+        break;
+    case QEMU_OPTION_bootp:
+        bootp_filename = optarg;
+        break;
+#ifndef _WIN32
+    case QEMU_OPTION_smb:
+        net_slirp_smb(optarg);
+        break;
 #endif
-
-#ifdef CONFIG_AC97
-    {
-        "ac97",
-        "Intel 82801AA AC97 Audio",
-        0,
-        0,
-        { .init_pci = ac97_init }
-    },
+    case QEMU_OPTION_redir:
+        net_slirp_redir(optarg);
+        break;
 #endif
-
-    {
-        "es1370",
-        "ENSONIQ AudioPCI ES1370",
-        0,
-        0,
-        { .init_pci = es1370_init }
-    },
+#ifdef HAS_AUDIO
+    case QEMU_OPTION_audio_help:
+        AUD_help ();
+        exit(0);
+        break;
+    case QEMU_OPTION_soundhw:
+        select_soundhw (optarg);
+        break;
 #endif
+    case QEMU_OPTION_h:
+        help(0);
+        break;
+    case QEMU_OPTION_m:
+        {
+            char *ptr;
+            uint64_t value;
 
-    { NULL, NULL, 0, 0, { NULL } }
-};
-
-static void select_soundhw (const char *optarg)
-{
-    struct soundhw *c;
-
-    if (*optarg == '?') {
-    show_valid_cards:
+            value = strtoul(optarg, &ptr, 10);
+            switch (*ptr) {
+            case 0: case 'M': case 'm':
+                value <<= 20;
+                break;
+            case 'G': case 'g':
+                value <<= 30;
+                break;
+            default:
+                fprintf(stderr, "qemu: invalid ram size: %s\n", optarg);
+                exit(1);
+            }
 
-        printf ("Valid sound card names (comma separated):\n");
-        for (c = soundhw; c->name; ++c) {
-            printf ("%-11s %s\n", c->name, c->descr);
+            /* On 32-bit hosts, QEMU is limited by virtual address space */
+            if (value > (2047 << 20)
+#ifndef USE_KQEMU
+                && HOST_LONG_BITS == 32
+#endif
+                ) {
+                fprintf(stderr,
+                        "qemu: at most 2047 MB RAM can be simulated\n");
+                exit(1);
+            }
+            if (value != (uint64_t)(ram_addr_t)value) {
+                fprintf(stderr, "qemu: ram size too large\n");
+                exit(1);
+            }
+            ram_size = value;
         }
-        printf ("\n-soundhw all will enable all of the above\n");
-        exit (*optarg != '?');
-    }
-    else {
-        size_t l;
-        const char *p;
-        char *e;
-        int bad_card = 0;
+        break;
+    case QEMU_OPTION_d:
+        {
+            int mask;
+            CPULogItem *item;
 
-        if (!strcmp (optarg, "all")) {
-            for (c = soundhw; c->name; ++c) {
-                c->enabled = 1;
+            mask = cpu_str_to_log_mask(optarg);
+            if (!mask) {
+                printf("Log items (comma separated):\n");
+                for(item = cpu_log_items; item->mask != 0; item++) {
+                    printf("%-10s %s\n", item->name, item->help);
+                }
+                exit(1);
             }
-            return;
+            cpu_set_log(mask);
         }
+        break;
+#ifdef CONFIG_GDBSTUB
+    case QEMU_OPTION_s:
+        use_gdbstub = 1;
+        break;
+    case QEMU_OPTION_p:
+        gdbstub_port = optarg;
+        break;
+#endif
+    case QEMU_OPTION_L:
+        bios_dir = optarg;
+        break;
+    case QEMU_OPTION_bios:
+        bios_name = optarg;
+        break;
+    case QEMU_OPTION_S:
+        autostart = 0;
+        break;
+    case QEMU_OPTION_k:
+        keyboard_layout = optarg;
+        break;
+    case QEMU_OPTION_localtime:
+        rtc_utc = 0;
+        break;
+    case QEMU_OPTION_cirrusvga:
+        cirrus_vga_enabled = 1;
+        vmsvga_enabled = 0;
+        break;
+    case QEMU_OPTION_vmsvga:
+        cirrus_vga_enabled = 0;
+        vmsvga_enabled = 1;
+        break;
+    case QEMU_OPTION_std_vga:
+        cirrus_vga_enabled = 0;
+        vmsvga_enabled = 0;
+        break;
+    case QEMU_OPTION_g:
+        {
+            int w, h, depth;
+            p = optarg;
+            w = strtol(p, (char **)&p, 10);
+            if (w <= 0) {
+            graphic_error:
+                fprintf(stderr, "qemu: invalid resolution or depth\n");
+                exit(1);
+            }
+            if (*p != 'x')
+                goto graphic_error;
+            p++;
+            h = strtol(p, (char **)&p, 10);
+            if (h <= 0)
+                goto graphic_error;
+            if (*p == 'x') {
+                p++;
+                depth = strtol(p, (char **)&p, 10);
+                if (depth != 8 && depth != 15 && depth != 16 &&
+                    depth != 24 && depth != 32)
+                    goto graphic_error;
+            } else if (*p == '\0') {
+                depth = graphic_depth;
+            } else {
+                goto graphic_error;
+            }
 
-        p = optarg;
-        while (*p) {
-            e = strchr (p, ',');
-            l = !e ? strlen (p) : (size_t) (e - p);
-
-            for (c = soundhw; c->name; ++c) {
-                if (!strncmp (c->name, p, l)) {
-                    c->enabled = 1;
-                    break;
-                }
+            graphic_width = w;
+            graphic_height = h;
+            graphic_depth = depth;
+        }
+        break;
+    case QEMU_OPTION_echr:
+        {
+            char *r;
+            term_escape_char = strtol(optarg, &r, 0);
+            if (r == optarg) {
+                fprintf(stderr, "Bad argument to echr\n");
+                exit(1);
             }
+        }
+        break;
+    case QEMU_OPTION_monitor:
+        monitor_device = optarg;
+        break;
+    case QEMU_OPTION_serial:
+        if (serial_device_index >= MAX_SERIAL_PORTS) {
+            fprintf(stderr, "qemu: too many serial ports\n");
+            exit(1);
+        }
+        serial_devices[serial_device_index] = optarg;
+        serial_device_index++;
+        break;
+    case QEMU_OPTION_parallel:
+        if (parallel_device_index >= MAX_PARALLEL_PORTS) {
+            fprintf(stderr, "qemu: too many parallel ports\n");
+            exit(1);
+        }
+        parallel_devices[parallel_device_index] = optarg;
+        parallel_device_index++;
+        break;
+    case QEMU_OPTION_loadvm:
+        loadvm = optarg;
+        break;
+    case QEMU_OPTION_full_screen:
+        full_screen = 1;
+        break;
+#ifdef CONFIG_SDL
+    case QEMU_OPTION_no_frame:
+        no_frame = 1;
+        break;
+    case QEMU_OPTION_alt_grab:
+        alt_grab = 1;
+        break;
+    case QEMU_OPTION_no_quit:
+        no_quit = 1;
+        break;
+#endif
+    case QEMU_OPTION_pidfile:
+        pid_file = optarg;
+        break;
+#ifdef TARGET_I386
+    case QEMU_OPTION_win2k_hack:
+        win2k_install_hack = 1;
+        break;
+#endif
+#ifdef USE_KQEMU
+    case QEMU_OPTION_no_kqemu:
+        kqemu_allowed = 0;
+        break;
+    case QEMU_OPTION_kernel_kqemu:
+        kqemu_allowed = 2;
+        break;
+#endif
+    case QEMU_OPTION_usb:
+        usb_enabled = 1;
+        break;
+    case QEMU_OPTION_usbdevice:
+        usb_enabled = 1;
+        if (usb_devices_index >= MAX_USB_CMDLINE) {
+            fprintf(stderr, "Too many USB devices\n");
+            exit(1);
+        }
+        usb_devices[usb_devices_index] = optarg;
+        usb_devices_index++;
+        break;
+    case QEMU_OPTION_smp:
+        smp_cpus = atoi(optarg);
+        if (smp_cpus < 1 || smp_cpus > MAX_CPUS) {
+            fprintf(stderr, "Invalid number of CPUs\n");
+            exit(1);
+        }
+        break;
+    case QEMU_OPTION_vnc:
+        vnc_display = optarg;
+        break;
+    case QEMU_OPTION_no_acpi:
+        acpi_enabled = 0;
+        break;
+    case QEMU_OPTION_no_reboot:
+        no_reboot = 1;
+        break;
+    case QEMU_OPTION_no_shutdown:
+        no_shutdown = 1;
+        break;
+    case QEMU_OPTION_show_cursor:
+        cursor_hide = 0;
+        break;
+    case QEMU_OPTION_daemonize:
+        daemonize = 1;
+        break;
+    case QEMU_OPTION_option_rom:
+        if (nb_option_roms >= MAX_OPTION_ROMS) {
+            fprintf(stderr, "Too many option ROMs\n");
+            exit(1);
+        }
+        option_rom[nb_option_roms] = optarg;
+        nb_option_roms++;
+        break;
+    case QEMU_OPTION_semihosting:
+        semihosting_enabled = 1;
+        break;
+    case QEMU_OPTION_name:
+        qemu_name = optarg;
+        break;
+#ifdef TARGET_SPARC
+    case QEMU_OPTION_prom_env:
+        if (nb_prom_envs >= MAX_PROM_ENVS) {
+            fprintf(stderr, "Too many prom variables\n");
+            exit(1);
+        }
+        prom_envs[nb_prom_envs] = optarg;
+        nb_prom_envs++;
+        break;
+#endif
+#ifdef TARGET_ARM
+    case QEMU_OPTION_old_param:
+        old_param = 1;
+        break;
+#endif
+    case QEMU_OPTION_clock:
+        configure_alarms(optarg);
+        break;
+    case QEMU_OPTION_startdate:
+        {
+            struct tm tm;
+            time_t rtc_start_date;
 
-            if (!c->name) {
-                if (l > 80) {
-                    fprintf (stderr,
-                             "Unknown sound card name (too big to show)\n");
+            if (!strcmp(optarg, "now")) {
+                rtc_date_offset = -1;
+            } else {
+                if (sscanf(optarg, "%d-%d-%dT%d:%d:%d",
+                        &tm.tm_year,
+                        &tm.tm_mon,
+                        &tm.tm_mday,
+                        &tm.tm_hour,
+                        &tm.tm_min,
+                        &tm.tm_sec) == 6) {
+                    /* OK */
+                } else if (sscanf(optarg, "%d-%d-%d",
+                                    &tm.tm_year,
+                                    &tm.tm_mon,
+                                    &tm.tm_mday) == 3) {
+                    tm.tm_hour = 0;
+                    tm.tm_min = 0;
+                    tm.tm_sec = 0;
+                } else {
+                    goto date_fail;
                 }
-                else {
-                    fprintf (stderr, "Unknown sound card name `%.*s'\n",
-                             (int) l, p);
+                tm.tm_year -= 1900;
+                tm.tm_mon--;
+                rtc_start_date = mktimegm(&tm);
+                if (rtc_start_date == -1) {
+                date_fail:
+                    fprintf(stderr, "Invalid date format. Valid format are:\n"
+                            "'now' or '2006-06-17T16:01:21' or '2006-06-17'\n");
+                    exit(1);
                 }
-                bad_card = 1;
+                rtc_date_offset = time(NULL) - rtc_start_date;
             }
-            p += l + (e != NULL);
         }
-
-        if (bad_card)
-            goto show_valid_cards;
+        break;
     }
 }
-#endif
 
-#ifdef _WIN32
-static BOOL WINAPI qemu_ctrl_handler(DWORD type)
+void qemu_register_option_set(QEMUOptionSet *option_set)
 {
-    exit(STATUS_CONTROL_C_EXIT);
-    return TRUE;
-}
-#endif
+    QEMUOptionSet *optset = &qemu_options;
 
-#define MAX_NET_CLIENTS 32
+    while (optset->next)
+        optset = optset->next;
+    optset->next = option_set;
+    option_set->next = NULL;
+}
 
 int main(int argc, char **argv)
 {
-#ifdef CONFIG_GDBSTUB
-    int use_gdbstub;
-    const char *gdbstub_port;
-#endif
-    uint32_t boot_devices_bitmap = 0;
     int i;
-    int snapshot, linux_boot, net_boot;
-    const char *initrd_filename;
-    const char *kernel_filename, *kernel_cmdline;
-    const char *boot_devices = "";
+    int linux_boot, net_boot;
     DisplayState *ds = &display_state;
-    int cyls, heads, secs, translation;
-    const char *net_clients[MAX_NET_CLIENTS];
-    int nb_net_clients;
-    int hda_index;
     int optind;
     const char *r, *optarg;
     CharDriverState *monitor_hd;
-    const char *monitor_device;
-    const char *serial_devices[MAX_SERIAL_PORTS];
-    int serial_device_index;
-    const char *parallel_devices[MAX_PARALLEL_PORTS];
-    int parallel_device_index;
-    const char *loadvm = NULL;
-    QEMUMachine *machine;
-    const char *cpu_model;
-    const char *usb_devices[MAX_USB_CMDLINE];
-    int usb_devices_index;
     int fds[2];
-    const char *pid_file = NULL;
     VLANState *vlan;
 
     LIST_INIT (&vm_change_state_head);
@@ -7733,43 +8243,23 @@ int main(int argc, char **argv)
 
     register_machines();
     machine = first_machine;
-    cpu_model = NULL;
-    initrd_filename = NULL;
     ram_size = 0;
     vga_ram_size = VGA_RAM_SIZE;
-#ifdef CONFIG_GDBSTUB
-    use_gdbstub = 0;
-    gdbstub_port = DEFAULT_GDBSTUB_PORT;
-#endif
-    snapshot = 0;
     nographic = 0;
     curses = 0;
-    kernel_filename = NULL;
-    kernel_cmdline = "";
-    cyls = heads = secs = 0;
-    translation = BIOS_ATA_TRANSLATION_AUTO;
-    monitor_device = "vc:800x600";
 
+    monitor_device = "vc:800x600";
     serial_devices[0] = "vc:80Cx24C";
-    for(i = 1; i < MAX_SERIAL_PORTS; i++)
-        serial_devices[i] = NULL;
-    serial_device_index = 0;
-
     parallel_devices[0] = "vc:640x480";
-    for(i = 1; i < MAX_PARALLEL_PORTS; i++)
-        parallel_devices[i] = NULL;
-    parallel_device_index = 0;
-
-    usb_devices_index = 0;
 
-    nb_net_clients = 0;
     nb_drives = 0;
     nb_drives_opt = 0;
-    hda_index = -1;
 
     nb_nics = 0;
     /* default mac address of the first network interface */
 
+    qemu_bin_name = argv[0];
+
     optind = 1;
     for(;;) {
         if (optind >= argc)
@@ -7778,18 +8268,25 @@ int main(int argc, char **argv)
         if (r[0] != '-') {
 	    hda_index = drive_add(argv[optind++], HD_ALIAS, 0);
         } else {
+            QEMUOptionSet *optset;
             const QEMUOption *popt;
 
             optind++;
             /* Treat --foo the same as -foo.  */
             if (r[1] == '-')
                 r++;
-            popt = qemu_options;
+
+            optset = &qemu_options;
+            popt = optset->options;
             for(;;) {
                 if (!popt->name) {
-                    fprintf(stderr, "%s: invalid option -- '%s'\n",
-                            argv[0], r);
-                    exit(1);
+                    optset = optset->next;
+                    if (!optset) {
+                        fprintf(stderr, "%s: invalid option -- '%s'\n",
+                                argv[0], r);
+                        exit(1);
+                    }
+                    popt = optset->options;
                 }
                 if (!strcmp(popt->name, r + 1))
                     break;
@@ -7805,498 +8302,7 @@ int main(int argc, char **argv)
             } else {
                 optarg = NULL;
             }
-
-            switch(popt->index) {
-            case QEMU_OPTION_M:
-                machine = find_machine(optarg);
-                if (!machine) {
-                    QEMUMachine *m;
-                    printf("Supported machines are:\n");
-                    for(m = first_machine; m != NULL; m = m->next) {
-                        printf("%-10s %s%s\n",
-                               m->name, m->desc,
-                               m == first_machine ? " (default)" : "");
-                    }
-                    exit(*optarg != '?');
-                }
-                break;
-            case QEMU_OPTION_cpu:
-                /* hw initialization will check this */
-                if (*optarg == '?') {
-/* XXX: implement xxx_cpu_list for targets that still miss it */
-#if defined(cpu_list)
-                    cpu_list(stdout, &fprintf);
-#endif
-                    exit(0);
-                } else {
-                    cpu_model = optarg;
-                }
-                break;
-            case QEMU_OPTION_initrd:
-                initrd_filename = optarg;
-                break;
-            case QEMU_OPTION_hda:
-                if (cyls == 0)
-                    hda_index = drive_add(optarg, HD_ALIAS, 0);
-                else
-                    hda_index = drive_add(optarg, HD_ALIAS
-			     ",cyls=%d,heads=%d,secs=%d%s",
-                             0, cyls, heads, secs,
-                             translation == BIOS_ATA_TRANSLATION_LBA ?
-                                 ",trans=lba" :
-                             translation == BIOS_ATA_TRANSLATION_NONE ?
-                                 ",trans=none" : "");
-                 break;
-            case QEMU_OPTION_hdb:
-            case QEMU_OPTION_hdc:
-            case QEMU_OPTION_hdd:
-                drive_add(optarg, HD_ALIAS, popt->index - QEMU_OPTION_hda);
-                break;
-            case QEMU_OPTION_drive:
-                drive_add(NULL, "%s", optarg);
-	        break;
-            case QEMU_OPTION_mtdblock:
-                drive_add(optarg, MTD_ALIAS);
-                break;
-            case QEMU_OPTION_sd:
-                drive_add(optarg, SD_ALIAS);
-                break;
-            case QEMU_OPTION_pflash:
-                drive_add(optarg, PFLASH_ALIAS);
-                break;
-            case QEMU_OPTION_snapshot:
-                snapshot = 1;
-                break;
-            case QEMU_OPTION_hdachs:
-                {
-                    const char *p;
-                    p = optarg;
-                    cyls = strtol(p, (char **)&p, 0);
-                    if (cyls < 1 || cyls > 16383)
-                        goto chs_fail;
-                    if (*p != ',')
-                        goto chs_fail;
-                    p++;
-                    heads = strtol(p, (char **)&p, 0);
-                    if (heads < 1 || heads > 16)
-                        goto chs_fail;
-                    if (*p != ',')
-                        goto chs_fail;
-                    p++;
-                    secs = strtol(p, (char **)&p, 0);
-                    if (secs < 1 || secs > 63)
-                        goto chs_fail;
-                    if (*p == ',') {
-                        p++;
-                        if (!strcmp(p, "none"))
-                            translation = BIOS_ATA_TRANSLATION_NONE;
-                        else if (!strcmp(p, "lba"))
-                            translation = BIOS_ATA_TRANSLATION_LBA;
-                        else if (!strcmp(p, "auto"))
-                            translation = BIOS_ATA_TRANSLATION_AUTO;
-                        else
-                            goto chs_fail;
-                    } else if (*p != '\0') {
-                    chs_fail:
-                        fprintf(stderr, "qemu: invalid physical CHS format\n");
-                        exit(1);
-                    }
-		    if (hda_index != -1)
-                        snprintf(drives_opt[hda_index].opt,
-                                 sizeof(drives_opt[hda_index].opt),
-                                 HD_ALIAS ",cyls=%d,heads=%d,secs=%d%s",
-                                 0, cyls, heads, secs,
-			         translation == BIOS_ATA_TRANSLATION_LBA ?
-			     	    ",trans=lba" :
-			         translation == BIOS_ATA_TRANSLATION_NONE ?
-			             ",trans=none" : "");
-                }
-                break;
-            case QEMU_OPTION_nographic:
-                serial_devices[0] = "stdio";
-                parallel_devices[0] = "null";
-                monitor_device = "stdio";
-                nographic = 1;
-                break;
-#ifdef CONFIG_CURSES
-            case QEMU_OPTION_curses:
-                curses = 1;
-                break;
-#endif
-            case QEMU_OPTION_portrait:
-                graphic_rotate = 1;
-                break;
-            case QEMU_OPTION_kernel:
-                kernel_filename = optarg;
-                break;
-            case QEMU_OPTION_append:
-                kernel_cmdline = optarg;
-                break;
-            case QEMU_OPTION_cdrom:
-                drive_add(optarg, CDROM_ALIAS);
-                break;
-            case QEMU_OPTION_boot:
-                boot_devices = optarg;
-                /* We just do some generic consistency checks */
-                {
-                    /* Could easily be extended to 64 devices if needed */
-                    const char *p;
-                    
-                    boot_devices_bitmap = 0;
-                    for (p = boot_devices; *p != '\0'; p++) {
-                        /* Allowed boot devices are:
-                         * a b     : floppy disk drives
-                         * c ... f : IDE disk drives
-                         * g ... m : machine implementation dependant drives
-                         * n ... p : network devices
-                         * It's up to each machine implementation to check
-                         * if the given boot devices match the actual hardware
-                         * implementation and firmware features.
-                         */
-                        if (*p < 'a' || *p > 'q') {
-                            fprintf(stderr, "Invalid boot device '%c'\n", *p);
-                            exit(1);
-                        }
-                        if (boot_devices_bitmap & (1 << (*p - 'a'))) {
-                            fprintf(stderr,
-                                    "Boot device '%c' was given twice\n",*p);
-                            exit(1);
-                        }
-                        boot_devices_bitmap |= 1 << (*p - 'a');
-                    }
-                }
-                break;
-            case QEMU_OPTION_fda:
-            case QEMU_OPTION_fdb:
-                drive_add(optarg, FD_ALIAS, popt->index - QEMU_OPTION_fda);
-                break;
-#ifdef TARGET_I386
-            case QEMU_OPTION_no_fd_bootchk:
-                fd_bootchk = 0;
-                break;
-#endif
-            case QEMU_OPTION_net:
-                if (nb_net_clients >= MAX_NET_CLIENTS) {
-                    fprintf(stderr, "qemu: too many network clients\n");
-                    exit(1);
-                }
-                net_clients[nb_net_clients] = optarg;
-                nb_net_clients++;
-                break;
-#ifdef CONFIG_SLIRP
-            case QEMU_OPTION_tftp:
-		tftp_prefix = optarg;
-                break;
-            case QEMU_OPTION_bootp:
-                bootp_filename = optarg;
-                break;
-#ifndef _WIN32
-            case QEMU_OPTION_smb:
-		net_slirp_smb(optarg);
-                break;
-#endif
-            case QEMU_OPTION_redir:
-                net_slirp_redir(optarg);
-                break;
-#endif
-#ifdef HAS_AUDIO
-            case QEMU_OPTION_audio_help:
-                AUD_help ();
-                exit (0);
-                break;
-            case QEMU_OPTION_soundhw:
-                select_soundhw (optarg);
-                break;
-#endif
-            case QEMU_OPTION_h:
-                help(0);
-                break;
-            case QEMU_OPTION_m: {
-                uint64_t value;
-                char *ptr;
-
-                value = strtoul(optarg, &ptr, 10);
-                switch (*ptr) {
-                case 0: case 'M': case 'm':
-                    value <<= 20;
-                    break;
-                case 'G': case 'g':
-                    value <<= 30;
-                    break;
-                default:
-                    fprintf(stderr, "qemu: invalid ram size: %s\n", optarg);
-                    exit(1);
-                }
-
-                /* On 32-bit hosts, QEMU is limited by virtual address space */
-                if (value > (2047 << 20)
-#ifndef USE_KQEMU
-                    && HOST_LONG_BITS == 32
-#endif
-                    ) {
-                    fprintf(stderr, "qemu: at most 2047 MB RAM can be simulated\n");
-                    exit(1);
-                }
-                if (value != (uint64_t)(ram_addr_t)value) {
-                    fprintf(stderr, "qemu: ram size too large\n");
-                    exit(1);
-                }
-                ram_size = value;
-                break;
-            }
-            case QEMU_OPTION_d:
-                {
-                    int mask;
-                    CPULogItem *item;
-
-                    mask = cpu_str_to_log_mask(optarg);
-                    if (!mask) {
-                        printf("Log items (comma separated):\n");
-                    for(item = cpu_log_items; item->mask != 0; item++) {
-                        printf("%-10s %s\n", item->name, item->help);
-                    }
-                    exit(1);
-                    }
-                    cpu_set_log(mask);
-                }
-                break;
-#ifdef CONFIG_GDBSTUB
-            case QEMU_OPTION_s:
-                use_gdbstub = 1;
-                break;
-            case QEMU_OPTION_p:
-                gdbstub_port = optarg;
-                break;
-#endif
-            case QEMU_OPTION_L:
-                bios_dir = optarg;
-                break;
-            case QEMU_OPTION_bios:
-                bios_name = optarg;
-                break;
-            case QEMU_OPTION_S:
-                autostart = 0;
-                break;
-	    case QEMU_OPTION_k:
-		keyboard_layout = optarg;
-		break;
-            case QEMU_OPTION_localtime:
-                rtc_utc = 0;
-                break;
-            case QEMU_OPTION_cirrusvga:
-                cirrus_vga_enabled = 1;
-                vmsvga_enabled = 0;
-                break;
-            case QEMU_OPTION_vmsvga:
-                cirrus_vga_enabled = 0;
-                vmsvga_enabled = 1;
-                break;
-            case QEMU_OPTION_std_vga:
-                cirrus_vga_enabled = 0;
-                vmsvga_enabled = 0;
-                break;
-            case QEMU_OPTION_g:
-                {
-                    const char *p;
-                    int w, h, depth;
-                    p = optarg;
-                    w = strtol(p, (char **)&p, 10);
-                    if (w <= 0) {
-                    graphic_error:
-                        fprintf(stderr, "qemu: invalid resolution or depth\n");
-                        exit(1);
-                    }
-                    if (*p != 'x')
-                        goto graphic_error;
-                    p++;
-                    h = strtol(p, (char **)&p, 10);
-                    if (h <= 0)
-                        goto graphic_error;
-                    if (*p == 'x') {
-                        p++;
-                        depth = strtol(p, (char **)&p, 10);
-                        if (depth != 8 && depth != 15 && depth != 16 &&
-                            depth != 24 && depth != 32)
-                            goto graphic_error;
-                    } else if (*p == '\0') {
-                        depth = graphic_depth;
-                    } else {
-                        goto graphic_error;
-                    }
-
-                    graphic_width = w;
-                    graphic_height = h;
-                    graphic_depth = depth;
-                }
-                break;
-            case QEMU_OPTION_echr:
-                {
-                    char *r;
-                    term_escape_char = strtol(optarg, &r, 0);
-                    if (r == optarg)
-                        printf("Bad argument to echr\n");
-                    break;
-                }
-            case QEMU_OPTION_monitor:
-                monitor_device = optarg;
-                break;
-            case QEMU_OPTION_serial:
-                if (serial_device_index >= MAX_SERIAL_PORTS) {
-                    fprintf(stderr, "qemu: too many serial ports\n");
-                    exit(1);
-                }
-                serial_devices[serial_device_index] = optarg;
-                serial_device_index++;
-                break;
-            case QEMU_OPTION_parallel:
-                if (parallel_device_index >= MAX_PARALLEL_PORTS) {
-                    fprintf(stderr, "qemu: too many parallel ports\n");
-                    exit(1);
-                }
-                parallel_devices[parallel_device_index] = optarg;
-                parallel_device_index++;
-                break;
-	    case QEMU_OPTION_loadvm:
-		loadvm = optarg;
-		break;
-            case QEMU_OPTION_full_screen:
-                full_screen = 1;
-                break;
-#ifdef CONFIG_SDL
-            case QEMU_OPTION_no_frame:
-                no_frame = 1;
-                break;
-            case QEMU_OPTION_alt_grab:
-                alt_grab = 1;
-                break;
-            case QEMU_OPTION_no_quit:
-                no_quit = 1;
-                break;
-#endif
-            case QEMU_OPTION_pidfile:
-                pid_file = optarg;
-                break;
-#ifdef TARGET_I386
-            case QEMU_OPTION_win2k_hack:
-                win2k_install_hack = 1;
-                break;
-#endif
-#ifdef USE_KQEMU
-            case QEMU_OPTION_no_kqemu:
-                kqemu_allowed = 0;
-                break;
-            case QEMU_OPTION_kernel_kqemu:
-                kqemu_allowed = 2;
-                break;
-#endif
-            case QEMU_OPTION_usb:
-                usb_enabled = 1;
-                break;
-            case QEMU_OPTION_usbdevice:
-                usb_enabled = 1;
-                if (usb_devices_index >= MAX_USB_CMDLINE) {
-                    fprintf(stderr, "Too many USB devices\n");
-                    exit(1);
-                }
-                usb_devices[usb_devices_index] = optarg;
-                usb_devices_index++;
-                break;
-            case QEMU_OPTION_smp:
-                smp_cpus = atoi(optarg);
-                if (smp_cpus < 1 || smp_cpus > MAX_CPUS) {
-                    fprintf(stderr, "Invalid number of CPUs\n");
-                    exit(1);
-                }
-                break;
-	    case QEMU_OPTION_vnc:
-		vnc_display = optarg;
-		break;
-            case QEMU_OPTION_no_acpi:
-                acpi_enabled = 0;
-                break;
-            case QEMU_OPTION_no_reboot:
-                no_reboot = 1;
-                break;
-            case QEMU_OPTION_no_shutdown:
-                no_shutdown = 1;
-                break;
-            case QEMU_OPTION_show_cursor:
-                cursor_hide = 0;
-                break;
-	    case QEMU_OPTION_daemonize:
-		daemonize = 1;
-		break;
-	    case QEMU_OPTION_option_rom:
-		if (nb_option_roms >= MAX_OPTION_ROMS) {
-		    fprintf(stderr, "Too many option ROMs\n");
-		    exit(1);
-		}
-		option_rom[nb_option_roms] = optarg;
-		nb_option_roms++;
-		break;
-            case QEMU_OPTION_semihosting:
-                semihosting_enabled = 1;
-                break;
-            case QEMU_OPTION_name:
-                qemu_name = optarg;
-                break;
-#ifdef TARGET_SPARC
-            case QEMU_OPTION_prom_env:
-                if (nb_prom_envs >= MAX_PROM_ENVS) {
-                    fprintf(stderr, "Too many prom variables\n");
-                    exit(1);
-                }
-                prom_envs[nb_prom_envs] = optarg;
-                nb_prom_envs++;
-                break;
-#endif
-#ifdef TARGET_ARM
-            case QEMU_OPTION_old_param:
-                old_param = 1;
-                break;
-#endif
-            case QEMU_OPTION_clock:
-                configure_alarms(optarg);
-                break;
-            case QEMU_OPTION_startdate:
-                {
-                    struct tm tm;
-                    time_t rtc_start_date;
-                    if (!strcmp(optarg, "now")) {
-                        rtc_date_offset = -1;
-                    } else {
-                        if (sscanf(optarg, "%d-%d-%dT%d:%d:%d",
-                               &tm.tm_year,
-                               &tm.tm_mon,
-                               &tm.tm_mday,
-                               &tm.tm_hour,
-                               &tm.tm_min,
-                               &tm.tm_sec) == 6) {
-                            /* OK */
-                        } else if (sscanf(optarg, "%d-%d-%d",
-                                          &tm.tm_year,
-                                          &tm.tm_mon,
-                                          &tm.tm_mday) == 3) {
-                            tm.tm_hour = 0;
-                            tm.tm_min = 0;
-                            tm.tm_sec = 0;
-                        } else {
-                            goto date_fail;
-                        }
-                        tm.tm_year -= 1900;
-                        tm.tm_mon--;
-                        rtc_start_date = mktimegm(&tm);
-                        if (rtc_start_date == -1) {
-                        date_fail:
-                            fprintf(stderr, "Invalid date format. Valid format are:\n"
-                                    "'now' or '2006-06-17T16:01:21' or '2006-06-17'\n");
-                            exit(1);
-                        }
-                        rtc_date_offset = time(NULL) - rtc_start_date;
-                    }
-                }
-                break;
-            }
+            optset->parse_handler(popt->index, optarg);
         }
     }
 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] Modular command line options
  2008-05-20 23:51 [Qemu-devel] [PATCH 1/3] Modular command line options Jan Kiszka
@ 2008-05-21 19:17 ` Glauber Costa
  2008-05-21 20:58 ` Fabrice Bellard
  2008-05-22 10:19 ` Ian Jackson
  2 siblings, 0 replies; 9+ messages in thread
From: Glauber Costa @ 2008-05-21 19:17 UTC (permalink / raw)
  To: qemu-devel

Looks awesome.

But if you allow me to be a bit boring again, I have some extra comments.

>
> +#define HAS_ARG 0x0001
> +
> +typedef struct QEMUOption {
> +    const char *name;
> +    int flags;
> +    int index;
> +} QEMUOption;
> +
> +typedef void QEMUOptionParser(int index, const char *optarg);
> +
> +typedef struct QEMUOptionSet {
> +    const QEMUOption *options;
> +    const char *help_string;
> +    QEMUOptionParser *parse_handler;
> +    struct QEMUOptionSet *next;
> +} QEMUOptionSet;

This is a bit counter-intuitive. If we switch to this model, the Help
string would be better with the option itself, rather than the option
set. Listing the options would then be a matter of iterating over each
option,
and then printing each of them. An optional help string in the option
set could be used as a header. Like this:

Help string for the option set:
      -foo help for foo option, in a QEMUoption
      -bar help for bar option, in a QEMUOption

This seems cleaner and more flexible to me. What do you think?

>  #ifdef TARGET_PPC
> -#define DEFAULT_RAM_SIZE 144
> +#define DEFAULT_RAM_SIZE        144
> +#define DEFAULT_RAM_SIZE_STR    "144"
>  #else
> -#define DEFAULT_RAM_SIZE 128
> +#define DEFAULT_RAM_SIZE        128
> +#define DEFAULT_RAM_SIZE_STR    "128"
>  #endif

This is a bit ugly IMHO.
Defining:
__STR(x) #x
STR(x) __STR(x) in a common header sounds better. We can then just
STR(DEFAULT_RAM_SIZE), or whatever, in any string pasting.
Since we're moving to this scheme, it is possible that new needs for
doing the same thing will arise, and then just coming up with a bunch
of FOO , FOO_STR binoms seems not elegant enough.

> +static int snapshot;
> -static int drive_init(struct drive_opt *arg, int snapshot,
> +static int drive_init(struct drive_opt *arg, int use_snapshot,
>                       QEMUMachine *machine)

static global variable, static function. Can't we just not use the
argument at all? Yeah, I know it was there before, but...

> +
> +#ifdef CONFIG_GUS
> +    {
> +        "gus",
> +        "Gravis Ultrasound GF1",
> +        0,
> +        1,
> +        { .init_isa = GUS_init }
> +    },
>  #endif

For options that are dependant on specific defines, it is better to
make them separate, and then register the option set conditionally.
But yeah, I know you're not doing everything in this patch.
Just wanted to leave the opinion, and see whether or not you agree with it.

Other than that, seems all beautiful.


-- 
Glauber Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] Modular command line options
  2008-05-20 23:51 [Qemu-devel] [PATCH 1/3] Modular command line options Jan Kiszka
  2008-05-21 19:17 ` Glauber Costa
@ 2008-05-21 20:58 ` Fabrice Bellard
  2008-05-22 10:11   ` Ian Jackson
  2008-05-22 10:19 ` Ian Jackson
  2 siblings, 1 reply; 9+ messages in thread
From: Fabrice Bellard @ 2008-05-21 20:58 UTC (permalink / raw)
  To: qemu-devel

Jan Kiszka wrote:
> Following up on my earlier proposal to introduce per-machine command
> line options, this version provides a more generic approach. It should
> also be usable for scenarios like per-arch or per-accelerator.
> 
> To summarize the approach: Command line options are now organized in
> sets. Each set consists of a list of options, a help text, and a pointer
> the a parse handler that can process the options. The base option set of
> QEMU is automatically registered, more sets can be added via
> qemu_register_option_set().
> 
> The patch includes quite some refactoring in order to move the basic
> switches over the new scheme. So you may have a look at the data
> structures in qemu-common.h, but otherwise it is better to apply it and
> then study vl.c. More refactoring of existing switches is likely
> feasible, but first this needs and an OK from (or merging by) the
> maintainers.
> [...]

I am not sure it is the way to follow because soon the configuration
file will be the way to pass elaborated options to QEMU.

Regards,

Fabrice.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] Modular command line options
  2008-05-21 20:58 ` Fabrice Bellard
@ 2008-05-22 10:11   ` Ian Jackson
  0 siblings, 0 replies; 9+ messages in thread
From: Ian Jackson @ 2008-05-22 10:11 UTC (permalink / raw)
  To: qemu-devel

Fabrice Bellard writes ("Re: [Qemu-devel] [PATCH 1/3] Modular command line options"):
> Jan Kiszka wrote:
> > [table-driven options]
> 
> I am not sure it is the way to follow because soon the configuration
> file will be the way to pass elaborated options to QEMU.

On the contrary, a table-driven command-line parsing arrangement can
be reused by the config file reader.  This makes it easy to ensure
that the command-line options and config file settings have the same
names and do the same things.

If, while inventing the new config file setup, we want to improve the
names and syntaxes of the settings, we can do that if we have a way to
make two differently formatted options control the same internal
setting variables.  But we will want one of those anyway because
sooner or later we will decide that some of the config file options we
have invented aren't the right way of expressing it, and will want to
change them without breaking old configurations.

Using the same data source for both also has the advantage that as new
syntaxes are provided in the config file, the same syntax (or some
easily-related syntax, depending on the exact form of the config file
and command line parser harnesses) automatically becomes available on
the command line.

But I agree with Glauber Costa's specific comments and I have some of
my own.

Ian.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] Modular command line options
  2008-05-20 23:51 [Qemu-devel] [PATCH 1/3] Modular command line options Jan Kiszka
  2008-05-21 19:17 ` Glauber Costa
  2008-05-21 20:58 ` Fabrice Bellard
@ 2008-05-22 10:19 ` Ian Jackson
  2008-05-22 10:20   ` Ian Jackson
  2008-05-22 12:49   ` [Qemu-devel] " Jan Kiszka
  2 siblings, 2 replies; 9+ messages in thread
From: Ian Jackson @ 2008-05-22 10:19 UTC (permalink / raw)
  To: qemu-devel

Jan Kiszka writes ("[Qemu-devel] [PATCH 1/3] Modular command line options"):
> Following up on my earlier proposal to introduce per-machine command
> line options, this version provides a more generic approach. It should
> also be usable for scenarios like per-arch or per-accelerator.

I approve of splitting the code up like this, and having a
table-driven parsing arrangement.  But ideally we could get rid of
`index' and the giant switch() statements too.  Something more like

  typedef void QEMUOptionParser(struct QEMUOption *option, const char *optarg);

  typedef struct QEMUOption {
      const char *name, *helpstring;
      QEMUOptionParser handler;
      int flags;
      int int_for_handler;
      void *void_for_handler;
  } QEMUOption;

  qemu_register_option_set(const QEMUOption *options);

We pass the QEMUOption* to the parser handler so it can see the
canonical name and any extra stuff put in the option structure.
and in vl.c you'd do something like this:

  static const QEMUOption basic_options[]= {
    ...
    { "hda", opthandler_drive, 0, 0 },
    { "hdb", opthandler_drive, 0, 1 },
    { "hdc", opthandler_drive, 0, 3 },
    { "hdd", opthandler_drive, 0, 4 },
    ...
    { 0 } /* null entry is required to terminate the table */
  }

    qemu_register_option_set(basic_options);

The linked list of option tables is private to the option parser.

Ian.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] Modular command line options
  2008-05-22 10:19 ` Ian Jackson
@ 2008-05-22 10:20   ` Ian Jackson
  2008-05-22 12:49   ` [Qemu-devel] " Jan Kiszka
  1 sibling, 0 replies; 9+ messages in thread
From: Ian Jackson @ 2008-05-22 10:20 UTC (permalink / raw)
  To: qemu-devel

iwj writes ("Re: [Qemu-devel] [PATCH 1/3] Modular command line options"):
> typedef void QEMUOptionParser(struct QEMUOption *option, const char *optarg);
> 
> typedef struct QEMUOption {
>     const char *name, *helpstring;
>     QEMUOptionParser handler;
                      ^
This should be 
      QEMUOptionParser *handler;
of course.

Ian.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Qemu-devel] Re: [PATCH 1/3] Modular command line options
  2008-05-22 10:19 ` Ian Jackson
  2008-05-22 10:20   ` Ian Jackson
@ 2008-05-22 12:49   ` Jan Kiszka
  2008-05-23 13:29     ` Glauber Costa
  1 sibling, 1 reply; 9+ messages in thread
From: Jan Kiszka @ 2008-05-22 12:49 UTC (permalink / raw)
  To: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 2487 bytes --]

Ian Jackson wrote:
> Jan Kiszka writes ("[Qemu-devel] [PATCH 1/3] Modular command line options"):
>> Following up on my earlier proposal to introduce per-machine command
>> line options, this version provides a more generic approach. It should
>> also be usable for scenarios like per-arch or per-accelerator.
> 
> I approve of splitting the code up like this, and having a
> table-driven parsing arrangement.  But ideally we could get rid of
> `index' and the giant switch() statements too.  Something more like
> 
>   typedef void QEMUOptionParser(struct QEMUOption *option, const char *optarg);
> 
>   typedef struct QEMUOption {
>       const char *name, *helpstring;
>       QEMUOptionParser handler;
>       int flags;

Ack. This just enforces a bit more effort to convert the existing
opts... :->

>       int int_for_handler;
>       void *void_for_handler;

I don't think there is an need for both. A plain

	void *parser_opaque;

should suffice as the user can perfectly typecast the void to int.

>   } QEMUOption;
> 
>   qemu_register_option_set(const QEMUOption *options);

Here I would then suggest

	qemu_register_option_set(const char *set_name,
	                         const QEMUOption *options);

to save the chance for visually grouping options.

> 
> We pass the QEMUOption* to the parser handler so it can see the
> canonical name and any extra stuff put in the option structure.
> and in vl.c you'd do something like this:
> 
>   static const QEMUOption basic_options[]= {
>     ...
>     { "hda", opthandler_drive, 0, 0 },
>     { "hdb", opthandler_drive, 0, 1 },
>     { "hdc", opthandler_drive, 0, 3 },
>     { "hdd", opthandler_drive, 0, 4 },
>     ...
>     { 0 } /* null entry is required to terminate the table */
>   }
> 
>     qemu_register_option_set(basic_options);
> 
> The linked list of option tables is private to the option parser.

Good idea. Then the structure should look like this:

struct QEMUOptionSet {
	const char *name;
	const QEMUOption *options;
	struct QEMUOptionSet *next;
};


OK, as this version would require even more refactoring, please let us
agree on the critical data structures first. Specifically, this takes an
ack from the maintainers.

Jan


PS: The element set of a future config file format could perfectly grow
with each QEMUOption registered to the core. So I also see no conflict
of this effort with the config file specification work.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 254 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] Re: [PATCH 1/3] Modular command line options
  2008-05-22 12:49   ` [Qemu-devel] " Jan Kiszka
@ 2008-05-23 13:29     ` Glauber Costa
  2008-05-23 15:12       ` Jan Kiszka
  0 siblings, 1 reply; 9+ messages in thread
From: Glauber Costa @ 2008-05-23 13:29 UTC (permalink / raw)
  To: qemu-devel

On Thu, May 22, 2008 at 9:49 AM, Jan Kiszka <jan.kiszka@web.de> wrote:
> Ian Jackson wrote:
>> Jan Kiszka writes ("[Qemu-devel] [PATCH 1/3] Modular command line options"):
>>> Following up on my earlier proposal to introduce per-machine command
>>> line options, this version provides a more generic approach. It should
>>> also be usable for scenarios like per-arch or per-accelerator.
>>
>> I approve of splitting the code up like this, and having a
>> table-driven parsing arrangement.  But ideally we could get rid of
>> `index' and the giant switch() statements too.  Something more like
>>
>>   typedef void QEMUOptionParser(struct QEMUOption *option, const char *optarg);
>>
>>   typedef struct QEMUOption {
>>       const char *name, *helpstring;
>>       QEMUOptionParser handler;
>>       int flags;
>
> Ack. This just enforces a bit more effort to convert the existing
> opts... :->
>
>>       int int_for_handler;
>>       void *void_for_handler;
>
> I don't think there is an need for both. A plain
>
>        void *parser_opaque;
>
> should suffice as the user can perfectly typecast the void to int.
>
>>   } QEMUOption;
>>
>>   qemu_register_option_set(const QEMUOption *options);
>
> Here I would then suggest
>
>        qemu_register_option_set(const char *set_name,
>                                 const QEMUOption *options);
>
> to save the chance for visually grouping options.

I don't follow the need for the set_name parameter. This should be in
the OptionSet structure.
You mean, joining two groups if they are registered with the same
name? This can be done by enforcing them to have
the same "name" parameter in the OptionSet structure. Am I failing to
understand anything here?

>
>
> Good idea. Then the structure should look like this:
>
> struct QEMUOptionSet {
>        const char *name;
          ^^^^^^^^^^^^


-- 
Glauber Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Qemu-devel] Re: [PATCH 1/3] Modular command line options
  2008-05-23 13:29     ` Glauber Costa
@ 2008-05-23 15:12       ` Jan Kiszka
  0 siblings, 0 replies; 9+ messages in thread
From: Jan Kiszka @ 2008-05-23 15:12 UTC (permalink / raw)
  To: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 2671 bytes --]

Glauber Costa wrote:
> On Thu, May 22, 2008 at 9:49 AM, Jan Kiszka <jan.kiszka@web.de> wrote:
>> Ian Jackson wrote:
>>> Jan Kiszka writes ("[Qemu-devel] [PATCH 1/3] Modular command line options"):
>>>> Following up on my earlier proposal to introduce per-machine command
>>>> line options, this version provides a more generic approach. It should
>>>> also be usable for scenarios like per-arch or per-accelerator.
>>> I approve of splitting the code up like this, and having a
>>> table-driven parsing arrangement.  But ideally we could get rid of
>>> `index' and the giant switch() statements too.  Something more like
>>>
>>>   typedef void QEMUOptionParser(struct QEMUOption *option, const char *optarg);
>>>
>>>   typedef struct QEMUOption {
>>>       const char *name, *helpstring;
>>>       QEMUOptionParser handler;
>>>       int flags;
>> Ack. This just enforces a bit more effort to convert the existing
>> opts... :->
>>
>>>       int int_for_handler;
>>>       void *void_for_handler;
>> I don't think there is an need for both. A plain
>>
>>        void *parser_opaque;
>>
>> should suffice as the user can perfectly typecast the void to int.
>>
>>>   } QEMUOption;
>>>
>>>   qemu_register_option_set(const QEMUOption *options);
>> Here I would then suggest
>>
>>        qemu_register_option_set(const char *set_name,
>>                                 const QEMUOption *options);
>>
>> to save the chance for visually grouping options.
> 
> I don't follow the need for the set_name parameter. This should be in
> the OptionSet structure.
> You mean, joining two groups if they are registered with the same
> name? This can be done by enforcing them to have
> the same "name" parameter in the OptionSet structure. Am I failing to
> understand anything here?

Let's consider this hypothetical scenario:

$ qemu -h
...
Options specific to KQEMU accelerator:
-kernel-kqemu   enable KQEMU full virtualization (default is user mode only)
-no-kqemu       disable KQEMU kernel module usage
...

The registration will work like this:

QEMUOption kqemu_options[] = {
	{ "kernel-kqemu", "enable KQEMU...", kqemu_enable_kernel, 0, NULL },
	{ "no-kqemu", "disable KQEMU...", kqemu_disable, 0, NULL },
	{ NULL }
};
...
cpu_get_phys_page_debug("KQEMU accelerator", kqemu_options);

The set name is then kept in the _private_ QEMUOptionSet structure that
is created and hooked into the global list by cpu_get_phys_page_debug.

Joining groups with identical names, at least for the help output, was
not planned yet. Might be a useful extension, but I would wait for
feedback on use cases first.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 254 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2008-05-23 15:12 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-05-20 23:51 [Qemu-devel] [PATCH 1/3] Modular command line options Jan Kiszka
2008-05-21 19:17 ` Glauber Costa
2008-05-21 20:58 ` Fabrice Bellard
2008-05-22 10:11   ` Ian Jackson
2008-05-22 10:19 ` Ian Jackson
2008-05-22 10:20   ` Ian Jackson
2008-05-22 12:49   ` [Qemu-devel] " Jan Kiszka
2008-05-23 13:29     ` Glauber Costa
2008-05-23 15:12       ` Jan Kiszka

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).