[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/3] machine: remove qemu_machine_opts global li
From: |
Alexander Graf |
Subject: |
Re: [Qemu-devel] [PATCH 1/3] machine: remove qemu_machine_opts global list |
Date: |
Mon, 15 Dec 2014 16:13:25 +0100 |
User-agent: |
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 |
On 11.12.14 06:05, Marcel Apfelbaum wrote:
> On Wed, 2014-12-10 at 16:59 -0600, Greg Bellows wrote:
>>
>>
>> On 10 December 2014 at 07:19, Marcel Apfelbaum <address@hidden>
>> wrote:
>> QEMU has support for options per machine, keeping
>> a global list of options is no longer necessary.
>>
>> Signed-off-by: Marcel Apfelbaum <address@hidden>
>> ---
>> hw/core/machine.c | 45 +++++++++++++++++++++++++++++
>> hw/i386/pc.c | 7 +++++
>> hw/ppc/spapr.c | 3 ++
>> vl.c | 84
>> ++++---------------------------------------------------
>> 4 files changed, 61 insertions(+), 78 deletions(-)
>>
>> diff --git a/hw/core/machine.c b/hw/core/machine.c
>> index 19d3e3a..a0ae5f9 100644
>> --- a/hw/core/machine.c
>> +++ b/hw/core/machine.c
>> @@ -291,48 +291,93 @@ static void machine_initfn(Object *obj)
>>
>> object_property_add_str(obj, "accel",
>> machine_get_accel,
>> machine_set_accel, NULL);
>> + object_property_set_description(obj, "accel",
>> + "Accelerator list",
>> + NULL);
>>
> Hi Greg,
> Thank you for your review!
>
>>
>> If this is a common combination of calls would it make sense to create
>> a wrapper routine that just does both of these steps together for
>> readability and convenience? Maybe something like:
>>
>>
>> object_property_add_str_with_desc(obj, "accel", "Accelerator
>> list"
>> machine_get_accel,
>> machine_set_accel,
>> NULL);
> I really thought about it, but one wrapper would not be enough,
> we need also one for object_property_add_bool, object_property_add and so on.
> The code needed would be twice as this one and for the moment this
> series is the only reason for that, maybe is not enough.
>
> If you or anyone else things we should still go for it, I'll be glad to.
Maybe something like this?
#define MACHINE_OPT(name, namestr, desc, type, ...) \
glue(object_property_add_, type)(obj, namestr, \
glue(machine_get, name), glue(machine_set, name), __VA_ARGS__); \
\
object_property_set_description(obj, namestr, desc, NULL);
>
>>
>> object_property_add_bool(obj, "kernel-irqchip",
>> machine_get_kernel_irqchip,
>> machine_set_kernel_irqchip,
>> NULL);
>> + object_property_set_description(obj, "kernel-irqchip",
>> + "Use KVM in-kernel
>> irqchip",
>> + NULL);
MACHINE_OPT(kvm_shadow_mem, "kvm-shadow-mem", "Use KVM in-kernel
irqchip", _bool, NULL);
Not sure it's a great improvement though. I certainly wouldn't mind to
leave it as this patch does it.
Alex
[Qemu-devel] [PATCH 2/3] vl.c: simplified machine_set_property, Marcel Apfelbaum, 2014/12/10
[Qemu-devel] [PATCH 3/3] vl.c: add HMP help to machine, Marcel Apfelbaum, 2014/12/10
Re: [Qemu-devel] [PATCH 0/3] machine: dynamic options per machine type, Alexander Graf, 2014/12/15
Re: [Qemu-devel] [PATCH 0/3] machine: dynamic options per machine type, Peter Maydell, 2014/12/15