WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] Re: [PATCH, v3]: change libxl IDL to export a saner interfac

To: Xen Devel <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: [Xen-devel] Re: [PATCH, v3]: change libxl IDL to export a saner interface for upcoming language bindings
From: Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>
Date: Fri, 10 Sep 2010 16:41:55 +0100
Cc: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>, Vincent Hanquez <vincent.hanquez@xxxxxxxxxxxxx>, Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
Delivery-date: Fri, 10 Sep 2010 08:46:57 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <1284133203.17452.163.camel@xxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <1284133203.17452.163.camel@xxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
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!

---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>

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