Re: [Qemu-devel] [PATCH 04/28] qom: add the base Object class (v2)

[Andreas Färber]

Am 25.01.2012 22:37, schrieb Anthony Liguori:
> On 01/25/2012 03:30 PM, Andreas Färber wrote:
>> Am 24.01.2012 20:32, schrieb Anthony Liguori:
>>> This class provides the main building block for QEMU Object Model and is
>>> extensively documented in the header file.  It is largely inspired by
>>> GObject.
>>>
>>> Signed-off-by: Anthony Liguori<address@hidden>
>>> ---
>>> v1 ->  v2
>>>   - remove printf() in type registration
>>>   - fix typo in comment (Paolo)
>>>   - make Interface private
>>>   - move object into a new directory and move header into include/qemu/
>>
>> Some of us had expressed concerns over introducing include/. Any
>> particular reason you're doing it still?
> 
> Because it's a great idea and I thought everyone loved it?
> 
> Can you point me to the concerns raised, I'll go back and look.  I
> didn't think it was contentious...

Can't find it right now (stupid search keyword!) but I remember having a
discussion around whether these are actually "public" APIs because to
date we have always claimed that all APIs are internal and no guarantees
are made. IMO moving headers to an include/ dir marks a change of that
policy because header in include/ usually get installed alongside a
library so if we do it we should do it conciously.

Thing is, headers that are private to one part of code are public to
another. It's not black and white. E.g., hw -> IDE -> AHCI -> ICH9.
Whenever there's multiple subclasses code needs to be shared; it
shouldn't usually be poked at from the outside though in favor of
qdev/QOM properties.

Personally, I find it more handy to find pci_dec.c and pci_dec.h in the
same directory in Nautilus/gedit/whatever (but bad example because I'm
working on making that header go away). On the other hand compared to
like r955 we have seen a huge inflation in hw/ files.
I can live with it either way, and as Paolo says, it can easily be
changed later with git-mv. And #include "qemu/foo.h" sounds fair.

For these new "public" headers I'd be interested in finding a solution
where we can all easily collaborate on the documentation though. Right
now we need to trust you to get the markup right.

Andreas

> To summarize my rationale for it:
> 
>  1) It avoids all of the non-sense with conflicting system headers
> (because we -Iinclude and the headers live in include/qemu)
> 
>  2) It establishes what are public functions for use in other parts of
> qemu vs. private headers (which we currently use based on ad-hoc naming
> schemes like block_int.h).
> 
>  3) I think the kernel serves as an existence proof that this method to
> manage headers works really well in practice.
> 
> Regards,
> 
> Anthony Liguori

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg