|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] Re: [PATCH, v3]: change libxl IDL to export a saner interface for upcoming language bindings
On Fri, 2010-09-10 at 16:41 +0100, Gianni Tedesco wrote:
> On Fri, 2010-09-10 at 16:40 +0100, Gianni Tedesco wrote:
> > Since version 2:
> > - Fold in Ian Campbells patch tidying formatting and to to make
> > autogenerate_destructor=False by default
> > Since version 1:
> > - Fix endian problem with removal of pcidev->value anon union member
>
> FFS, forgot to actually update the patch! What is described above, now
> reproduced below!
:-D
>
> ---8<------------------------------------------------------------
> Firstly remove an anonymous union in libxl_device_pci structure which
> was making auto-generating language bindings more complicated than
> necessary and exporting random bits of low level ABI that libxl that
> would rather hide anyway. There is a corresponding (untested) change to
> the ocaml binding which maintains previous ml API.
>
> Secondly make the libxl_file_reference type a Builtin. This is a
> 'semantic
> correctness' issue in that libxl ABI/API won't change. But it makes it
> so that when the IDL is used to generate language bindings that a
> file_reference type is not exported.
>
> Also implement a Numeric type which all integers are derived from. Make
> sure a boolean signed/unsigned attribute is set accordingly. This is
> required to allow language bindings to correctly handle the sign bit in
> environments with arbitrarily long integers.
>
> Signed-off-by: Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>
Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
>
> diff -r 4137ca280d14 tools/libxl/idl.txt
> --- a/tools/libxl/idl.txt Fri Sep 10 15:39:33 2010 +0100
> +++ b/tools/libxl/idl.txt Fri Sep 10 16:31:07 2010 +0100
> @@ -13,7 +13,8 @@ contain the initial namespace element (e
> how to specify a namespace.
>
> The Type.typename contains the C name of the type _including_ the
> -namespace element.
> +namespace element while Type.rawname is always set to the 'base' name
> +of the type.
>
> The libxltypes.Type base class has several other properties which
> apply to all types. The properties are set by passing a named
> diff -r 4137ca280d14 tools/libxl/libxl.h
> --- a/tools/libxl/libxl.h Fri Sep 10 15:39:33 2010 +0100
> +++ b/tools/libxl/libxl.h Fri Sep 10 16:31:07 2010 +0100
> @@ -136,8 +136,10 @@
> typedef uint8_t libxl_mac[6];
>
> typedef char **libxl_string_list;
> +void libxl_string_list_destroy(libxl_string_list *sl);
>
> typedef char **libxl_key_value_list;
> +void libxl_key_value_list_destroy(libxl_key_value_list *kvl);
>
> typedef uint64_t *libxl_cpumap;
>
> @@ -172,6 +174,18 @@ typedef enum {
> NICTYPE_VIF,
> } libxl_nic_type;
>
> +typedef struct {
> + /*
> + * Path is always set if the file reference is valid. However if
> + * mapped is true then the actual file may already be unlinked.
> + */
> + char * path;
> + int mapped;
> + void * data;
> + size_t size;
> +} libxl_file_reference;
> +void libxl_file_reference_destroy(libxl_file_reference *p);
> +
> #define LIBXL_PCI_FUNC_ALL (~0U)
>
> #include "_libxl_types.h"
> @@ -227,11 +241,6 @@ int libxl_domain_shutdown(libxl_ctx *ctx
> int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid, int force);
> int libxl_domain_preserve(libxl_ctx *ctx, uint32_t domid,
> libxl_domain_create_info *info, const char *name_suffix, libxl_uuid new_uuid);
>
> -/* destructors for builtin data types */
> -void libxl_string_list_destroy(libxl_string_list *sl);
> -void libxl_key_value_list_destroy(libxl_key_value_list *kvl);
> -void libxl_file_reference_destroy(libxl_file_reference *f);
> -
> /*
> * Run the configured bootloader for a PV domain and update
> * info->kernel, info->u.pv.ramdisk and info->u.pv.cmdline as
> diff -r 4137ca280d14 tools/libxl/libxl.idl
> --- a/tools/libxl/libxl.idl Fri Sep 10 15:39:33 2010 +0100
> +++ b/tools/libxl/libxl.idl Fri Sep 10 16:31:07 2010 +0100
> @@ -6,14 +6,15 @@
> libxl_ctx = Builtin("ctx")
> libxl_uuid = Builtin("uuid")
> libxl_mac = Builtin("mac")
> -libxl_qemu_machine_type = Builtin("qemu_machine_type")
> -libxl_console_consback = Builtin("console_consback")
> -libxl_console_constype = Builtin("console_constype")
> -libxl_disk_phystype = Builtin("disk_phystype")
> -libxl_nic_type = Builtin("nic_type")
> +libxl_qemu_machine_type = Number("qemu_machine_type", namespace="libxl_")
> +libxl_console_consback = Number("console_consback", namespace="libxl_")
> +libxl_console_constype = Number("console_constype", namespace="libxl_")
> +libxl_disk_phystype = Number("disk_phystype", namespace="libxl_")
> +libxl_nic_type = Number("nic_type", namespace="libxl_")
>
> libxl_string_list = Builtin("string_list",
> destructor_fn="libxl_string_list_destroy", passby=PASS_BY_REFERENCE)
> libxl_key_value_list = Builtin("key_value_list",
> destructor_fn="libxl_key_value_list_destroy", passby=PASS_BY_REFERENCE)
> +libxl_file_reference = Builtin("file_reference",
> destructor_fn="libxl_file_reference_destroy", passby=PASS_BY_REFERENCE)
>
> libxl_cpumap = Builtin("cpumap", destructor_fn="free")
>
> @@ -79,14 +80,6 @@ libxl_domain_create_info = Struct("domai
> ("poolname", string),
> ])
>
> -libxl_file_reference = Struct("file_reference",[
> - ("path", string, False,
> -"""Path is always set if the file reference is valid. However if
> -mapped is true then the actual file may already be unlinked."""),
> - ("mapped", integer),
> - ("data", void),
> - ("size", size_t)], autogenerate_destructor=False)
> -
> libxl_domain_build_info = Struct("domain_build_info",[
> ("max_vcpus", integer),
> ("cur_vcpus", integer),
> @@ -240,17 +233,11 @@ libxl_device_net2 = Struct("device_net2"
> ])
>
> libxl_device_pci = Struct("device_pci", [
> - (None, Union(None, [("value", unsigned_integer),
> - (None, Struct(None,[("reserved1",
> BitField(unsigned_integer, 2)),
> - ("reg",
> BitField(unsigned_integer, 6)),
> - ("func",
> BitField(unsigned_integer, 3)),
> - ("dev",
> BitField(unsigned_integer, 5)),
> - ("bus",
> BitField(unsigned_integer, 8)),
> - ("reserved2",
> BitField(unsigned_integer, 7)),
> - ("enable",
> BitField(unsigned_integer, 1)),
> - ])),
> - ])
> - ),
> + ("reg", uint8),
> + ("func", uint8),
> + ("dev", uint8),
> + ("bus", uint8),
> + ("enable", bool),
> ("domain", unsigned_integer),
> ("vdevfn", unsigned_integer),
> ("vfunc_mask", unsigned_integer),
> diff -r 4137ca280d14 tools/libxl/libxl_pci.c
> --- a/tools/libxl/libxl_pci.c Fri Sep 10 15:39:33 2010 +0100
> +++ b/tools/libxl/libxl_pci.c Fri Sep 10 16:31:07 2010 +0100
> @@ -41,6 +41,31 @@
> #define PCI_BDF_SHORT "%02x:%02x.%01x"
> #define PCI_BDF_VDEVFN "%04x:%02x:%02x.%01x@%02x"
>
> +static unsigned int pcidev_value(libxl_device_pci *pcidev)
> +{
> + union {
> + unsigned int value;
> + struct {
> + unsigned int reserved1:2;
> + unsigned int reg:6;
> + unsigned int func:3;
> + unsigned int dev:5;
> + unsigned int bus:8;
> + unsigned int reserved2:7;
> + unsigned int enable:1;
> + }fields;
> + }u;
> +
> + u.value = 0;
> + u.fields.reg = pcidev->reg;
> + u.fields.func = pcidev->func;
> + u.fields.dev = pcidev->dev;
> + u.fields.bus = pcidev->bus;
> + u.fields.enable = pcidev->enable;
> +
> + return u.value;
> +}
> +
> static int pcidev_init(libxl_device_pci *pcidev, unsigned int domain,
> unsigned int bus, unsigned int dev,
> unsigned int func, unsigned int vdevfn)
> @@ -699,7 +724,7 @@ static int do_pci_add(libxl__gc *gc, uin
> }
> out:
> if (!libxl_is_stubdom(ctx, domid, NULL)) {
> - rc = xc_assign_device(ctx->xch, domid, pcidev->value);
> + rc = xc_assign_device(ctx->xch, domid, pcidev_value(pcidev));
> if (rc < 0 && (hvm || errno != ENOSYS)) {
> LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, rc, "xc_assign_device
> failed");
> return ERROR_FAIL;
> @@ -915,7 +940,7 @@ out:
> }
>
> if (!libxl_is_stubdom(ctx, domid, NULL)) {
> - rc = xc_deassign_device(ctx->xch, domid, pcidev->value);
> + rc = xc_deassign_device(ctx->xch, domid, pcidev_value(pcidev));
> if (rc < 0 && (hvm || errno != ENOSYS))
> LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, rc,
> "xc_deassign_device failed");
> }
> diff -r 4137ca280d14 tools/libxl/libxltypes.py
> --- a/tools/libxl/libxltypes.py Fri Sep 10 15:39:33 2010 +0100
> +++ b/tools/libxl/libxltypes.py Fri Sep 10 16:31:07 2010 +0100
> @@ -14,10 +14,13 @@ class Type(object):
>
> if typename is None: # Anonymous type
> self.typename = None
> + self.rawname = None
> elif self.namespace is None: # e.g. system provided types
> self.typename = typename
> + self.rawname = typename
> else:
> self.typename = self.namespace + typename
> + self.rawname = typename
>
> if self.typename is not None:
> self.destructor_fn = kwargs.setdefault('destructor_fn',
> self.typename + "_destroy")
> @@ -30,13 +33,22 @@ class Builtin(Type):
> """Builtin type"""
> def __init__(self, typename, **kwargs):
> kwargs.setdefault('destructor_fn', None)
> + kwargs.setdefault('autogenerate_destructor', False)
> Type.__init__(self, typename, **kwargs)
>
> -class UInt(Type):
> +class Number(Builtin):
> + def __init__(self, ctype, **kwargs):
> + kwargs.setdefault('namespace', None)
> + kwargs.setdefault('destructor_fn', None)
> + kwargs.setdefault('signed', False)
> + self.signed = kwargs['signed']
> + Builtin.__init__(self, ctype, **kwargs)
> +
> +class UInt(Number):
> def __init__(self, w, **kwargs):
> kwargs.setdefault('namespace', None)
> kwargs.setdefault('destructor_fn', None)
> - Type.__init__(self, "uint%d_t" % w, **kwargs)
> + Number.__init__(self, "uint%d_t" % w, **kwargs)
>
> self.width = w
>
> @@ -128,12 +140,12 @@ class Reference(Type):
>
> void = Builtin("void *", namespace = None)
> bool = Builtin("bool", namespace = None)
> -size_t = Builtin("size_t", namespace = None)
> +size_t = Number("size_t", namespace = None)
>
> -integer = Builtin("int", namespace = None)
> -unsigned_integer = Builtin("unsigned int", namespace = None)
> -unsigned = Builtin("unsigned int", namespace = None)
> -unsigned_long = Builtin("unsigned long", namespace = None)
> +integer = Number("int", namespace = None, signed = True)
> +unsigned_integer = Number("unsigned int", namespace = None)
> +unsigned = Number("unsigned int", namespace = None)
> +unsigned_long = Number("unsigned long", namespace = None)
>
> uint8 = UInt(8)
> uint16 = UInt(16)
> diff -r 4137ca280d14 tools/ocaml/libs/xl/xl_stubs.c
> --- a/tools/ocaml/libs/xl/xl_stubs.c Fri Sep 10 15:39:33 2010 +0100
> +++ b/tools/ocaml/libs/xl/xl_stubs.c Fri Sep 10 16:31:07 2010 +0100
> @@ -269,9 +269,28 @@ static int device_vfb_val(caml_gc *gc, l
>
> static int device_pci_val(caml_gc *gc, libxl_device_pci *c_val, value v)
> {
> + union {
> + unsigned int value;
> + struct {
> + unsigned int reserved1:2;
> + unsigned int reg:6;
> + unsigned int func:3;
> + unsigned int dev:5;
> + unsigned int bus:8;
> + unsigned int reserved2:7;
> + unsigned int enable:1;
> + }fields;
> + }u;
> CAMLparam1(v);
>
> - c_val->value = Int_val(Field(v, 0));
> + /* FIXME: propagate API change to ocaml */
> + u.value = Int_val(Field(v, 0));
> + c_val->reg = u.fields.reg;
> + c_val->func = u.fields.func;
> + c_val->dev = u.fields.dev;
> + c_val->bus = u.fields.bus;
> + c_val->enable = u.fields.enable;
> +
> c_val->domain = Int_val(Field(v, 1));
> c_val->vdevfn = Int_val(Field(v, 2));
> c_val->msitranslate = Bool_val(Field(v, 3));
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |