[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 10/11] make devid a type so it is initialized properly



On Fri, 2012-09-28 at 14:23 +0100, George Dunlap wrote:
> On Thu, Sep 27, 2012 at 6:10 PM, Matthew Fioravante
> <matthew.fioravante@xxxxxxxxxx> wrote:
> > Previously device ids in libxl were treated as integers meaning they
> > were being initialized to 0, which is a valid device id. This patch
> > makes devid its own type in libxl and initializes it to -1, an invalid
> > value.
> >
> > This fixes a bug where if you try to do a xl DEV-attach multiple
> > time it will continuously try to reattach device 0 instead of
> > generating a new device id.
> >
> > Signed-off-by: Matthew Fioravante <matthew.fioravante@xxxxxxxxxx>
> 
> Reviewed-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx>

Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

> Should this be backported to 4.2-testing?

That's a good question. I think it is basically a bugfix to the API and
since basically the only way to correctly use the current interface is
for the application to always initialise that field changing the initial
value seems harmless from a backwards compat PoV.

Ian.

> 
>  -George
> 
> >
> > diff --git a/tools/libxl/gentest.py b/tools/libxl/gentest.py
> > index 2915f71..84b4fd7 100644
> > --- 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
> > index 599c7f1..7a7c419 100644
> > --- 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
> > index cf83c60..de111a6 100644
> > --- 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
> > index 42f374e..97d088d 100644
> > --- 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
> > index c47623c..dcc1a38 100644
> > --- 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
> > index 4717bac..3fd0165 100644
> > --- 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
> > index 0551c76..32f982a 100644
> > --- 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.9.5
> >
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@xxxxxxxxxxxxx
> > http://lists.xen.org/xen-devel



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.