Re: [Xen-devel] [PATCH v3 2/5] libxl: add new pvusb backend "qusb" provided by qemu

["Chun Yan Liu" <cyliu@xxxxxxxx>]

>>> On 3/25/2016 at 02:29 PM, in message <56F4DADE.4040200@xxxxxxxx>, Juergen 
>>> Gross
<jgross@xxxxxxxx> wrote: 
> On 25/03/16 03:23, Chun Yan Liu wrote: 
> >  
> >  
> >>>> On 3/23/2016 at 08:24 PM, in message 
> > <1458735847-9448-3-git-send-email-jgross@xxxxxxxx>, Juergen Gross 
> > <jgross@xxxxxxxx> wrote:  
> >> Add a new pvusb backend type "qusb" which is provided by qemu. It can  
> >> be selected either by specifying the type directly in the configuration  
> >> or it is selected automatically by libxl in case there is no "usbback"  
> >> driver loaded.  
> >>   
> >> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>  
> >> ---  
> >>  docs/man/xl.cfg.pod.5                |  11 +++-  
> >>  tools/libxl/libxl_device.c           |   3 +-  
> >>  tools/libxl/libxl_dm.c               |   8 +++  
> >>  tools/libxl/libxl_internal.h         |   1 +  
> >>  tools/libxl/libxl_pvusb.c            | 102  
> +++++++++++++++++++++++++++--------  
> >>  tools/libxl/libxl_types.idl          |   1 +  
> >>  tools/libxl/libxl_types_internal.idl |   1 +  
> >>  7 files changed, 101 insertions(+), 26 deletions(-)  
> >>   
> >> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h  
> >> index fc7bdab..2db8b1b 100644  
> >> --- a/tools/libxl/libxl_internal.h  
> >> +++ b/tools/libxl/libxl_internal.h  
> >> @@ -22,6 +22,21 @@  
> >>    
> >>  #define USBHUB_CLASS_CODE 9  
> >>    
> >> +static int usbback_is_loaded(libxl__gc *gc)  
> >> +{  
> >> +    int r;  
> >> +    struct stat st;  
> >> +  
> >> +    r = lstat(SYSFS_USBBACK_DRIVER, &st);  
> >> +  
> >> +    if (r == 0)  
> >> +        return 1;  
> >> +    if (r < 0 && errno == ENOENT)  
> >> +        return 0;  
> >> +    LOGE(ERROR, "Accessing %s", SYSFS_USBBACK_DRIVER);  
> >> +    return -1;  
> >> +}  
> >> +  
> >>  static int libxl__device_usbctrl_setdefault(libxl__gc *gc, uint32_t 
> >> domid,  
>  
> >>                                              libxl_device_usbctrl 
> >> *usbctrl)  
>  
> >>  {  
> >> @@ -36,7 +51,8 @@ static int libxl__device_usbctrl_setdefault(libxl__gc  
> *gc,   
> >> uint32_t domid,  
> >>    
> >>      if (usbctrl->type == LIBXL_USBCTRL_TYPE_AUTO) {  
> >>          if (domtype == LIBXL_DOMAIN_TYPE_PV) {  
> >> -            usbctrl->type = LIBXL_USBCTRL_TYPE_PV;  
> >> +            usbctrl->type = usbback_is_loaded(gc) ? LIBXL_USBCTRL_TYPE_PV 
> >>  
> > The condition should be (usbback_is_loaded(gc) > 0)? 
> > usbback_is_loaded(gc) < 0 means lstat error, cannot determine if the 
> > usbback driver is loaded. 
>  
> Good point. I think in error case I should rather abort the operation. 
>  
> Thoughts? 

I think it's OK. In another case when we check if a USB device is assigned
or not, we call get_assigned_devices(), it that function fails, we cannot
determine if a USB device is assigned or not, the handling is just abort.
So I think here if cannot determine usbback is loaded or not, we can
also abort and report error directly.

Chunyan


>  
>  
> Juergen 
>  



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