Re: [Xen-devel] [PATCH xm/xl enhancements for vptm 5/6] make devid a libxl type

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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.

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

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



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