[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



Hi,

On 06/26/2013 06:12 PM, Marc-André Lureau wrote:


----- Mensaje original -----
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.

We never explicitely made it public (it's not documented either), but I guess it's time to do it, now that we have the counterpart public.

Erm, on my system it is declared in:
/usr/include/spice-client-glib-2.0/spice-channel.h

How more public do you want to have it ?  If it was not intended as public,
it should not have a prototype in a public header.

Regards,

Hans


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]