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] [PATCH, v3]: change libxl IDL to export a saner interface fo

To: Xen Devel <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: [Xen-devel] [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:40:03 +0100
Cc: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>, Vincent Hanquez <vincent.hanquez@xxxxxxxxxxxxx>, Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
Delivery-date: Fri, 10 Sep 2010 08:42:56 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
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>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
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@xxxxxxxxxx>

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@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel