[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH 1/3] Modular command line options
From: |
Jan Kiszka |
Subject: |
[Qemu-devel] Re: [PATCH 1/3] Modular command line options |
Date: |
Fri, 23 May 2008 17:12:23 +0200 |
User-agent: |
Thunderbird 2.0.0.12 (X11/20080226) |
Glauber Costa wrote:
> On Thu, May 22, 2008 at 9:49 AM, Jan Kiszka <address@hidden> 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
signature.asc
Description: OpenPGP digital signature