Am 05.07.2016 um 22:50 hat John Snow geschrieben: > > > On 07/05/2016 11:49 AM, Daniel P. Berrange wrote: > > On Tue, Jul 05, 2016 at 11:24:04AM -0400, Colin Lord wrote: > >> This puts the bochs probe function into its own separate file as part of > >> the process of modularizing block drivers. Having the probe functions > >> separate from the rest of the driver allows us to probe without having > >> to potentially unnecessarily load the driver. > >> > >> Signed-off-by: Colin Lord <cl...@redhat.com> > >> --- > >> block/Makefile.objs | 1 + > >> block/bochs.c | 55 > >> ++------------------------------------------ > >> block/probe/bochs.c | 21 +++++++++++++++++ > > > > Do we really need a sub-dir for this ? If we were going to > > have sub-dirs under block/, I'd suggest we have one subdir > > per block driver, not spread code for one block driver > > across multiple dirs. > > > > Admittedly I have been nudging Colin to shoot from the hip a bit on > filename organization because I was short of ideas. > > Some ideas: > > (1) A combined probe.c file. This keeps the existing organization and > localizes everything to just one new file. > > Downside: many formats rely on at least some minimal amount of structure > and constant definitions, and some of those overlap with each other. > qcow and qcow2 both using "QcowHeader" would be a prominent example. > > They could all be disentangled, but it's less clear on where all the > common definitions go. A common probe.h is a bad idea since the modular > portion of the driver has no business gaining access to other formats' > definitions. We could create a probe.c and matching > include/block/bdrv/fmt.h includes, but we lost our zeal for this method. > > (2) Separate probe files for each driver. > > What we went with. Keeps refactoring to a minimum. Adds a bunch of > little files, but it's minimal and fairly noninvasive. > > Like #1 though, we still have to figure out what to do with the common > includes. > > > IMHO a block/bochs-probe.c would be better, unless we did > > move block/bochs.c into a block/bochs/driver.c dir. > > > > Either way, you should update MAINTAINERS file to record > > this newly added filename, against the bochs entry. The > > same applies to most other patches in this series adding > > new files. > > > > > > Regards, > > Daniel > > > > So, something like: > > block/drivers/bochs/
block/bochs/ if anything. We don't have to nest deeply just because we can. I don't really like new subdirectories, but when all drivers start to have multiple files, it might be unavoidable. > bochs.c > probe.c (or bochs-probe.c) > > and > > include/block/drivers/bochs/ > > common.h (or internal.h) block/bochs/internal.h (or bochs.h) Just like we already have some header files directly in block/ (e.g. qcow2.h). They are internal to the block driver, so no reason to move them to the global include directory. Kevin
