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

[Andreas Färber]

Am 27.01.2012 16:42, schrieb Anthony Liguori:
> On 01/27/2012 09:05 AM, Andreas Färber wrote:
>> 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!)
> 
> No problem.
> 
>> 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.
> 
> When I think of include/, I think of "internally public" headers.  IOW,
> things that are meant to be shared in other parts of QEMU.  For
> instance, something like qemu-queue.h.
> 
> In fact, there are 25 header files in $SRCDIR that are in the form
> qemu-$file.h.  They could (and should) probably be moved to
> include/qemu/$file.h

Okay, agree.

Regards,
Andreas

> 
> As we introduce more directory structure, having a single include
> directory will be very handy.  For instance, pci_dec.c may move to
> drivers/ppc/pci_dec.c.  But having pci_dec.h in include/qemu means that
> we don't have to worry about very long #include paths.
> 
> Regards,
> 
> Anthony Liguori
> 
>>
>> 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