Re: [PATCH v2 2/3] libxl: add new pvusb backend "qusb" provided by qemu

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Tue, Mar 22, 2016 at 08:29:22AM +0100, Juergen Gross 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@suse.com>
> ---
>  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/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index ec739cc..a4cc1b3 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -737,8 +737,15 @@ Possible B<KEY>s are:
>  
>  =item B<type=TYPE>
>  
> -Specifies the usb controller type.  Currently only 'pv' and 'auto'
> -are supported.
> +Specifies the usb controller type.
> +
> +"pv" denotes a kernel based pvusb backend.
> +
> +"qusb" specifies a qemu base backend for pvusb.
> +
> +"auto" (the default) determines whether a kernel based backend is installed.
> +If this is the case, "pv" is selected, "qusb" will be selected if no kernel
> +backend is currently available.
>  
>  =item B<version=VERSION>
>  
> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> index 4ced9b6..eba3087 100644
> --- a/tools/libxl/libxl_device.c
> +++ b/tools/libxl/libxl_device.c
> @@ -680,7 +680,8 @@ void libxl__devices_destroy(libxl__egc *egc, libxl__devices_remove_state *drs)
>                  aodev->action = LIBXL__DEVICE_ACTION_REMOVE;
>                  aodev->dev = dev;
>                  aodev->force = drs->force;
> -                if (dev->backend_kind == LIBXL__DEVICE_KIND_VUSB)
> +                if (dev->backend_kind == LIBXL__DEVICE_KIND_VUSB ||
> +                    dev->backend_kind == LIBXL__DEVICE_KIND_QUSB)
>                      libxl__initiate_device_usbctrl_remove(egc, aodev);
>                  else
>                      libxl__initiate_device_generic_remove(egc, aodev);
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 897f3f9..361e584 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -2138,6 +2138,14 @@ int libxl__need_xenpv_qemu(libxl__gc *gc, libxl_domain_config *d_config)
>          }
>      }
>  
> +    for (i = 0; i < d_config->num_usbctrls; i++) {
> +       if (d_config->usbctrls[i].type == LIBXL_USBCTRL_TYPE_QUSB &&
> +           d_config->usbctrls[i].backend_domid == domid) {
> +            ret = 1;
> +            goto out;
> +        }
> +    }
> +
>  out:
>      return ret;
>  }
> 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
> @@ -487,6 +487,7 @@ typedef struct {
>  #define QEMU_BACKEND(dev) (\
>      (dev)->backend_kind == LIBXL__DEVICE_KIND_QDISK || \
>      (dev)->backend_kind == LIBXL__DEVICE_KIND_VFB || \
> +    (dev)->backend_kind == LIBXL__DEVICE_KIND_QUSB || \
>      (dev)->backend_kind == LIBXL__DEVICE_KIND_VKBD)
>  
>  #define XC_PCI_BDF             "0x%x, 0x%x, 0x%x, 0x%x"
> diff --git a/tools/libxl/libxl_pvusb.c b/tools/libxl/libxl_pvusb.c
> index 5f92628..7200ead 100644
> --- a/tools/libxl/libxl_pvusb.c
> +++ b/tools/libxl/libxl_pvusb.c
> @@ -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;

I believe the CODING STYLE in libxl asks for you to use { } for these
ones.

> +    LOGE(ERROR, "Accessing %s", SYSFS_USBBACK_DRIVER);

Why is this an error?

> +    return -1;
> +}
> +

Otherwise looks OK to me.

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