Re: [PATCH 0/5] RFC: Support UsbDk backend

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

 



On Sun, May 31, 2015 at 12:11:11PM +0300, Dmitry Fleytman wrote:
> Hi Christophe,
> 
> Thanks for the review. See below.
> 
> 
> > On May 28, 2015, at 19:03 PM, Christophe Fergeau <cfergeau@xxxxxxxxxx> wrote:
> > 
> > On Thu, May 28, 2015 at 01:23:59PM +0300, Kirill Moizik wrote:
> > Hey,
> > 
> >> This set of patches add UsbDk backend support to spice-gtk. This series currently cannot be applied since it require next patches series in libusb 
> >> http://marc.info/?l=libusb-devel&m=142532078226137&w=2 .
> >> We are waiting for this patches to be commited to libusb soon and then this series can be applied.
> > 
> > What happens when spice-gtk is built/run against a libusb version
> > without the patches? Is there a way to detect this situation at runtime?
> > Or do we need a configure check on the libusb version which is being
> > used in order to disable usbdk if it's too old?
> 
> Since UsbDk patches introduce no changes to libusb interface I see no way to detect this in runtime.
> The same problem exists for the compile time verification - libusb, even withUsbDk patches, may be
> compiled without UsbDk support, so checks based on libusb version will be not trusted.
> 
> Surely it is a good idea to verify consistency of configuration between spice-gtk and libusb,
> so any idea of how to do it in a robust way is highly welcomed.

I gather that if spice-gtk is trying to use usbdk, but libusb is too old
to have the usbdk patches/was not built with usbdk support, then
spice-gtk will build and work fine except that one of the usbdk-related
call will fail with an error? Or will something else happen?

> 
> > 
> > 
> >> 
> >> Dmitry Fleytman (3):
> >>  build: add build option for non-winusb redirection backends
> >>  usbdk: Add UsbDk hider interface wrapper
> >>  usb-device-manager: Configure UsbDk hiding rules on auto-redirection
> >> 
> >> Pavel Gurvich (2):
> >>  windows: fix device matching for non-WinUSB configurations
> >>  usbdk: make backend selection dynamic
> > 
> > As far as I understand, this series addresses the automatic redirection
> > of devices when they are plugged in. What happens for already plugged
> > devices that the user may want to redirect (eg the built-in webcam in
> > laptops)? On linux you can go to the USB Device Selection menu, and
> > manually redirect a device from there. Is it already working on Windows?
> 
> AFAICT there should be no difference between usbclerk and UsbDk behaviour for auto-redirection.

I was asking what happens for manual redirection? Is this supported by
UsbDk?

> 
> > 
> > Last but not least, I think you mentioned some freeze occurring when
> > redirecting some devices, and said it would be best to fix it in
> > spice-gtk. Can you give more details as to when this freezing occurs,
> > where exactly in the code this occurs (eg spice-gtk method name, and
> > libusb function which freezes) ? Do you need help with fixing that?
> 
> 
> Yes, the issue is libusb_open/libsub_close functions take 2-3 seconds
> with UsbDk because they now reset USB device internally. 
> With current patches spice-gtk freezes for that period of time which
> is a slight problem from UX point of view.
> 
> Since the issue is spice-gtk specific, i.e. it is not relevant for
> non-interactive or CLI applications, we believe it should be fixed in
> spice-gtk.

I disagree with the "it is not relevant for non-interactive or CLI
applications" bit. As soon as you are using a mainloop, you don't want
to be blocking it for several seconds, so this blocking can be an issue
too for non-interactive/CLI applications. However, I don't know if
libusb_open/libusb_close are supposed to be 'cheap' calls, or if we
were supposed to assume they could block for too long even before the
usbdk patches.

> Our current idea is to spawn separate thread(s) for start/stop
> redirection operations to allow spice-gtk process user input and
> behave smoothly.
> 
> Of course we will be glad if you help deal with that issue. What would you suggest?

libusb_open is called from spice_usbredir_channel_open_device, which is
only called from an async context
(spice_usbredir_channel_connect_device_async), so this could be replaced
with a spice_usbredir_channel_open_device_async call which would spawn a
thread to do its work.
libusb_close() (called from usbredirhost_close) might be a bit trickier,
maybe this can be delayed in an idle which would spawn a thread?

Christophe

Attachment: pgpjYyl2wm3Ai.pgp
Description: PGP signature

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]