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

[Juergen Gross <jgross@suse.com>]

On 17/03/16 17:39, George Dunlap wrote:
> 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?

Hmm, could work. This would require the frontend to register rather
early at boot in order to bind the devices to it before the emulated USB
connector can grab them, right? Or would the emulated USB devices seem
to be unplugged from the emulated adapter and then again plugged in via
the pvusb frontend?

> 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?

No. The frontend won't need to know whether the backend is kernel or
qemu based.


Juergen


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