Re: [Xen-devel] [PATCH 1/4] a fix in libxl_device_usbdev_list

[Ian Jackson]

Ian Jackson writes ("Re: [PATCH 1/4] a fix in libxl_device_usbdev_list"):
> We won't fix that bad API pattern for 4.7.  So
> 
> Acked-by: Ian Jackson 

Queued.

Ian.

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

Re: [Xen-devel] [PATCH 1/4] a fix in libxl_device_usbdev_list

[Ian Jackson]

Chun Yan Liu writes ("Re: [PATCH 1/4] a fix in libxl_device_usbdev_list"):
> On 4/8/2016 at 12:45 AM, in message
> <22278.36492.245114.295...@mariner.uk.xensource.com>, Ian Jackson
>  wrote: 
> > If libxl__device_usbdev_list_for_usbctrl fails, shouldn't 
> > libxl_device_usbdev_list fail too ? 
> 
> Following the similar definitions of other device types, the return value of
> this function is "libxl_device_usbdev *", to treat the above case as failure,
> it cannot be reflected through return value, we can only set return value
> to NULL, but that will be confusing with a real no-device case.

Urk.  Yes, you're right.  *sigh*

We won't fix that bad API pattern for 4.7.  So

Acked-by: Ian Jackson 

Thanks,
Ian.

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

Re: [Xen-devel] [PATCH 1/4] a fix in libxl_device_usbdev_list

[Chun Yan Liu]

>>> On 4/8/2016 at 12:45 AM, in message
<22278.36492.245114.295...@mariner.uk.xensource.com>, Ian Jackson
 wrote: 
> Chunyan Liu writes ("[PATCH 1/4] a fix in libxl_device_usbdev_list"): 
> > In testing with libvirt pvusb functionality, found a rc check 
> > error in libxl_device_usbdev_list. Correct it. 
>  
> Thanks.  But now that I look at this code I'm not sure your fix is 
> complete. 
>  
> > diff --git a/tools/libxl/libxl_pvusb.c b/tools/libxl/libxl_pvusb.c 
> > index 5f92628..04e41b4 100644 
> > --- a/tools/libxl/libxl_pvusb.c 
> > +++ b/tools/libxl/libxl_pvusb.c 
> > @@ -701,13 +701,13 @@ libxl_device_usbdev_list(libxl_ctx *ctx, uint32_t  
> domid, int *num) 
> >  usbctrls = libxl__xs_directory(gc, XBT_NULL, path, ); 
> >   
> >  for (i = 0; i < nc; i++) { 
> > -int r, nd = 0; 
> > +int rc, nd = 0; 
> >  libxl_device_usbdev *tmp = NULL; 
> >   
> > -r = libxl__device_usbdev_list_for_usbctrl(gc, domid, 
> > +rc = libxl__device_usbdev_list_for_usbctrl(gc, domid, 
> >atoi(usbctrls[i]), 
> >, ); 
> > -if (!r || !nd) continue; 
> > +if (rc || !nd) continue; 
>  
> If libxl__device_usbdev_list_for_usbctrl fails, shouldn't 
> libxl_device_usbdev_list fail too ? 

Following the similar definitions of other device types, the return value of
this function is "libxl_device_usbdev *", to treat the above case as failure,
it cannot be reflected through return value, we can only set return value
to NULL, but that will be confusing with a real no-device case.

- Chunyan
>  
> Ian. 
>  
>  



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

Re: [Xen-devel] [PATCH 1/4] a fix in libxl_device_usbdev_list

[Ian Jackson]

Chunyan Liu writes ("[PATCH 1/4] a fix in libxl_device_usbdev_list"):
> In testing with libvirt pvusb functionality, found a rc check
> error in libxl_device_usbdev_list. Correct it. This function
> is not used by xl.

Thanks for these.  I have applied 2,3, and 4.  But not 1, as I had
comments on it.

Regards,
Ian.

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

Re: [Xen-devel] [PATCH 1/4] a fix in libxl_device_usbdev_list

[Ian Jackson]

Chunyan Liu writes ("[PATCH 1/4] a fix in libxl_device_usbdev_list"):
> In testing with libvirt pvusb functionality, found a rc check
> error in libxl_device_usbdev_list. Correct it.

Thanks.  But now that I look at this code I'm not sure your fix is
complete.

> diff --git a/tools/libxl/libxl_pvusb.c b/tools/libxl/libxl_pvusb.c
> index 5f92628..04e41b4 100644
> --- a/tools/libxl/libxl_pvusb.c
> +++ b/tools/libxl/libxl_pvusb.c
> @@ -701,13 +701,13 @@ libxl_device_usbdev_list(libxl_ctx *ctx, uint32_t 
> domid, int *num)
>  usbctrls = libxl__xs_directory(gc, XBT_NULL, path, );
>  
>  for (i = 0; i < nc; i++) {
> -int r, nd = 0;
> +int rc, nd = 0;
>  libxl_device_usbdev *tmp = NULL;
>  
> -r = libxl__device_usbdev_list_for_usbctrl(gc, domid,
> +rc = libxl__device_usbdev_list_for_usbctrl(gc, domid,
>atoi(usbctrls[i]),
>, );
> -if (!r || !nd) continue;
> +if (rc || !nd) continue;

If libxl__device_usbdev_list_for_usbctrl fails, shouldn't
libxl_device_usbdev_list fail too ?

Ian.

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

[Xen-devel] [PATCH 1/4] a fix in libxl_device_usbdev_list

[Chunyan Liu]

In testing with libvirt pvusb functionality, found a rc check
error in libxl_device_usbdev_list. Correct it. This function
is not used by xl.

Signed-off-by: Chunyan Liu 
CC: Simon Cao 
CC: George Dunlap 
CC: Ian Jackson 
---
 tools/libxl/libxl_pvusb.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl_pvusb.c b/tools/libxl/libxl_pvusb.c
index 5f92628..04e41b4 100644
--- a/tools/libxl/libxl_pvusb.c
+++ b/tools/libxl/libxl_pvusb.c
@@ -701,13 +701,13 @@ libxl_device_usbdev_list(libxl_ctx *ctx, uint32_t domid, 
int *num)
 usbctrls = libxl__xs_directory(gc, XBT_NULL, path, );
 
 for (i = 0; i < nc; i++) {
-int r, nd = 0;
+int rc, nd = 0;
 libxl_device_usbdev *tmp = NULL;
 
-r = libxl__device_usbdev_list_for_usbctrl(gc, domid,
+rc = libxl__device_usbdev_list_for_usbctrl(gc, domid,
   atoi(usbctrls[i]),
   , );
-if (!r || !nd) continue;
+if (rc || !nd) continue;
 
 usbdevs = libxl__realloc(NOGC, usbdevs,
  sizeof(*usbdevs) * (*num + nd));
-- 
2.1.4


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