[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH xm/xl enhancements for vptm 5/6] make devid a libxl type
On 09/25/2012 12:14 PM, Ian Campbell wrote: > On Tue, 2012-09-25 at 17:00 +0100, Matthew Fioravante wrote: >> On 09/25/2012 06:36 AM, Ian Campbell wrote: >>> On Fri, 2012-09-21 at 20:20 +0100, Matthew Fioravante wrote: >>>> This fixes a bug in libxl where device ids are not initialized properly. >>>> The devid field for each device is set to be an integer type which are >>>> always initialized to 0. >>>> >>>> This makes the xl DEV-attach commands always fail to add more than one >>>> device, because each time the device id is initialized to 0, when it >>>> should be initialized to -1, which in the code will trigger a search for >>>> free id. >>> Which of the DEV-attach commands can be used to add more than one >>> device? >> network-attach, block-attach, and also my vtpm-attach. > Oh, you mean by being called repeatedly, not adding multiple devices in > one go! > >>> Where is the code to do the search? I had a look in the disk and network >>> cases and couldn't find it. >> libxl.c >> >> void libxl__device_nic_add( >> >> if (nic->devid == -1) { >> if (!(dompath = libxl__xs_get_dompath(gc, domid))) { >> rc = ERROR_FAIL; >> goto out_free; >> } >> if (!(l = libxl__xs_directory(gc, XBT_NULL, >> libxl__sprintf(gc, "%s/device/vif", >> dompath), &nb))) { >> nic->devid = 0; >> } else { >> nic->devid = strtoul(l[nb - 1], NULL, 10) + 1; >> } >> } > Not sure how I missed this. > >> Right now, nic-devid is 0 on attach. > Is devid not a parameter to network-attach? It is to block-attach. It might be, but if its not specified I think the logical thing to do is to automatically choose the next available id. > >> So it always tries to use 0. When you do a network-attach twice, >> the second time it trys and fails to create device/vif/0 > Your change to use -1 as thye default is certainly correct. I'm just > trying to figure out why xl does things this way in the first place. > >>>> diff --git a/tools/ocaml/libs/xs/xs.mli b/tools/ocaml/libs/xs/xs.mli >>>> --- a/tools/ocaml/libs/xs/xs.mli >>>> +++ b/tools/ocaml/libs/xs/xs.mli >>>> @@ -27,6 +27,7 @@ exception Failed_to_connect >>>> type perms = Xsraw.perms >>>> >>>> type domid = int >>>> +type devid = int >>> I can see why these were needed in the xl modules, but I don't see how >>> this type can be required in the xenstore ones. >> It shouldn't be required for xenstore or xc. The problem is they won't >> compile without it because of the way the ocaml build system works. As >> far as I understand it creates types for xl, xc, and xs from the >> libxl_types.idl. > No, only xl does this. > > The others are the libxc and xenstore bindings which are completely hand > coded and there is no concept of a devid at those layers anyway. What > error do you get if you omit the changes to those module? Ok I must be crazy. I went back and removed them and tried to compile it again and it works fine. I think I was forgetting to update both the .mli and the .ml files. Anyway a new patch with the xc/xs removed is attached. > >> Either the build system needs to be changed, or devid >> needs to be a type in all libraries, or we find some other way to fix >> this initialization bug. >>> Ian. >>> >> > Signed off by Matthew Fioravante matthew.fioravante@xxxxxxxxxx --- * Rebased off of latest xen-unstable * Fixed whitespace errors * Removed devid from libxc and libxs diff --git a/tools/libxl/gentest.py b/tools/libxl/gentest.py --- a/tools/libxl/gentest.py +++ b/tools/libxl/gentest.py @@ -60,7 +60,7 @@ def gen_rand_init(ty, v, indent = " ", parent = None): passby=idl.PASS_BY_REFERENCE)) elif ty.typename in ["libxl_uuid", "libxl_mac", "libxl_hwcap"]: s += "rand_bytes((uint8_t *)%s, sizeof(*%s));\n" % (v,v) - elif ty.typename in ["libxl_domid"] or isinstance(ty, idl.Number): + elif ty.typename in ["libxl_domid", "libxl_devid"] or isinstance(ty, idl.Number): s += "%s = rand() %% (sizeof(%s)*8);\n" % \ (ty.pass_arg(v, parent is None), ty.pass_arg(v, parent is None)) diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h --- a/tools/libxl/libxl.h +++ b/tools/libxl/libxl.h @@ -307,6 +307,7 @@ void libxl_cpuid_dispose(libxl_cpuid_policy_list *cpuid_list); #define LIBXL_PCI_FUNC_ALL (~0U) typedef uint32_t libxl_domid; +typedef int libxl_devid; /* * Formatting Enumerations. diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl --- a/tools/libxl/libxl_types.idl +++ b/tools/libxl/libxl_types.idl @@ -8,6 +8,7 @@ namespace("libxl_") libxl_defbool = Builtin("defbool", passby=PASS_BY_REFERENCE) libxl_domid = Builtin("domid", json_fn = "yajl_gen_integer", autogenerate_json = False) +libxl_devid = Builtin("devid", json_fn = "yajl_gen_integer", autogenerate_json = False, signed = True, init_val="-1") libxl_uuid = Builtin("uuid", passby=PASS_BY_REFERENCE) libxl_mac = Builtin("mac", passby=PASS_BY_REFERENCE) libxl_bitmap = Builtin("bitmap", dispose_fn="libxl_bitmap_dispose", passby=PASS_BY_REFERENCE) @@ -343,7 +344,7 @@ libxl_domain_build_info = Struct("domain_build_info",[ libxl_device_vfb = Struct("device_vfb", [ ("backend_domid", libxl_domid), - ("devid", integer), + ("devid", libxl_devid), ("vnc", libxl_vnc_info), ("sdl", libxl_sdl_info), # set keyboard layout, default is en-us keyboard @@ -352,7 +353,7 @@ libxl_device_vfb = Struct("device_vfb", [ libxl_device_vkb = Struct("device_vkb", [ ("backend_domid", libxl_domid), - ("devid", integer), + ("devid", libxl_devid), ]) libxl_device_disk = Struct("device_disk", [ @@ -369,7 +370,7 @@ libxl_device_disk = Struct("device_disk", [ libxl_device_nic = Struct("device_nic", [ ("backend_domid", libxl_domid), - ("devid", integer), + ("devid", libxl_devid), ("mtu", integer), ("model", string), ("mac", libxl_mac), @@ -399,7 +400,7 @@ libxl_diskinfo = Struct("diskinfo", [ ("backend_id", uint32), ("frontend", string), ("frontend_id", uint32), - ("devid", integer), + ("devid", libxl_devid), ("state", integer), ("evtch", integer), ("rref", integer), @@ -410,7 +411,7 @@ libxl_nicinfo = Struct("nicinfo", [ ("backend_id", uint32), ("frontend", string), ("frontend_id", uint32), - ("devid", integer), + ("devid", libxl_devid), ("state", integer), ("evtch", integer), ("rref_tx", integer), diff --git a/tools/ocaml/libs/xl/genwrap.py b/tools/ocaml/libs/xl/genwrap.py --- a/tools/ocaml/libs/xl/genwrap.py +++ b/tools/ocaml/libs/xl/genwrap.py @@ -10,6 +10,7 @@ builtins = { "int": ("int", "%(c)s = Int_val(%(o)s)", "Val_int(%(c)s)" ), "char *": ("string", "%(c)s = dup_String_val(gc, %(o)s)", "caml_copy_string(%(c)s)"), "libxl_domid": ("domid", "%(c)s = Int_val(%(o)s)", "Val_int(%(c)s)" ), + "libxl_devid": ("devid", "%(c)s = Int_val(%(o)s)", "Val_int(%(c)s)" ), "libxl_defbool": ("bool option", "%(c)s = Defbool_val(%(o)s)", "Val_defbool(%(c)s)" ), "libxl_uuid": ("int array", "Uuid_val(gc, lg, &%(c)s, %(o)s)", "Val_uuid(&%(c)s)"), "libxl_key_value_list": ("(string * string) list", None, None), @@ -41,8 +42,8 @@ def stub_fn_name(ty, name): return "stub_xl_%s_%s" % (ty.rawname,name) def ocaml_type_of(ty): - if ty.rawname == "domid": - return "domid" + if ty.rawname in ["domid","devid"]: + return ty.rawname elif isinstance(ty,idl.UInt): if ty.width in [8, 16]: # handle as ints diff --git a/tools/ocaml/libs/xl/xenlight.ml.in b/tools/ocaml/libs/xl/xenlight.ml.in --- a/tools/ocaml/libs/xl/xenlight.ml.in +++ b/tools/ocaml/libs/xl/xenlight.ml.in @@ -16,6 +16,7 @@ exception Error of string type domid = int +type devid = int (* @@LIBXL_TYPES@@ *) diff --git a/tools/ocaml/libs/xl/xenlight.mli.in b/tools/ocaml/libs/xl/xenlight.mli.in --- a/tools/ocaml/libs/xl/xenlight.mli.in +++ b/tools/ocaml/libs/xl/xenlight.mli.in @@ -16,6 +16,7 @@ exception Error of string type domid = int +type devid = int (* @@LIBXL_TYPES@@ *) diff --git a/tools/python/xen/lowlevel/xl/xl.c b/tools/python/xen/lowlevel/xl/xl.c --- a/tools/python/xen/lowlevel/xl/xl.c +++ b/tools/python/xen/lowlevel/xl/xl.c @@ -281,6 +281,11 @@ int attrib__libxl_domid_set(PyObject *v, libxl_domid *domid) { return 0; } +int attrib__libxl_devid_set(PyObject *v, libxl_devid *devid) { + *devid = PyInt_AsLong(v); + return 0; +} + int attrib__struct_in_addr_set(PyObject *v, struct in_addr *pptr) { PyErr_SetString(PyExc_NotImplementedError, "Setting in_addr"); @@ -342,6 +347,10 @@ PyObject *attrib__libxl_domid_get(libxl_domid *domid) { return PyInt_FromLong(*domid); } +PyObject *attrib__libxl_devid_get(libxl_devid *devid) { + return PyInt_FromLong(*devid); +} + PyObject *attrib__struct_in_addr_get(struct in_addr *pptr) { PyErr_SetString(PyExc_NotImplementedError, "Getting in_addr"); -- 1.7.4.4 Attachment:
smime.p7s _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |