[Qemu-devel] [PATCH] RFC: options parse in vl.c should be moduled

[Qemu-devel] [PATCH] RFC: options parse in vl.c should be moduled

[Wanpeng Li]

Consider of the options parse process in main function of vl.c is too
long.It should be module into single function to clear ideas, strengthen
the source code management, and increase code readability.So I module the 
process of options parse as function options_parse, and expose some variables
in order to not influence command-line invocations.

Signed-off-by: Wanpeng Li <liwp@linux.vnet.ibm.com>
---
 vl.c |  159 ++++++++++++++++++++++++++++++++++-------------------------------
 1 files changed, 83 insertions(+), 76 deletions(-)

diff --git a/vl.c b/vl.c
index 0fccf50..fa4d0a9 100644
--- a/vl.c
+++ b/vl.c
@@ -2251,84 +2251,40 @@ int qemu_init_main_loop(void)
     return main_loop_init();
 }
 
-int main(int argc, char **argv, char **envp)
-{
-    int i;
-    int snapshot, linux_boot;
-    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;
-    QemuOptsList *olist;
-    int optind;
-    const char *optarg;
-    const char *loadvm = NULL;
-    QEMUMachine *machine;
-    const char *cpu_model;
-    const char *vga_model = NULL;
-    const char *pid_file = NULL;
-    const char *incoming = NULL;
+int snapshot, linux_boot;
+const char *icount_option;
+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 , *opts, *machine_opts;
+QemuOptsList *olist;
+int optind;
+const char *loadvm;
+QEMUMachine *machine;
+const char *cpu_model;
+const char *vga_model;
+const char *pid_file;
+const char *incoming;
 #ifdef CONFIG_VNC
-    int show_vnc_port = 0;
+int show_vnc_port;
 #endif
-    int defconfig = 1;
-    const char *log_mask = NULL;
-    const char *log_file = NULL;
-    GMemVTable mem_trace = {
-        .malloc = malloc_and_trace,
-        .realloc = realloc_and_trace,
-        .free = free_and_trace,
-    };
-    const char *trace_events = NULL;
-    const char *trace_file = NULL;
-
-    atexit(qemu_run_exit_notifiers);
-    error_set_progname(argv[0]);
-
-    g_mem_set_vtable(&mem_trace);
-    if (!g_thread_supported()) {
-#if !GLIB_CHECK_VERSION(2, 31, 0)
-        g_thread_init(NULL);
-#else
-        fprintf(stderr, "glib threading failed to initialize.\n");
-        exit(1);
-#endif
-    }
-
-    module_call_init(MODULE_INIT_QOM);
-
-    runstate_init();
-
-    init_clocks();
-    rtc_clock = host_clock;
-
-    qemu_cache_utils_init(envp);
-
-    QLIST_INIT (&vm_change_state_head);
-    os_setup_early_signal_handling();
-
-    module_call_init(MODULE_INIT_MACHINE);
-    machine = find_default_machine();
-    cpu_model = NULL;
-    ram_size = 0;
-    snapshot = 0;
-    cyls = heads = secs = 0;
-    translation = BIOS_ATA_TRANSLATION_AUTO;
-
-    for (i = 0; i < MAX_NODES; i++) {
-        node_mem[i] = 0;
-        node_cpumask[i] = 0;
-    }
-
-    nb_numa_nodes = 0;
-    nb_nics = 0;
-
-    autostart= 1;
+int defconfig = 1;
+const char *log_mask;
+const char *log_file;
+GMemVTable mem_trace = {
+    .malloc = malloc_and_trace,
+    .realloc = realloc_and_trace,
+    .free = free_and_trace,
+};
+const char *trace_events;
+const char *trace_file;
 
+static void options_parse(int argc, char **argv)
+{
+    const char *optarg;
     /* first pass of option parsing */
     optind = 1;
     while (optind < argc) {
@@ -2867,7 +2823,7 @@ int main(int argc, char **argv, char **envp)
                 if (watchdog) {
                     fprintf(stderr,
                             "qemu: only one watchdog option may be given\n");
-                    return 1;
+                    exit(1);
                 }
                 watchdog = optarg;
                 break;
@@ -3186,6 +3142,57 @@ int main(int argc, char **argv, char **envp)
             }
         }
     }
+}
+
+int main(int argc, char **argv, char **envp)
+{
+    int i;
+
+    atexit(qemu_run_exit_notifiers);
+    error_set_progname(argv[0]);
+
+    g_mem_set_vtable(&mem_trace);
+    if (!g_thread_supported()) {
+#if !GLIB_CHECK_VERSION(2, 31, 0)
+        g_thread_init(NULL);
+#else
+        fprintf(stderr, "glib threading failed to initialize.\n");
+        exit(1);
+#endif
+    }
+
+    module_call_init(MODULE_INIT_QOM);
+
+    runstate_init();
+
+    init_clocks();
+    rtc_clock = host_clock;
+
+    qemu_cache_utils_init(envp);
+
+    QLIST_INIT(&vm_change_state_head);
+    os_setup_early_signal_handling();
+
+    module_call_init(MODULE_INIT_MACHINE);
+    machine = find_default_machine();
+    cpu_model = NULL;
+    ram_size = 0;
+    snapshot = 0;
+    cyls = heads = secs = 0;
+    translation = BIOS_ATA_TRANSLATION_AUTO;
+
+    for (i = 0; i < MAX_NODES; i++) {
+        node_mem[i] = 0;
+        node_cpumask[i] = 0;
+    }
+
+    nb_numa_nodes = 0;
+    nb_nics = 0;
+
+    autostart = 1;
+
+    options_parse(argc, argv);
+
     loc_set_none();
 
     /* Init CPU def lists, based on config
-- 
1.7.5.4

Re: [Qemu-devel] [PATCH] RFC: options parse in vl.c should be moduled

[Daniel P. Berrange]

On Fri, Mar 30, 2012 at 08:36:43PM +0800, Wanpeng Li wrote:
> Consider of the options parse process in main function of vl.c is too
> long.It should be module into single function to clear ideas, strengthen
> the source code management, and increase code readability.So I module the 
> process of options parse as function options_parse, and expose some variables
> in order to not influence command-line invocations.
> 
> Signed-off-by: Wanpeng Li <liwp@linux.vnet.ibm.com>
> ---
>  vl.c |  159 ++++++++++++++++++++++++++++++++++-------------------------------
>  1 files changed, 83 insertions(+), 76 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index 0fccf50..fa4d0a9 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2251,84 +2251,40 @@ int qemu_init_main_loop(void)
>      return main_loop_init();
>  }
>  
> -int main(int argc, char **argv, char **envp)
> -{
> -    int i;
> -    int snapshot, linux_boot;
> -    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;
> -    QemuOptsList *olist;
> -    int optind;
> -    const char *optarg;
> -    const char *loadvm = NULL;
> -    QEMUMachine *machine;
> -    const char *cpu_model;
> -    const char *vga_model = NULL;
> -    const char *pid_file = NULL;
> -    const char *incoming = NULL;
> +int snapshot, linux_boot;
> +const char *icount_option;
> +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 , *opts, *machine_opts;
> +QemuOptsList *olist;
> +int optind;
> +const char *loadvm;
> +QEMUMachine *machine;
> +const char *cpu_model;
> +const char *vga_model;
> +const char *pid_file;
> +const char *incoming;
>  #ifdef CONFIG_VNC
> -    int show_vnc_port = 0;
> +int show_vnc_port;

[snip]

> +int defconfig = 1;
> +const char *log_mask;
> +const char *log_file;
> +GMemVTable mem_trace = {
> +    .malloc = malloc_and_trace,
> +    .realloc = realloc_and_trace,
> +    .free = free_and_trace,
> +};
> +const char *trace_events;
> +const char *trace_file;
>  
> +static void options_parse(int argc, char **argv)
> +{

While code modularization is a worthy goal, I don't think this patch is
really an improvement. QEMU already has far too many adhoc global variables,
without adding another 30 or more. The resulting code isn't even simplified
or more readable IMHO, it is merely different.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH] RFC: options parse in vl.c should be moduled

[Wanpeng Li]

On Fri, Mar 30, 2012 at 01:53:14PM +0100, Daniel P. Berrange wrote:
>On Fri, Mar 30, 2012 at 08:36:43PM +0800, Wanpeng Li wrote:
>> Consider of the options parse process in main function of vl.c is too
>> long.It should be module into single function to clear ideas, strengthen
>> the source code management, and increase code readability.So I module the 
>> process of options parse as function options_parse, and expose some variables
>> in order to not influence command-line invocations.
>> 
>> Signed-off-by: Wanpeng Li <liwp@linux.vnet.ibm.com>
>> ---
>>  vl.c |  159 ++++++++++++++++++++++++++++++++++-------------------------------
>>  1 files changed, 83 insertions(+), 76 deletions(-)
>> 
>> diff --git a/vl.c b/vl.c
>> index 0fccf50..fa4d0a9 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -2251,84 +2251,40 @@ int qemu_init_main_loop(void)
>>      return main_loop_init();
>>  }
>>  
>> -int main(int argc, char **argv, char **envp)
>> -{
>> -    int i;
>> -    int snapshot, linux_boot;
>> -    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;
>> -    QemuOptsList *olist;
>> -    int optind;
>> -    const char *optarg;
>> -    const char *loadvm = NULL;
>> -    QEMUMachine *machine;
>> -    const char *cpu_model;
>> -    const char *vga_model = NULL;
>> -    const char *pid_file = NULL;
>> -    const char *incoming = NULL;
>> +int snapshot, linux_boot;
>> +const char *icount_option;
>> +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 , *opts, *machine_opts;
>> +QemuOptsList *olist;
>> +int optind;
>> +const char *loadvm;
>> +QEMUMachine *machine;
>> +const char *cpu_model;
>> +const char *vga_model;
>> +const char *pid_file;
>> +const char *incoming;
>>  #ifdef CONFIG_VNC
>> -    int show_vnc_port = 0;
>> +int show_vnc_port;
>
>[snip]
>
>> +int defconfig = 1;
>> +const char *log_mask;
>> +const char *log_file;
>> +GMemVTable mem_trace = {
>> +    .malloc = malloc_and_trace,
>> +    .realloc = realloc_and_trace,
>> +    .free = free_and_trace,
>> +};
>> +const char *trace_events;
>> +const char *trace_file;
>>  
>> +static void options_parse(int argc, char **argv)
>> +{
>
>While code modularization is a worthy goal, I don't think this patch is
>really an improvement. QEMU already has far too many adhoc global variables,
>without adding another 30 or more. The resulting code isn't even simplified
>or more readable IMHO, it is merely different.
>
>Daniel

There are about 856 lines of codes handle options parse in main function.
It is ugly and reduce readability. So I module these codes to a
single function called "options_parse".Since there are amounts of
command_line parameters which lead to must transfer many parameters to function
options_parse, so I expose some variables to global in order to handler
this issue.

Regards,
Wanpeng Li

-- 
LTC China, IBM, Shanghai

Re: [Qemu-devel] [PATCH] RFC: options parse in vl.c should be moduled

[Michael Roth]

On Fri, Mar 30, 2012 at 09:25:47PM +0800, Wanpeng Li wrote:
> On Fri, Mar 30, 2012 at 01:53:14PM +0100, Daniel P. Berrange wrote:
> >On Fri, Mar 30, 2012 at 08:36:43PM +0800, Wanpeng Li wrote:
> >> Consider of the options parse process in main function of vl.c is too
> >> long.It should be module into single function to clear ideas, strengthen
> >> the source code management, and increase code readability.So I module the 
> >> process of options parse as function options_parse, and expose some variables
> >> in order to not influence command-line invocations.
> >> 
> >> Signed-off-by: Wanpeng Li <liwp@linux.vnet.ibm.com>
> >> ---
> >>  vl.c |  159 ++++++++++++++++++++++++++++++++++-------------------------------
> >>  1 files changed, 83 insertions(+), 76 deletions(-)
> >> 
> >> diff --git a/vl.c b/vl.c
> >> index 0fccf50..fa4d0a9 100644
> >> --- a/vl.c
> >> +++ b/vl.c
> >> @@ -2251,84 +2251,40 @@ int qemu_init_main_loop(void)
> >>      return main_loop_init();
> >>  }
> >>  
> >> -int main(int argc, char **argv, char **envp)
> >> -{
> >> -    int i;
> >> -    int snapshot, linux_boot;
> >> -    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;
> >> -    QemuOptsList *olist;
> >> -    int optind;
> >> -    const char *optarg;
> >> -    const char *loadvm = NULL;
> >> -    QEMUMachine *machine;
> >> -    const char *cpu_model;
> >> -    const char *vga_model = NULL;
> >> -    const char *pid_file = NULL;
> >> -    const char *incoming = NULL;
> >> +int snapshot, linux_boot;
> >> +const char *icount_option;
> >> +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 , *opts, *machine_opts;
> >> +QemuOptsList *olist;
> >> +int optind;
> >> +const char *loadvm;
> >> +QEMUMachine *machine;
> >> +const char *cpu_model;
> >> +const char *vga_model;
> >> +const char *pid_file;
> >> +const char *incoming;
> >>  #ifdef CONFIG_VNC
> >> -    int show_vnc_port = 0;
> >> +int show_vnc_port;
> >
> >[snip]
> >
> >> +int defconfig = 1;
> >> +const char *log_mask;
> >> +const char *log_file;
> >> +GMemVTable mem_trace = {
> >> +    .malloc = malloc_and_trace,
> >> +    .realloc = realloc_and_trace,
> >> +    .free = free_and_trace,
> >> +};
> >> +const char *trace_events;
> >> +const char *trace_file;
> >>  
> >> +static void options_parse(int argc, char **argv)
> >> +{
> >
> >While code modularization is a worthy goal, I don't think this patch is
> >really an improvement. QEMU already has far too many adhoc global variables,
> >without adding another 30 or more. The resulting code isn't even simplified
> >or more readable IMHO, it is merely different.
> >
> >Daniel
> 
> There are about 856 lines of codes handle options parse in main function.
> It is ugly and reduce readability. So I module these codes to a
> single function called "options_parse".Since there are amounts of
> command_line parameters which lead to must transfer many parameters to function
> options_parse, so I expose some variables to global in order to handler
> this issue.

Anthony's aforementioned patches avoid the new globals by moving them into a
single global configuration struct. Keeps things a little tidier and
makes is easier to recognize when code is accessing a global config
option.

> 
> Regards,
> Wanpeng Li
> 
> -- 
> LTC China, IBM, Shanghai
> 
> 

Re: [Qemu-devel] [PATCH] RFC: options parse in vl.c should be moduled

[Anthony Liguori]

On 03/30/2012 07:36 AM, Wanpeng Li wrote:
> Consider of the options parse process in main function of vl.c is too
> long.It should be module into single function to clear ideas, strengthen
> the source code management, and increase code readability.So I module the
> process of options parse as function options_parse, and expose some variables
> in order to not influence command-line invocations.
>
> Signed-off-by: Wanpeng Li<liwp@linux.vnet.ibm.com>

I've got pending patches that significantly refactor this code.

http://mid.gmane.org/1332169763-30665-1-git-send-email-aliguori@us.ibm.com

I think I'll drop some of the more controversial patches and resubmit soon.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH] RFC: options parse in vl.c should be moduled

[Lluís Vilanova]

Wanpeng Li writes:

> Consider of the options parse process in main function of vl.c is too
> long.It should be module into single function to clear ideas, strengthen
> the source code management, and increase code readability.So I module the 
> process of options parse as function options_parse, and expose some variables
> in order to not influence command-line invocations.

Another approach would be to use something similar to QOM type registration.

In order to keep code to a minimum, you can define an array of pointers to
QParameter structures, instead of an array of function pointers (what
__attribute__((constructors)) does).

So, for example:

    static void opt_icount_handler(const char *args)
    {
        if (kvm_enabled() || xen_enabled()) {
            fprintf(stderr, "-icount is not allowed with kvm or xen\n");
            exit(1);
        }
        /* ... contents of configure_icount ... */
    }

    static QEMUOption opt_icount = {
       .name = "icount",
       .signature = "[N|auto]",
       .help = "enable virtual instruction counter with 2^N clock ticks per instruction",
       .mode = QEMU_MODE_SYSTEM,
       .priority = QEMU_OPT_PRIO_BEFORE_DEVICES,
       .section = QEMU_OPT_SECTION_DEBUG,
       .handler = opt_icount_handler
    };
     
    param_register(opt_icount);


