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 <marce...@redhat.com>
>> wrote:
>> QEMU has support for options per machine, keeping
>> a global list of options is no longer necessary.
>>
>> Signed-off-by: Marcel Apfelbaum <marce...@redhat.com>
>> ---
>> 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
