From mboxrd@z Thu Jan 1 00:00:00 1970 From: Matthew Fioravante Subject: Re: [PATCH xm/xl enhancements for vptm 5/6] make devid a libxl type Date: Tue, 25 Sep 2012 12:23:38 -0400 Message-ID: <5061DA8A.3080508@jhuapl.edu> References: <505CBDFD.5070408@jhuapl.edu> <1348569397.3452.180.camel@zakaz.uk.xensource.com> <5061D52B.9080503@jhuapl.edu> <1348589660.12592.19.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0023937725986436489==" Return-path: In-Reply-To: <1348589660.12592.19.camel@zakaz.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: "xen-devel@lists.xensource.com" List-Id: xen-devel@lists.xenproject.org This is a cryptographically signed message in MIME format. --===============0023937725986436489== Content-Type: multipart/signed; protocol="application/pkcs7-signature"; micalg=sha1; boundary="------------ms000301000008030904040501" This is a cryptographically signed message in MIME format. --------------ms000301000008030904040501 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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 prope= rly. >>>> The devid field for each device is set to be an integer type which a= re >>>> always initialized to 0. >>>> >>>> This makes the xl DEV-attach commands always fail to add more than o= ne >>>> 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 netw= ork >>> cases and couldn't find it. >> libxl.c >> >> void libxl__device_nic_add( >> >> if (nic->devid =3D=3D -1) { >> if (!(dompath =3D libxl__xs_get_dompath(gc, domid))) { >> rc =3D ERROR_FAIL; >> goto out_free; >> } >> if (!(l =3D libxl__xs_directory(gc, XBT_NULL, >> libxl__sprintf(gc, "%s/device/vif= ", >> dompath), &nb))) { >> nic->devid =3D 0; >> } else { >> nic->devid =3D 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 =3D Xsraw.perms >>>> =20 >>>> type domid =3D int >>>> +type devid =3D int >>> I can see why these were needed in the xl modules, but I don't see ho= w >>> 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 han= d > 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 =2Emli 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@jhuapl.edu --- * 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 =3D " ", parent =3D = None): passby=3Didl.PASS_BY_REFERENCE))= elif ty.typename in ["libxl_uuid", "libxl_mac", "libxl_hwcap"]: s +=3D "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 +=3D "%s =3D 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 *cpu= id_list); #define LIBXL_PCI_FUNC_ALL (~0U) =20 typedef uint32_t libxl_domid; +typedef int libxl_devid; =20 /* * 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 =3D Builtin("defbool", passby=3DPASS_BY_REFERENCE) =20 libxl_domid =3D Builtin("domid", json_fn =3D "yajl_gen_integer", autogen= erate_json =3D False) +libxl_devid =3D Builtin("devid", json_fn =3D "yajl_gen_integer", autogen= erate_json =3D False, signed =3D True, init_val=3D"-1") libxl_uuid =3D Builtin("uuid", passby=3DPASS_BY_REFERENCE) libxl_mac =3D Builtin("mac", passby=3DPASS_BY_REFERENCE) libxl_bitmap =3D Builtin("bitmap", dispose_fn=3D"libxl_bitmap_dispose", = passby=3DPASS_BY_REFERENCE) @@ -343,7 +344,7 @@ libxl_domain_build_info =3D Struct("domain_build_info= ",[ =20 libxl_device_vfb =3D 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 =3D Struct("device_vfb", [ =20 libxl_device_vkb =3D Struct("device_vkb", [ ("backend_domid", libxl_domid), - ("devid", integer), + ("devid", libxl_devid), ]) =20 libxl_device_disk =3D Struct("device_disk", [ @@ -369,7 +370,7 @@ libxl_device_disk =3D Struct("device_disk", [ =20 libxl_device_nic =3D 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 =3D 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 =3D 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= =2Epy --- a/tools/ocaml/libs/xl/genwrap.py +++ b/tools/ocaml/libs/xl/genwrap.py @@ -10,6 +10,7 @@ builtins =3D { "int": ("int", "%(c)s =3D Int_va= l(%(o)s)", "Val_int(%(c)s)" ), "char *": ("string", "%(c)s =3D dup_St= ring_val(gc, %(o)s)", "caml_copy_string(%(c)s)"), "libxl_domid": ("domid", "%(c)s =3D Int_va= l(%(o)s)", "Val_int(%(c)s)" ), + "libxl_devid": ("devid", "%(c)s =3D Int_va= l(%(o)s)", "Val_int(%(c)s)" ), "libxl_defbool": ("bool option", "%(c)s =3D Defboo= l_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) =20 def ocaml_type_of(ty): - if ty.rawname =3D=3D "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/xen= light.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 =20 type domid =3D int +type devid =3D int =20 (* @@LIBXL_TYPES@@ *) =20 diff --git a/tools/ocaml/libs/xl/xenlight.mli.in b/tools/ocaml/libs/xl/xe= nlight.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 =20 type domid =3D int +type devid =3D int =20 (* @@LIBXL_TYPES@@ *) =20 diff --git a/tools/python/xen/lowlevel/xl/xl.c b/tools/python/xen/lowleve= l/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; } =20 +int attrib__libxl_devid_set(PyObject *v, libxl_devid *devid) { + *devid =3D 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); } =20 +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"); --=20 1.7.4.4 --------------ms000301000008030904040501 Content-Type: application/pkcs7-signature; name="smime.p7s" Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="smime.p7s" Content-Description: S/MIME Cryptographic Signature MIAGCSqGSIb3DQEHAqCAMIACAQExCzAJBgUrDgMCGgUAMIAGCSqGSIb3DQEHAQAAoIIDyjCC A8YwggMvoAMCAQICBD/xyf0wDQYJKoZIhvcNAQEFBQAwLzELMAkGA1UEBhMCVVMxDzANBgNV BAoTBkpIVUFQTDEPMA0GA1UECxMGQklTRENBMB4XDTEwMDYxMTE4MjIwNloXDTEzMDYxMTE4 NTIwNlowZjELMAkGA1UEBhMCVVMxDzANBgNVBAoTBkpIVUFQTDEPMA0GA1UECxMGUGVvcGxl MTUwFgYDVQQLEw9WUE5Hcm91cC1CSVNEQ0EwGwYDVQQDExRNYXR0aGV3IEUgRmlvcmF2YW50 ZTCBnzANBgkqhkiG9w0BAQEFAAOBjQAwgYkCgYEAnpbwVSP6o1Nb5lcW7dd3yTo9iBJdi7qz 4nANOMFPK7JOy5npKN1iiousl28U/scUJES55gPwAWYJK3uVyQAsA4adgDKi5DoD1UHDQEwp bY7iHLJeq0NPr4BqYNqnCFPbE6HC8zSJrr4qKn+gVUQT39SIFqdiIPJwZL8FYTRQ/zsCAwEA AaOCAbYwggGyMAsGA1UdDwQEAwIHgDArBgNVHRAEJDAigA8yMDEwMDYxMTE4MjIwNlqBDzIw MTIwNzE3MjI1MjA2WjAbBg0rBgEEAbMlCwMBAQEBBAoWCGZpb3JhbWUxMBsGDSsGAQQBsyUL AwEBAQIEChIIMDAxMDQyNjEwWAYJYIZIAYb6ax4BBEsMSVRoZSBwcml2YXRlIGtleSBjb3Jy ZXNwb25kaW5nIHRvIHRoaXMgY2VydGlmaWNhdGUgbWF5IGhhdmUgYmVlbiBleHBvcnRlZC4w KAYDVR0RBCEwH4EdTWF0dGhldy5GaW9yYXZhbnRlQGpodWFwbC5lZHUwUgYDVR0fBEswSTBH oEWgQ6RBMD8xCzAJBgNVBAYTAlVTMQ8wDQYDVQQKEwZKSFVBUEwxDzANBgNVBAsTBkJJU0RD QTEOMAwGA1UEAxMFQ1JMNTYwHwYDVR0jBBgwFoAUCDUpmxH52EU2CyWmF2EJMB1yqeswHQYD VR0OBBYEFO6LYxg6r9wHZ+zdQtBHn1dZ/YTNMAkGA1UdEwQCMAAwGQYJKoZIhvZ9B0EABAww ChsEVjcuMQMCBLAwDQYJKoZIhvcNAQEFBQADgYEAJO9HQh4YNChVLzuZqK5ARJARD8JoujGZ fdo75quvg2jXFQe2sEjvLnxJZgm/pv8fdZakq48CWwjYHKuvIp7sDjTEsQfo+y7SpN/N2NvJ WU5SqfK1VgYtNLRRoGJUB5Q1aZ+Dg95g3kqpyfpUMISJL8IKVLtJVfN4fggFVUYZ9wwxggGr MIIBpwIBATA3MC8xCzAJBgNVBAYTAlVTMQ8wDQYDVQQKEwZKSFVBUEwxDzANBgNVBAsTBkJJ U0RDQQIEP/HJ/TAJBgUrDgMCGgUAoIHLMBgGCSqGSIb3DQEJAzELBgkqhkiG9w0BBwEwHAYJ KoZIhvcNAQkFMQ8XDTEyMDkyNTE2MjMzOFowIwYJKoZIhvcNAQkEMRYEFDQGxeiHvt9t7cG4 ycfIC015MquzMGwGCSqGSIb3DQEJDzFfMF0wCwYJYIZIAWUDBAEqMAsGCWCGSAFlAwQBAjAK BggqhkiG9w0DBzAOBggqhkiG9w0DAgICAIAwDQYIKoZIhvcNAwICAUAwBwYFKw4DAgcwDQYI KoZIhvcNAwICASgwDQYJKoZIhvcNAQEBBQAEgYCEqiQVmQfKhuTWqwJJZnwec5bYmGSztt7c RaYvMxgfPUr0NgfyRnQiA/nC7BDQU5145WJpIzoslDrW76Ce4/UsV/gr2V31jQNbpds/CSWn N6d734QSedr01ajKbyK5LozCQf9DEo2SvyGOJzCnJBBx3NhwOxFE04QN2DGujAApPwAAAAAA AA== --------------ms000301000008030904040501-- --===============0023937725986436489== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============0023937725986436489==--