Gianni Tedesco
2010-Sep-10 15:40 UTC
[Xen-devel] [PATCH, v3]: change libxl IDL to export a saner interface for upcoming language bindings
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
---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@citrix.com>
diff -r f59b829c0b48 tools/libxl/idl.txt
--- a/tools/libxl/idl.txt Thu Sep 09 15:38:36 2010 +0100
+++ b/tools/libxl/idl.txt Thu Sep 09 17:26:21 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 f59b829c0b48 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h Thu Sep 09 15:38:36 2010 +0100
+++ b/tools/libxl/libxl.h Thu Sep 09 17:26:21 2010 +0100
@@ -172,6 +172,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"
diff -r f59b829c0b48 tools/libxl/libxl.idl
--- a/tools/libxl/libxl.idl Thu Sep 09 15:38:36 2010 +0100
+++ b/tools/libxl/libxl.idl Thu Sep 09 17:26:21 2010 +0100
@@ -6,11 +6,11 @@
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)
@@ -79,13 +79,10 @@ 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_file_reference = Builtin("file_reference",
+ autogenerate_destructor=False,
+
destructor_fn="libxl_file_reference_destroy",
+ passby=PASS_BY_REFERENCE)
libxl_domain_build_info = Struct("domain_build_info",[
("max_vcpus", integer),
@@ -240,17 +237,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 f59b829c0b48 tools/libxl/libxl_pci.c
--- a/tools/libxl/libxl_pci.c Thu Sep 09 15:38:36 2010 +0100
+++ b/tools/libxl/libxl_pci.c Thu Sep 09 17:26:21 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 f59b829c0b48 tools/libxl/libxltypes.py
--- a/tools/libxl/libxltypes.py Thu Sep 09 15:38:36 2010 +0100
+++ b/tools/libxl/libxltypes.py Thu Sep 09 17:26:21 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")
@@ -32,11 +35,19 @@ class Builtin(Type):
kwargs.setdefault(''destructor_fn'', None)
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 +139,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 f59b829c0b48 tools/ocaml/libs/xl/xl_stubs.c
--- a/tools/ocaml/libs/xl/xl_stubs.c Thu Sep 09 15:38:36 2010 +0100
+++ b/tools/ocaml/libs/xl/xl_stubs.c Thu Sep 09 17:26:21 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@lists.xensource.com
http://lists.xensource.com/xen-devel
Gianni Tedesco
2010-Sep-10 15:41 UTC
[Xen-devel] Re: [PATCH, v3]: change libxl IDL to export a saner interface for upcoming language bindings
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 memberFFS, forgot to actually update the patch! What is described above, now reproduced below! ---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@citrix.com> 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@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2010-Sep-10 15:43 UTC
[Xen-devel] Re: [PATCH, v3]: change libxl IDL to export a saner interface for upcoming language bindings
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.... doesn''t look like you did, or you sent the v2 patch again by mistake. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2010-Sep-10 15:56 UTC
[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@citrix.com>Acked-by: Ian Campbell <ian.campbell@citrix.com>> > 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@lists.xensource.com http://lists.xensource.com/xen-devel