[virt-tools-list] [PATCH 0/2] add USB redirection support in virt-manager

[Cole Robinson]

On 06/26/2013 11:55 AM, Hans de Goede wrote:
> Hi,
> 
> On 06/26/2013 05:16 PM, Guannan Ren wrote:
>> On 06/26/2013 03:51 PM, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 06/25/2013 04:39 PM, Guannan Ren wrote:
>>>> On 06/25/2013 10:18 PM, Cole Robinson wrote:
>>>>> On 06/25/2013 09:58 AM, Guannan Ren wrote:
>>>>>> On 06/25/2013 01:52 AM, Hans de Goede wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 06/24/2013 12:11 PM, Guannan Ren wrote:
>>>>>>>> This two patch use spicy-client python binding to add USB redirection
>>>>>>>> support in virt-manager if guests use spice viewer.
>>>>>>> Very cool, thanks for working on this. I've done a quick review, mostly
>>>>>>> from the usbredir pov rather then a code pov, and there is one thing
>>>>>>> missing.
>>>>>>>
>>>>>>> Now that virt-manager will have a usb-device selection dialog, it
>>>>>>> should probably also enable usb autoredirection by setting the
>>>>>>> auto-connect property of the UsbDeviceManager to true, this will enable
>>>>>>> the "If the virt-viewer window has the current focus and I insert a USB
>>>>>>> device, it will be automatically redirected to the guest" behavior
>>>>>>> Leonardo is talking about.
>>>>>>>
>>>>>>> I see that Cole is not necessarily a fan of it. Cole, you should give
>>>>>>> this a try, most users love it. In virt-viewer we just always enable
>>>>>>> it without any complaints. Alternatively it could be made an option
>>>>>>> but I would default it to true.
>>>>>>>
>>>>>> I tried it several times, for virt-viewer, auto-usbredir property is set to
>>>>>> TRUE by default, so
>>>>>> it supports auto-rediret by default. it works well for a single guest.
>>>>>> When there are multiple virt-viewer instance opened for multiple guests,
>>>>>> the
>>>>>> auto usbredirection
>>>>>> becomes a little confusing, sometimes it is redirected to guest A,
>>>>>> sometimes
>>>>>> it goes to guest B
>>>>>> between I re-plugin the usb devices.
>>>>>>
>>>>>> It is a really good feature. for virt-manager, multiple viewers more often
>>>>>> stay opened for guests
>>>>>> than virt-viewer, so it possibly cause some confusion for user. So I think
>>>>>> using *disable* to this
>>>>>> feature by default probably is better. we can give an option to enable it
>>>>>> somehow.
>>>>>>
>>>>> Hmm, maybe we could conditionally enable the property only when the spice
>>>>> widget has focus. So it's only turned on for one console at a time.
>>>>>
>>>>> - Cole
>>>>>
>>>>
>>>>      Currently it works so, the spice widget which has focus gets the usb
>>>> device finally.
>>>>      Even so,  it is till confusing unless we document it somewhere,
>>>> otherwise, the user
>>>>      will be curious and try to figure out what is going on about this
>>>> magic behaviours.
>>>
>>> Right, the has focus check Cole is suggestion is already done in the
>>> spice-gtk widget,
>>> auto-redirecting usb-stick (ie) just because a virt-viewer window is
>>> running minimized
>>> somewhere is not exactly user-friendly.
>>>
>>> Guannan, how is the current behavior confusing to you ? Unless you switch
>>> windows right
>>> at the moment you plug in a usb device, the usb-device will always show up
>>> in the guest
>>> you're working with, since that is the one which window has the keyboard
>>> focus.
>>
>> I added tooltips to it for giving more info and enable auto-redir it by
>> default which I think it is clearer.
>> I encountered an two gtk-client library issues.  I just paste it there. I
>> can file bugs if necessary.
>> First:
>> when I call  SpiceClientGLib.Channel.string_to_type('usbredir').
>> error is reported: Could not locate spice_channel_string_to_type:
>> `spice_channel_string_to_type': /lib64/libspice-client-glib-2.0.so.8:
>> undefined symbol: spice_channel_string_to_type
>> It seems the symbol is local rather than GLOBAL
>> # readelf -s /lib64/libspice-client-glib-2.0.so |grep spice_channel_string
>> 3275: 000000000001e910   159 FUNC    LOCAL  DEFAULT   12
>> spice_channel_string_to_t
>>
> 
> spice-gtk bug, I just send a patch to the spice-devel list for this.
> 
>> Second issue:
>> In virt-viewer, virt_viewer_session_spice_has_usb returns False always, in
>> my testing, it seems that the usbredir channel is created later than calling
>> main channel. So the Select USB devices to redirect option in virt-viewer is
>> disable, but the auto-redir is enabled.
> 
> Hmm, are you sure, I'm not seeing this. Does your test vm have usb-channels ?
> 
> Looking at the virt-viewer code, I see that on paper this can happen, I'll write
> a virt-viewer patch for this tomorrow, which will hopefully be easy to port to
> python
> for virt-manager.
> 

And as mentioned yesterday, there is

SpiceClientGtk.UsbDeviceWidget.new(self.session, None)
TypeError: Argument 1 does not allow None as a value

So I'm guessing there's an (allow-none) annotation missing.

- Cole