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 <pbonz...@redhat.com> > > 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
