Re: [Xen-devel] [PATCH V4 3/7] libxl: add pvusb API

[George Dunlap <george.dunlap@xxxxxxxxxxxxx>]

On 06/16/2015 03:20 PM, Juergen Gross wrote:
> On 06/16/2015 04:06 PM, George Dunlap wrote:
>> On 06/16/2015 02:49 PM, Juergen Gross wrote:
>>> On 06/16/2015 03:29 PM, George Dunlap wrote:
>>>> On 06/16/2015 02:23 PM, Juergen Gross wrote:
>>>>> Hmm, I'd rather have it all in one place. Putting it in qemu would
>>>>> enable us to handle hotplug as well. A USB device assigned via it's
>>>>> serial to a domU could be automatically passed through by qemu when
>>>>> showing up. This isn't possible today, but we wouldn't have to change
>>>>> libxl again for supporting it, only qemu would have to be adapted.
>>>>
>>>> Look, we're talking here about the libxl interface.  Ian thinks that
>>>> *at
>>>> the libxl layer* we need to be able to specify all these things.  It
>>>> doesn't matter at this point whether qemu can also do it or not.
>>>
>>> When I'm adding new functionality (and this is the case here) I always
>>> try to add it at the level where it should be. qemu already is capable
>>> of finding a USB device by various means and can be extended easily to
>>> support our needs. And please remember, for writing the pvusb backend
>>> I'm already doing changes to qemu.
>>>
>>> So why should we add the same functionality to libxl by reading sysfs
>>> instead of letting it do qemu via libusb? And what about BSD? Letting
>>> qemu find the device would avoid two variants in libxl.
>>
>> If the libxl interface only has "bus" and "port", then it doesn't matter
>> what qemu can do -- the caller has no way of asking qemu to watch for
>> vid:did:serial.
>>
>> If libxl has vid:did:serial in the interface, then it's only an
>> implementation detail whether we pass that into qemu or whether we pass
>> some other way of uniquely indentifying a particular device.  And given
>> that no *current* versions of qemu support vid:did:serial, the most
>> sensible way to implement this would be to have libxl do the conversion
>> and send over bus:addr -- that way when you update libxl you get that
>> functionality regardless of whether qemu has it.
>>
>> Unless you're suggesting that the libxl interface should be a string
>> that we just pass verbatim to qemu.  If that's what you mean, it's a
>> complete non-starter, and I'm sure Ian will agree with me.  There's no
>> way that we can provide any interface consistency or any documentation
>> of what this string does if it boils down to "This does whatever the
>> version of qemu you happen to be running does".
> 
> No, I didn't expect libxl to just pass an arbitrary string to qemu.
> My point was to avoid the sysfs accesses in libxl in order to support
> BSD as well and to reduce the complexity.
> 
>>>> In the future, if we actually implement the "automatically grab this
>>>> device when it shows up" functionality, then yes, having it in qemu
>>>> rather than having a libxl daemon sit around and watch for those
>>>> things,
>>>> might be handy.  But we can cross that bridge when we come to it.
>>>
>>> I would agree if the efforts in libxl would be much smaller than in
>>> qemu. But if the efforts are comparable I'd rather do it in the
>>> component which will make such an enhancement easier.
>>
>> They're both small, so this is really bikeshedding at the moment.  The
>> most important question is the interface: do we include vid:did:serial
>> in the libxl interface.  Since you want qemu to do that, I take it that
>> your answer to that is "yes".
> 
> Correct.
> 
>> When we have patches submitted, we can discuss whether libxl should only
>> support the (as-yet unreleased) versions of qemu that do the search
>> themselves, or whether they should support all versions of qemu-xen.
>> (The answer seems pretty obvious to me...)
> 
> May be I made one wrong assumption: I thought adding USB-passthrough for
> HVM domains would require qemu changes as well. If this is not the case
> I can understand your reasoning.

Oh, right.  No, getting HVM hotplug working over QMP was pretty
straightforward; probably only a week's work.  It's been the interface
that has been the real difficult bit.

There are some things that would be nicer to implement in qemu.  Right
now there's no way to *query* what devices have been plugged in; my
patch series had to store information in xenstore.

At some point in the future, it might make sense for libxl to mainly
just ferry the options back and forth to qemu; even then I'm not sure.
In any case it's an implementation detail that can be changed at any
time.  It's the interface we have to be really careful about.

 -George


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