Here, param_register would define an array of pointers to QEMUOption structures,
although I'm not sure how this can be handled on formats other than ELF.


This is mostly what you already have in QEMUOptionParameter, and could still be
automatically generated from an extended qemu-options.hx when necessary; so I'm
talking about extending it to also contain:

* Priority; a set of predefined priorities can be provided, so that you just
  decide the partial order of your option.

* Handler function.

* QEMU mode(s) that provide this option; this could also be extended to
  establish target restrictions (e.g., only available in system-x86_64).


This might help improving the current situation by having the code where it
logically belongs (e.g., HW-specific options registered along the code for that
HW), or might just add more cruft to it...


Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth

Re: [Qemu-devel] [PATCH] RFC: options parse in vl.c should be moduled

[Anthony Liguori]

On 03/30/2012 10:59 AM, Lluís Vilanova wrote:
> Wanpeng Li writes:
>
>> Consider of the options parse process in main function of vl.c is too
>> long.It should be module into single function to clear ideas, strengthen
>> the source code management, and increase code readability.So I module the
>> process of options parse as function options_parse, and expose some variables
>> in order to not influence command-line invocations.
>
> Another approach would be to use something similar to QOM type registration.
>
> In order to keep code to a minimum, you can define an array of pointers to
> QParameter structures, instead of an array of function pointers (what
> __attribute__((constructors)) does).
>
> So, for example:
>
>      static void opt_icount_handler(const char *args)
>      {
>          if (kvm_enabled() || xen_enabled()) {
>              fprintf(stderr, "-icount is not allowed with kvm or xen\n");
>              exit(1);
>          }
>          /* ... contents of configure_icount ... */
>      }
>
>      static QEMUOption opt_icount = {
>         .name = "icount",
>         .signature = "[N|auto]",
>         .help = "enable virtual instruction counter with 2^N clock ticks per instruction",
>         .mode = QEMU_MODE_SYSTEM,
>         .priority = QEMU_OPT_PRIO_BEFORE_DEVICES,
>         .section = QEMU_OPT_SECTION_DEBUG,
>         .handler = opt_icount_handler
>      };
>
>      param_register(opt_icount);
>
>
> Here, param_register would define an array of pointers to QEMUOption structures,
> although I'm not sure how this can be handled on formats other than ELF.

