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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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
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.

Jan


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