[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH RFC 3/5] x86: split PV dom0 builder to domain_build_pv.c



On Fri, Mar 03, 2017 at 04:15:19AM -0700, Jan Beulich wrote:
> >>> On 03.03.17 at 12:06, <roger.pau@xxxxxxxxxx> wrote:
> > On Fri, Mar 03, 2017 at 09:41:09AM +0000, Wei Liu wrote:
> >> Long term we want to be able to disentangle PV and HVM code. Move the PV
> >> domain builder to a dedicated file.
> >> 
> >> This in turn requires exposing a few functions and variables via
> >> a new header domain_build.h.
> > 
> > I would add: "No functional change introduced", to make it clearer that it's
> > mostly just code movement.
> > 
> >> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> >> ---
> >>  xen/arch/x86/Makefile          |   1 +
> >>  xen/arch/x86/domain_build.c    | 894 
> >> +---------------------------------------
> >>  xen/arch/x86/domain_build.h    |  32 ++
> > 
> > IMHO I would place this in include/asm-x86/, there are really no headers in
> > arch/x86/.
> 
> That's the wrong criteria: If all consumers of a header are in the same
> directory, putting it there is fine (even if it's the first one). In fact in
> such a case I consider it worse to put it in asm-x86/, as that more
> widely exposes such a supposedly limited scope header. And along

My thought as well.

> those lines I actually have plans to move some of the stuff currently
> under include/asm-x86/ into (sub-trees of) arch/x86/.
> 
> However, with ...
> 
> >> diff --git a/xen/arch/x86/domain_build_pv.c 
> >> b/xen/arch/x86/domain_build_pv.c
> > 
> > I would place this in arch/xen/x86/pv/domain_build.c
> 
> ... this the case becomes less clear: Personally I'm not a fan of
> #include "../file.h", so I wouldn't be certain which of the two is
> the less undesirable place for the header.
> 

But in this case I would prefer to move the to xen/include/xen and named
in dom0_build.h.

Wei.

> Jan
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.