My plan is to have a -query-capabilities so we can change the help text, then 
use GOptionContext.

It would take a small function that goes through and promotes any option in the 
format '-foo' to '--foo'.  But that's not too hard.

Then we can completely rip out the option parsing, qemu-options.hx, and all of 
that other junk.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH] RFC: options parse in vl.c should be moduled

[Anthony Liguori]

On 03/30/2012 11:09 AM, Anthony Liguori wrote:
> On 03/30/2012 10:59 AM, Lluís Vilanova wrote:
>> Wanpeng Li writes:
>>
>>> Consider of the options parse process in main function of vl.c is too
>>> long.It should be module into single function to clear ideas, strengthen
>>> the source code management, and increase code readability.So I module the
>>> process of options parse as function options_parse, and expose some variables
>>> in order to not influence command-line invocations.
>>
>> Another approach would be to use something similar to QOM type registration.
>>
>> In order to keep code to a minimum, you can define an array of pointers to
>> QParameter structures, instead of an array of function pointers (what
>> __attribute__((constructors)) does).
>>
>> So, for example:
>>
>> static void opt_icount_handler(const char *args)
>> {
>> if (kvm_enabled() || xen_enabled()) {
>> fprintf(stderr, "-icount is not allowed with kvm or xen\n");
>> exit(1);
>> }
>> /* ... contents of configure_icount ... */
>> }
>>
>> static QEMUOption opt_icount = {
>> .name = "icount",
>> .signature = "[N|auto]",
>> .help = "enable virtual instruction counter with 2^N clock ticks per
>> instruction",
>> .mode = QEMU_MODE_SYSTEM,
>> .priority = QEMU_OPT_PRIO_BEFORE_DEVICES,
>> .section = QEMU_OPT_SECTION_DEBUG,
>> .handler = opt_icount_handler
>> };
>>
>> param_register(opt_icount);
>>
>>
>> Here, param_register would define an array of pointers to QEMUOption structures,
>> although I'm not sure how this can be handled on formats other than ELF.
>
> My plan is to have a -query-capabilities so we can change the help text, then
> use GOptionContext.
>
> It would take a small function that goes through and promotes any option in the
> format '-foo' to '--foo'. But that's not too hard.
>
> Then we can completely rip out the option parsing, qemu-options.hx, and all of
> that other junk.

