[Xen-devel] Arch changes process suggestion (was: [patch] final header fixes)

[Magenheimer, D. (HP L. F. Collins) <dan...@hp...>]

When I was at this stage (working through header file issues)
of the Xen/ia64 port, I had a long list of minor patches to
common files.  The core Xen team (Ian and Keir) asked that,
wherever possible, I handle ia64 changes _with minimal impact_
to common files/headers.   This was annoying as I had to carry
a lot of inelegant code in ia64-specific files for a long time.

Looking back, however, I think it was the right decision.  As
the Xen/ia64 code evolved (and eventually ran), many of the
common changes I asked for also changed or, in some cases,
went away.  In the end, there were a small number of patches
I needed that were unavoidable.

I'd like to suggest that this is also the right approach for
new arch's:  If something can be made to work with just changes
to arch-specific files, even unaesthetic changes, just do it.
Then later when Xen/ppc (and perhaps other arch's) are working,
we can have a "clean up the common code" fest.  In the meantime,
minor cosmetic changes that require work both in the core
and in other arch's are just keeping us from making real
progress on needed functionality and stability.

Just my opinion...
Dan

> -----Original Message-----
> From: Christian Limpach [mailto:chr...@gm...]=20
> Sent: Friday, March 25, 2005 7:15 PM
> To: Hollis Blanchard
> Cc: xen...@li...; Magenheimer, Dan (HP=20
> Labs Fort Collins)
> Subject: Re: [Xen-devel] Re: [patch] final header fixes
>=20
> On Fri, 25 Mar 2005 10:22:50 -0600, Hollis Blanchard=20
> <ho...@us...> wrote:
> > On Thu, 2005-03-24 at 21:24 +0000, Christian Limpach wrote:
> > > On Thu, 24 Mar 2005 15:03:19 -0600, Hollis Blanchard=20
> <ho...@us...> wrote:
> > > > On Thursday 24 March 2005 14:40, Dan Magenheimer wrote:
> > > > > This second patch looks good to me, though I would prefer
> > > > > to change to '#include <asm/regs.h>' from 'struct xen_regs;'
> > > > > as xen_regs is #define'd to pt_regs on ia64 and the explicit
> > > > > use of struct xen_regs in a header could cause header ordering
> > > > > problems later.  Will that work for ppc?
> > > >
> > > > Ah, sure... I was just trying to avoid extra dependency=20
> trees where possible.
> > > > Apparently it is not possible here. :)
> > > >
> > > > So that should be #include <public/xen.h> (which=20
> includes arch-*.h which
> > > > defines xen_regs). It looks like we may want to move=20
> ia64's #define to
> > > > arch-ia64.h ...
> > >
> > > Could you please include asm/regs.h or public/xen.h=20
> wherever you want
> > > to include keyhandler.h?  Thanks.
> >=20
> > That works great, until somebody other than me includes or modifies
> > keyhandler.h (or any of the other headers we've been talking about).
> >=20
> > I guess I don't see the difficulty in making sure you=20
> include what you
> > use...?
>=20
> I think we have too many header files which include other header files
> already -- to the point that a lot of .c files don't include all the
> header files they need because some random header file includes it for
> them.
>=20
> keyhandler.c is one of the header files you probably want to include
> in source code level add-on software, software which might be require
> using a header set which is not compatible with Xen's header set.
>=20
>      christian
>=20