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

[George Dunlap <George.Dunlap@eu.citrix.com>]

On Thu, Mar 10, 2016 at 3:00 PM, Juergen Gross <jgross@suse.com> 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>

It would be awesome if we could get a patch like this in for 4.7.

However...

> ---
>  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 1dde66b..06eeb42 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.

I'm going back and forth a bit about whether having a "qusb" option
here makes the most sense, or whether it makes more sense to have a
separate option, like "backendtype" (as we have for disks).

The reason I suggested having "type" here at all is that libxl cannot
know ahead of time whether the guest has pvusbfront available.  It
*can*, however, tell whether pvusbback is available (as your code
demonstrates).

On the other hand -- given that qemu will shortly  have pvusbback, I'm
wondering if it's possible to make pvusbfront do something similar to
what the blkfront drivers do -- write a magic port that tell qemu,
"Actually, I have pvusb drivers; unplug the emulated controller."

From a UI point of view that would be the best, right?  Just like
disks or network cards -- you can simply say you want a controller,
and you automatically get the most appropriate kind based on what the
kernel you run actually has available.

I think you've been working on the pvusbfront drivers for Linux, as
well as the qusb backend in qemu -- do you think that's workable?

If so, I think adding 'qusb' to this field is probably the right thing to do.

One technical question...

> @@ -1559,7 +1616,6 @@ int libxl_ctrlport_to_device_usbdev(libxl_ctx *ctx,
>      dompath = libxl__xs_get_dompath(gc, domid);
>
>      fe_path = GCSPRINTF("%s/device/vusb/%d", dompath, ctrl);

Did you miss a vusb -> %s here?

 -George

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