[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH v3 04/32] blockdev: Move bochs prob
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH v3 04/32] blockdev: Move bochs probe into separate file |
Date: |
Wed, 6 Jul 2016 10:24:28 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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 <address@hidden>
> >> ---
> >> 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
- [Qemu-block] [PATCH v3 07/32] blockdev: Move dmg probe to its own file, (continued)
- [Qemu-block] [PATCH v3 07/32] blockdev: Move dmg probe to its own file, Colin Lord, 2016/07/05
- [Qemu-block] [PATCH v3 08/32] blockdev: Move parallels probe to its own file, Colin Lord, 2016/07/05
- [Qemu-block] [PATCH v3 04/32] blockdev: Move bochs probe into separate file, Colin Lord, 2016/07/05
- Re: [Qemu-block] [Qemu-devel] [PATCH v3 04/32] blockdev: Move bochs probe into separate file,
Kevin Wolf <=
- Re: [Qemu-block] [Qemu-devel] [PATCH v3 04/32] blockdev: Move bochs probe into separate file, John Snow, 2016/07/06
- Re: [Qemu-block] [Qemu-devel] [PATCH v3 04/32] blockdev: Move bochs probe into separate file, Markus Armbruster, 2016/07/07
- Re: [Qemu-block] [Qemu-devel] [PATCH v3 04/32] blockdev: Move bochs probe into separate file, John Snow, 2016/07/07
- Re: [Qemu-block] [Qemu-devel] [PATCH v3 04/32] blockdev: Move bochs probe into separate file, Paolo Bonzini, 2016/07/07
- Re: [Qemu-block] [Qemu-devel] [PATCH v3 04/32] blockdev: Move bochs probe into separate file, John Snow, 2016/07/07
- Re: [Qemu-block] [Qemu-devel] [PATCH v3 04/32] blockdev: Move bochs probe into separate file, Kevin Wolf, 2016/07/08
- Re: [Qemu-block] [Qemu-devel] [PATCH v3 04/32] blockdev: Move bochs probe into separate file, Colin Lord, 2016/07/07
- Re: [Qemu-block] [Qemu-devel] [PATCH v3 04/32] blockdev: Move bochs probe into separate file, Kevin Wolf, 2016/07/08
- Re: [Qemu-block] [Qemu-devel] [PATCH v3 04/32] blockdev: Move bochs probe into separate file, Markus Armbruster, 2016/07/08
- Re: [Qemu-block] [Qemu-devel] [PATCH v3 04/32] blockdev: Move bochs probe into separate file, Paolo Bonzini, 2016/07/07