On Thu, 2010-09-09 at 12:48 +0100, Ian Campbell wrote:
> On Thu, 2010-09-09 at 12:18 +0100, Gianni Tedesco wrote:
> > Changes since last time:
> > - split auto-generated code in to c and h files
> > - un-break the build system
> > - fix ocaml binding due to libxl API change
> > - lot's of tidy-ups too numerous to mention
> >
> > Please consider and apply :)
>
> Like it.
>
> I think it'd be nice to separate out stuff like the PCI API change and
> IDL changes into preparatory patches. Patches which do one logical thing
> are generally easier to review (and useful for bisecting and such too).
Yeah, that's not a problem, in this case I can just split it out by
filename.
> > -----8<---------------------------------------------------------------
> > Introduce python binding for libxl. The binding is not yet complete but
> > list_domains, domid_to_name, domain_shutdown and domain_destroy methods
> > are implemented and tested. These functions provide examples of the two
> > basic patterns that all future methods should follow.
> >
> > About 5,000 lines of boilerplate is automatically generated to wrap and
> > export all relevant libxl structure definitions. There are a few places
> > where such code cannot be fully auto-generated and special hooks are
> > declared and stubbed where, for example, conversion between
> > libxl_file_reference and a python file object is required.
>
> I think this is acceptable for Builtin() types and is really part of the
> meaning of builtin. (except as I read on I realise that
> libxl_file_reference isn't actually a Builtin like I thought, more on
> that later!)
>
> > There are minor changes to libxltypes.py and the libxl.idl file and the
> > new rules are documented. I have also removed the use of a nasty union
> > in libxl_device_pci structure which was making the binding more
> > complicated than necessary and for no good reason (exporting random bits
> > of low level ABI that libxl would rather hide anyway). Finally there is
> > a corresponding (untested) change to the ocaml binding to fix a compile
> > error and maintain previous API in ocaml.
> >
> > Signed-off-by: Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>
>
> > +Type.internal (default: False):
> > + Indicates that type is not suitable for export, for example, via accessors
> > + in auto-generated language bindings.
>
> Maybe the issue here is that libxl_file_reference should really be a
> Builtin? It's suspicious that it is the only non-Builtin to which both
> this flag and autogenerate_destructor=False apply.
I think your analysis is right and this in fact works for me. I had
misunderstood full meaning of Builtins even though you had mentioned the
(builtins,types) tuple thing. Fixed this and it works fine.
> If that doesn't make sense then perhaps a new sub-class of Builtin might
> be more appropriate than an attribute (not sure what it would be called
> though, SuperBuiltin ;-)). I'm not sure that a non-Builtin should ever
> be internal and this would help prevent that.
Meh
> > diff -r b19856f6dd76 tools/python/Makefile
> > --- a/tools/python/Makefile Thu Sep 09 09:24:24 2010 +0100
> > +++ b/tools/python/Makefile Thu Sep 09 12:06:50 2010 +0100
> > @@ -20,6 +20,10 @@ genpath-target = $(call buildmakevars2fi
> >
> > .PHONY: build buildpy
> > buildpy: genpath
> > + PYTHONPATH=$(XEN_ROOT)/tools/libxl $(PYTHON) genwrap.py \
> > + $(XEN_ROOT)/tools/libxl/libxl.idl \
> > + xen/lowlevel/xl/_pyxl_types.h \
> > + xen/lowlevel/xl/_pyxl_types.c
> > CC="$(CC)" CFLAGS="$(CFLAGS)" $(PYTHON) setup.py build
>
> Needs some additional dependencies on genwrap.py and
> $(XE_ROOT)/tools/libxl/libxl.idl?
Yes, and libxltypes.py, fixed.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home