glib's option parsing support has some really nice features like translation 
support, grouping, and inline help for option definition.

The grouping is a really nice touch.  It will let us have a target specific 
group, device groups, etc.

Regards,

Anthony Liguori

>
> Regards,
>
> Anthony Liguori
>

Re: [Qemu-devel] [PATCH] RFC: options parse in vl.c should be moduled

[Lluís Vilanova]

Anthony Liguori writes:
>> My plan is to have a -query-capabilities so we can change the help text, then
>> use GOptionContext.
>> 
>> It would take a small function that goes through and promotes any option in the
>> format '-foo' to '--foo'. But that's not too hard.
>> 
>> Then we can completely rip out the option parsing, qemu-options.hx, and all of
>> that other junk.

> glib's option parsing support has some really nice features like translation
> support, grouping, and inline help for option definition.

> The grouping is a really nice touch.  It will let us have a target specific
> group, device groups, etc.

This sounds really nice and much more "uncrufty".

What I'd like to see is some "type_init"-like black magic so that code can
easily register GOptionEntry structures without having to resort to modifying a
central file (otherwise I think the story will repeat itself again).

For this to work, besides defining the GOptionEntry itself, the programmer
should also establish:

* The option group (for cmdline help grouping). This is already supported by
  GOptionGroup, and everybody should register against a set of pre-defined
  groups.

* The parsing priority (so that it's easy to establish a partial order on the
  parsing of options). This also requires a set of pre-defined priorities to
  register against.

What I'm not sure is whether both can be merged into a single one. That is, that
options in the same group don't care about the order in which their parsing
callbacks are called (as long as you know the order between groups).



Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth