[Xen-devel] Re: [PATCH] ocaml: xc bindings: use libxenctrl and libxenguest

[Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>]

On Fri, 2010-09-10 at 16:23 +0100, Vincent Hanquez wrote:
> On 10/09/10 11:10, Ian Campbell wrote:
> > # HG changeset patch
> > # User Ian Campbell<ian.campbell@xxxxxxxxxx>
> > # Date 1284113402 -3600
> > # Node ID c2610f86abfb2c34a5a653dea29d5518fb355628
> > # Parent  8a710e0eb0881cad6156500bd4cedcebc7824a18
> > ocaml: xc bindings: use libxenctrl and libxenguest
> >
> > Now that tools/libxc is licensed under LGPL I don't think there is any
> > need for an LGPL reimplementation under tools/ocaml.
> >
> > For the most part the conversion to the up-to-date libxc API (xc_lib.c
> > essentially implemented the same interface as an older libxc) was
> > pretty automatic. There are some functions which appear to no longer
> > exist in libxc which I therefore simply removed the bindings for and a
> > small number of interfaces which had changed.
> >
> > Many of the functions bound by the stubs have no in-tree users (which
> > I think is fine for a language binding) so I have no way to confirm
> > correctness other than by eye. I was however able to confirm that
> > oxenstored still worked.
> >    
> I can't say, i'm particularly thrilled by this patch.
> 
> on one hand, it renders oxenstored runnable on netbsd for example and 
> merge libxc into one.
> on the other hand, it's a pretty big change and done only "one-sidedly":
> 
> - lose the logging improvement over libxenctrl version.

libxenctrl has infrastructure to record and retrieve the last error --
it could perhaps with being more widely/consistently used. This would be
a good project for someone.

I don't think maintaining a fork of a library just to improve logging is
worthwhile though.

> - some bindings get lost, need to adapt all clients.

These looked to me like they should be replaced by libxl bindings
anyway. e.g. the s3 event stuff, timer stuff etc.

I don't think the ocaml binding to libxc needs to have any more API
stability guarantee than the underlying C library (which is very
little).

Also the version which is in the Xen source tree doesn't need to
maintain source compatibility with previous Xen releases (e.g. this
patch removes an interface around XEN_DOMCTL_setvmxassist which no
longer exists), we don't do this for the C library either.

> - introduce pthread functions in the ocaml stack, which as discussed in 
> the past, might be a dealkiller, but unfortunately very hard to test.

The hypercall buffer patchset which I am working on removes one of the
two uses of pthreads in libxc.

The other case uses the pthread_key stuff to provide a threadsafe
strerror for use by error logging (which perhaps ties into your first
comment). I've no idea why this code doesn't just use strerror_r.

...

OK, so, reading the hg history it looks like using strerror_r was made
difficult (in 2006) by an incompatible GNU version of strerror_r which
didn't have the same prototype nor semantics as the POSIX specified
strerror_r.

I'm not sure we need to care about that anymore, it seems to have been
fixed in glibc 2.4 (For comparison Debian Lenny has 2.7) so I think we
could consider switching back to just plain strerror_r at this point
which would completely remove pthreads from libxc.

> - almost completely not tested patch.
> 
> oxenstored won't be affected much by all this, since it's using only a 
> tiny part of xc, but i suspect it might be a big deal for XCP and such, 
> and thus i wonder if XCP is going to use this version of xc (which was 
> the goal when merging those libraries alongside oxenstored). it would 
> probably be easier to test this in XCP first, since there's a 
> comprehensive test suite there.

Hmm. perhaps.

I think we could switch xen-unstable over now and then whenever XCP
updates to a new enough Xen and moves over to using this version of the
bindings deal with any problems there (to be honest I expect that issues
arising from this particular change will be a small fraction of the work
required to make that transition).

Ian.


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