Re: [Qemu-devel] [PATCH 14/19] hw: use default-configs for more devices instead of hw/ARCH/Makefile.objs

[Paolo Bonzini]

Il 04/02/2013 18:52, Andreas Färber ha scritto:
> Am 04.02.2013 18:29, schrieb Paolo Bonzini:
>> Other devices are now moved out of hw/ARCH, adding new CONFIG_*
>> symbols that let them be included selectively in the emulators.
>> The devices however are still compiled in target-specific
>> directories.
>>
>> Signed-off-by: Paolo Bonzini <address@hidden>
> 
> I don't think this is a good idea. For one thing the patch does too many
> movements at once for proper commenting.

True, but posting (and maintaining a series of) 120 patches is
cumbersome.  There are automated ways to check that this patch is
correct.  What we need to discuss is the final desired directory structure.

For other patches you're right (especially patch 18 that you commented
on), but then there is no need to rush in those patches.

> For another you group devices strictly by busses.

Actually I don't in general.  I group by exposed interface.  I have all
NICs under hw/net, all character devices under hw/char, all SCSI
adapters under hw/scsi.  SCSI is a bus, NICs and character devices aren't.

Of the existing directories, hw/usb groups devices strictly by bus,
while hw/ide's is the scheme I followed.  hw/pci only has core code, so
I thought of how it would look like at the end of the movement.  I chose
to follow hw/ide because otherwise we'd end up with a hundred unrelated
files in hw/pci.  According to this scheme, instead, PCI host bridges
are logically under hw/pci, because they expose a PCI bus.

Still, USB remains as the biggest exception.  I left it where it is
because for example usb-storage patches would need Gerd's review as much
as (or more than) mine, and because Gerd has pretty broad knowledge
around all of QEMU.

There are a few other exceptions such as scsi-disk being under hw/scsi
and not hw/block.  Again, I tried to follow maintainance areas as the
guideline, and also the Linux kernel (drivers/scsi vs. drivers/block in
this case).  In this particular case, I know Kevin doesn't really care
about reviewing scsi-disk patches.

There is a large degree of subjective judgement.  If there are mistakes
or possible improvements, please point them out.  I and Peter already
reached consensus on his proposal, for example.

> And once again this invasive series does not seem to
> CC the respective maintainers whose code you move around.

It touches every single target and platform, should I Cc every single
maintainer?

I don't think we need that, because for the most part the review of this
patch can be automated.  What we need is consensus on the general
direction, and that can be done by a group of people that a) are
interested b) are willing to produce patches.

> We just had a discussion in the Port I/O context that devices should not
> derive according to busses but should have-a bus-specific connector
> interface/object from a pure chipset object. That would contradict the
> split by bus IMO (but I wasn't the one to call for that).
> 
> Also you are moving files like, e.g., sPAPR or ppc4xx into mst's pci/
> directory, where he is rather unlikely to do much work on.

Do work on?  No.  Maintain, yes.

David is working on sPAPR, but remember neither him nor Alex maintain
the entire platform.  I certainly want to review spapr_vscsi.c patches;
they would need my Acked-by if they were to get in via Alex's tree.
Having it in hw/scsi makes it easier for whoever pulls to check for
missing Acked-by, and for me to spot patches that need my review.
hw/ppc makes it harder.

> It would be
> more sensible to have sPAPR PHB live alongside other sPAPR devices that
> David et al. maintain, say hw/ppc/spapr/. Apart from that I already
> commented on the USB movement that, e.g., spapr_pci is more natural to
> read than the reversed host-spapr.

In a deep directory structure, spapr_pci.c is the worst of both worlds.
 If it is under hw/ppc/spapr/spapr_pci.c, it duplicates spapr.  If it is
under hw/pci/spapr_pci.c, ditto.

Paolo