Re: [Xen-devel] [PATCH 2/6] libxl: portiblity fixes

[Christoph Egger <Christoph.Egger@xxxxxxx>]

On Wednesday 28 July 2010 11:21:54 Keir Fraser wrote:
> On 28/07/2010 10:06, "Christoph Egger" <Christoph.Egger@xxxxxxx> wrote:
> >> This patch is wrong because it introduces a couple of function
> >> declarations but it does not introduce the definitions; your later
> >> patch which introduces the definitions is wrong because it introduces
> >> some functions which are intended to replace existing code, but the
> >> patch does not replace the existing code and the new functions are not
> >> called anywhere in that patch.
> >
> > The function declarations are the API and the function defintions
> > are the OS dependent implementations of the API.
> > Implementations and use of the API is used in different patches.
> > This is my understanding of defining and implementing an API
> > in C.
>
> I find that kind of way of splitting up a patch series annoying as well. As
> Ian said, we want each patch to be a logical and separate whole. That means
> providing an interface *and* its implementation. Possibly its users as
> well, depending on how complicated that bit is -- it's certainly arguable
> they belong in a separate patch, at least.
>
> > blktap support for linux and netbsd are very different in their
> > implementation.
> > In netbsd, blktap will be implemented using puffs
> > (http://netbsd.gw.com/cgi-bin/man-cgi?puffs+3+NetBSD-current)
>
> A bit of a sidestep I know, but: shouldn't the blktap library be hiding
> this osdep stuff?

That's a good point. With this in mind, having libxl_blktap.c and 
libxl_noblktap.c as the Ian's suggested makes perfect sense to me.

Christoph

-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel