Re: [PATCH RFC V2 3/5] libxl: add pvusb API

[Juergen Gross <jgross@suse.com>]

Hi Chunyan,

I've found another problem while trying to write a qemu based pvUSB
backend.

On 01/19/2015 09:28 AM, Chunyan Liu wrote:
> Add pvusb APIs, including:
>   - attach/detach (create/destroy) virtual usb controller.
>   - attach/detach usb device
>   - list assignable usb devices in host
>   - some other helper functions
>
> Signed-off-by: Chunyan Liu <cyliu@suse.com>
> Signed-off-by: Simon Cao <caobosimon@gmail.com>
> ---

...

> diff --git a/tools/libxl/libxl_usb.c b/tools/libxl/libxl_usb.c
> new file mode 100644
> index 0000000..830a846
> --- /dev/null
> +++ b/tools/libxl/libxl_usb.c

...

> +/* xenstore usb data */
> +static int libxl__device_usb_add_xenstore(libxl__gc *gc, uint32_t domid,
> +                                          libxl_device_usb *usb)
> +{
> +    libxl_ctx *ctx = CTX;
> +    char *be_path;
> +    int rc;
> +    libxl_domain_config d_config;
> +    libxl_device_usb usb_saved;
> +    libxl__domain_userdata_lock *lock = NULL;
> +
> +    libxl_domain_config_init(&d_config);
> +    libxl_device_usb_init(&usb_saved);
> +    libxl_device_usb_copy(CTX, &usb_saved, usb);
> +
> +    be_path = libxl__sprintf(gc, "%s/backend/vusb/%d/%d",
> +                        libxl__xs_get_dompath(gc, 0), domid, usb->ctrl);
> +    if (libxl__wait_for_backend(gc, be_path, "4") < 0) {

Don't do this! That's the reason I had to change my backend driver in
order to support assignment of a usb device via config file. Normally
the backend will witch to state 4 only after the frontend is started.

You can just remove waiting for the backend here. The backend has to
check all ports when it is changing is state to 4 ("connected").

> +        rc = ERROR_FAIL;
> +        goto out;
> +    }
> +
> +    lock = libxl__lock_domain_userdata(gc, domid);
> +    if (!lock) {
> +        rc = ERROR_LOCK_FAIL;
> +        goto out;
> +    }
> +
> +    rc = libxl__get_domain_configuration(gc, domid, &d_config);
> +    if (rc) goto out;
> +
> +    DEVICE_ADD(usb, usbs, domid, &usb_saved, COMPARE_USB, &d_config);
> +
> +    rc = libxl__set_domain_configuration(gc, domid, &d_config);
> +    if (rc) goto out;
> +
> +    be_path = libxl__sprintf(gc, "%s/port/%d", be_path, usb->port);
> +    LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Adding new usb device to xenstore");
> +    if (libxl__xs_write_checked(gc, XBT_NULL, be_path, usb->intf)) {
> +        rc = ERROR_FAIL;
> +        goto out;
> +    }
> +
> +    rc = 0;
> +
> +out:
> +    if (lock) libxl__unlock_domain_userdata(lock);
> +    libxl_device_usb_dispose(&usb_saved);
> +    libxl_domain_config_dispose(&d_config);
> +    return rc;
> +
> +}
> +
> +static int libxl__device_usb_remove_xenstore(libxl__gc *gc, uint32_t domid,
> +                                             libxl_device_usb *usb)
> +{
> +    libxl_ctx *ctx = CTX;
> +    char *be_path;
> +
> +    be_path = libxl__sprintf(gc, "%s/backend/vusb/%d/%d",
> +                        libxl__xs_get_dompath(gc, 0), domid, usb->ctrl);
> +    if (libxl__wait_for_backend(gc, be_path, "4") < 0)

Remove this one, too.

Juergen