Re: [Qemu-devel] [PATCH 1/8] machine: query iommu machine property rather than qemu opts

[Markus Armbruster]

Marcel Apfelbaum <address@hidden> writes:

> On 02/04/2015 06:47 PM, Markus Armbruster wrote:
>> Marcel Apfelbaum <address@hidden> writes:
>>
>>> Fixes a QEMU crash when passing iommu parameter in command line.
>>>
>>> Signed-off-by: Marcel Apfelbaum <address@hidden>
>>> ---
>>>   hw/core/machine.c   | 5 +++++
>>>   hw/pci-host/q35.c   | 2 +-
>>>   include/hw/boards.h | 1 +
>>>   3 files changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/core/machine.c b/hw/core/machine.c
>>> index fbd91be..096eb10 100644
>>> --- a/hw/core/machine.c
>>> +++ b/hw/core/machine.c
>>> @@ -403,6 +403,11 @@ bool machine_usb(MachineState *machine)
>>>       return machine->usb;
>>>   }
>>>
>>> +bool machine_iommu(MachineState *machine)
>>> +{
>>> +    return machine->iommu;
>>> +}
>>> +
>>>   static const TypeInfo machine_info = {
>>>       .name = TYPE_MACHINE,
>>>       .parent = TYPE_OBJECT,
>>
>> What does this buy us over a straight current_machine->iommu?
> Following QOM guidelines/conventions all object fields are private
> to machine files only. The *only* ways that they can be exposed are:
> 1. A wrapper
> 2. object_property_get...
>
> I chose the wrapper because the other variant would be really
> ugly and should be used only on a generic code.
>
> This is the real reason I used this, but pure theoretical speaking
> using  wrappers will give us the opportunity to change machine_iommu
> implementation without the need to change the call sites.

So we pay the notational and cognitive overhead of wrappers now and
going forward to maybe some day save us fixing up a couple of
machine->iommu instances.

A theory that makes sense for interfacing complex machinery with
non-trivial implementation details becomes silly when applied to a
couple of straightforward configuration flags.  As Doug McIlroy
observed, "KISS became MICAHI: make it complicated and hide it."

> To wrap it up, I just followed previous comments I received on QOM handling.

In other words, it's somebody else's fault.  I'll shut up now ;)

[...]