[Spice-devel] [spice-gtk 2/5] usb-redir: unify device hotplug/unplug for Windows and Linux
Frediano Ziglio
fziglio at redhat.com
Mon Jul 22 09:53:11 UTC 2019
>
> On Fri, Jul 19, 2019 at 1:14 PM Victor Toso <victortoso at redhat.com> wrote:
> >
> > Hi,
> >
> > On Sun, Jul 14, 2019 at 05:07:38PM +0300, Yuri Benditovich wrote:
> > > Remove Windows-specific part of hotplug detection from
> > > usb-device-manager and win-usb-dev and place it into
> > > USB backend module. Connect the hotplug/unplug detection
> > > in Windows to the same callbacks already used in Linux.
> > > This removes significant part of Windows-specific code
> > > and simpifies things.
> > > Note regarding 'redirecting' property of usb-device-manager:
> > > - Previously usb-device-manager under Windows maintained
> > > 'redirection' property in win-usb-dev
> > > - Now it maintains its own property in both Windows and
> > > Linux (for GUI and single redirect at a time)
> > > - The USB backend maintains its own 'redirecting' field
> > > in Windows to prevent update of device list when
> > > libusb opens the device (device redirection causes
> > > temporary removal of target device)
> >
> > Thanks for the description. Only minor things, logic seems fine,
> > did quick tests only. Would be nice to reduce diff from things
> > that can be smaller patches whenever needed, function renames or
> > being moved or variables being dropped for instance so it would
> > get easier to focus on the actual interesting part of proposal.
> > This is always mentioned and that's one of the reasons that it
> > takes time to review.
> >
> > > Signed-off-by: Yuri Benditovich <yuri.benditovich at daynix.com>
> > > ---
> > > meson.build | 2 +-
> > > src/meson.build | 4 +-
> > > src/usb-backend.c | 360 ++++++++++++++++++++++----------
> > > src/usb-backend.h | 8 -
> > > src/usb-device-manager.c | 61 +-----
> > > src/win-usb-dev.c | 436 ---------------------------------------
> > > src/win-usb-dev.h | 84 --------
> > > 7 files changed, 255 insertions(+), 700 deletions(-)
> >
> > Quite happy with the code reduction! That's one thing I was
> > expecting from introducing usb-backend.[ch]
> >
> > > delete mode 100644 src/win-usb-dev.c
> > > delete mode 100644 src/win-usb-dev.h
> > >
> > > diff --git a/meson.build b/meson.build
> > > index 54160f9..1f6ea6d 100644
> > > --- a/meson.build
> > > +++ b/meson.build
> > > @@ -117,7 +117,7 @@ endforeach
> > >
> > > deps = []
> > > if host_machine.system() == 'windows'
> > > - deps += ['libws2_32', 'libgdi32']
> > > + deps += ['libws2_32', 'libgdi32', 'libcomctl32']
> > > endif
> > >
> > > foreach dep : deps
> > > diff --git a/src/meson.build b/src/meson.build
> > > index a51d0a9..adcfaec 100644
> > > --- a/src/meson.build
> > > +++ b/src/meson.build
> > > @@ -158,9 +158,7 @@ endif
> > >
> > > if spice_gtk_has_usbredir and host_machine.system() == 'windows'
> > > spice_client_glib_sources += ['usbdk_api.c',
> > > - 'usbdk_api.h',
> > > - 'win-usb-dev.c',
> > > - 'win-usb-dev.h']
> > > + 'usbdk_api.h']
> > > endif
> > >
> > > #
> > > diff --git a/src/usb-backend.c b/src/usb-backend.c
> > > index a2c502d..9964c4f 100644
> > > --- a/src/usb-backend.c
> > > +++ b/src/usb-backend.c
> > > @@ -32,6 +32,9 @@
> > > #include <libusb.h>
> > > #include <string.h>
> > > #include <fcntl.h>
> > > +#ifdef G_OS_WIN32
> > > +#include <commctrl.h>
> > > +#endif
> > > #include "usbredirhost.h"
> > > #include "usbredirparser.h"
> > > #include "spice-util.h"
> > > @@ -55,6 +58,11 @@ struct _SpiceUsbBackend
> > > usb_hot_plug_callback hotplug_callback;
> > > void *hotplug_user_data;
> > > libusb_hotplug_callback_handle hotplug_handle;
> > > +#ifdef G_OS_WIN32
> > > + HANDLE hWnd;
> > > + libusb_device **libusb_device_list;
> > > + gint redirecting;
> > > +#endif
> > > };
> > >
> > > struct _SpiceUsbBackendChannel
> > > @@ -66,9 +74,229 @@ struct _SpiceUsbBackendChannel
> > > int rules_count;
> > > SpiceUsbBackendDevice *attached;
> > > SpiceUsbredirChannel *user_data;
> > > + SpiceUsbBackend *backend;
> > > GError **error;
> > > };
> > >
> >
> > Some code below is just being moved, makes the diff a little
> > bigger and review a bit slower. If I comment on code that has
> > been only moved and you consider the comments relevant, please
> > address them in different patches.
> >
> > > +static gboolean fill_usb_info(SpiceUsbBackendDevice *bdev)
> >
> > In several places we have different variable names for giving
> > structures, it would be good to have it somewhat similar so that
> > reading the code you don't have to think if dev is libusb_device
> > or SpiceUsbBackendDevice, etc.
> >
> > > +{
> > > + UsbDeviceInformation *info = &bdev->device_info;
> > > +
> > > + struct libusb_device_descriptor desc;
> > > + libusb_device *libdev = bdev->libusb_device;
> > > + libusb_get_device_descriptor(libdev, &desc);
> > > + info->bus = libusb_get_bus_number(libdev);
> > > + info->address = libusb_get_device_address(libdev);
> > > + if (info->address == 0xff || /* root hub (HCD) */
> > > + info->address <= 1 || /* root hub or bad address */
> > > + (desc.bDeviceClass == LIBUSB_CLASS_HUB) /*hub*/) {
> > > + return FALSE;
> > > + }
> > > +
> > > + info->vid = desc.idVendor;
> > > + info->pid = desc.idProduct;
> > > + info->class = desc.bDeviceClass;
> > > + info->subclass = desc.bDeviceSubClass;
> > > + info->protocol = desc.bDeviceProtocol;
> >
> > Filling UsbDeviceInformation from libusb_device can be made into
> > a utility function as this happens here and is_same_libusb_dev(),
> > but also because it can remove a extra parameter on the later.
> > Taking a shot at the name, get_device_info_from_libusb_device()!
> >
> > > +
> > > + return TRUE;
> > > +}
> > > +
> > > +static SpiceUsbBackendDevice *allocate_backend_device(libusb_device
> > > *libdev)
> > > +{
> > > + SpiceUsbBackendDevice *dev = g_new0(SpiceUsbBackendDevice, 1);
> > > + dev->ref_count = 1;
> > > + dev->libusb_device = libdev;
> > > + if (!fill_usb_info(dev)) {
> > > + g_clear_pointer(&dev, g_free);
> > > + }
> > > + return dev;
> > > +}
> > > +
> > > +static int LIBUSB_CALL hotplug_callback(libusb_context *ctx,
> > > + libusb_device *device,
> > > + libusb_hotplug_event event,
> >
> > Perhaps adding lib prefix to libusb related variables, as you did
> > in the other functions above, would be enough.
> >
> > > + void *user_data)
> > > +{
> > > + SpiceUsbBackend *be = (SpiceUsbBackend *)user_data;
> >
> > No need to cast void*
> >
> > > + if (be->hotplug_callback) {
> >
> > Should warn if fail, no? This callback should only be called
> > after spice_usb_backend_register_hotplug() - If you agree,
> > g_return_val_if_fail() might fit
> >
> > > + SpiceUsbBackendDevice *dev;
> > > + gboolean val = event == LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED;
> >
> > val -> has_arrived
> >
> > > + dev = allocate_backend_device(device);
> > > + if (dev) {
> > > + SPICE_DEBUG("created dev %p, usblib dev %p", dev, device);
> > > + libusb_ref_device(device);
> > > + be->hotplug_callback(be->hotplug_user_data, dev, val);
> > > + spice_usb_backend_device_unref(dev);
> > > + }
> > > + }
> > > + return 0;
> > > +}
> > > +
> > > +#ifdef G_OS_WIN32
> > > +/* Windows-specific: get notification on device change */
> > > +
> > > +static gboolean is_same_libusb_dev(libusb_device *dev1,
> > > + UsbDeviceInformation *info1,
> > > + libusb_device *dev2)
> >
> > This function should just be comparing both libusb_devices, info1
> > argument is for debug on the caller so can be removed...
> >
> > > +{
> > > + struct libusb_device_descriptor desc;
> > > + info1->bus = libusb_get_bus_number(dev1);
> > > + info1->address = libusb_get_device_address(dev1);
> > > + libusb_get_device_descriptor(dev1, &desc);
> > > + info1->vid = desc.idVendor;
> > > + info1->pid = desc.idProduct;
> > > + if (dev2 == NULL) {
> > > + return FALSE;
> > > + }
> >
> > ... I think the first check is if either libdev1 or libdev2 are NULL
> > and return FALSE, otherwise you can then compare after
> >
It looks like this test is just because the caller mistake,
maybe the caller should never call this function passing NULLs ?
> > info1 = get_device_info_from_libusb_device(dev1);
> > info2 = get_device_info_from_libusb_device(dev2);
> >
> > > + libusb_get_device_descriptor(dev2, &desc);
> > > + return info1->bus == libusb_get_bus_number(dev2) &&
> > > + info1->address == libusb_get_device_address(dev2) &&
> > > + info1->vid == desc.idVendor &&
> > > + info1->pid == desc.idProduct;
> > > +}
> > > +
> > > +static int compare_dev_list(SpiceUsbBackend *be,
> > > + libusb_device **l1,
> > > + libusb_device **l2,
> >
> > Hmm const them? Or..
> >
l1 and l2 seems a bit short, maybe list1 and list2 ?
I think best type would be "const libusb_device* const *".
> > > + gboolean value)
> >
> > value -> has_arrived
> >
> > .. This function tries to find elements of list1 in list2 and calls
> > hotplug_callback() for every element not found. This is a bit
> > more than the name of the function suggests.
> >
> >
> > I would rather have it as
> > GList* dev_list_difference(libusb_device **libdevp1,
> > libusb_device **libdevp2);
> >
> > and the caller would call hotplug_callback() and
> > g_list_free() it.
> >
>
> I never allocate memory when there is easy way to deal without it
>
I agree a bit with both. The split is just a not so useful complication.
The name is confusing however. I would expect such a function to not
change anything and return is the lists are the same, instead the
difference is computed and for each device hotplug_callback is called.
The number of added/removed devices are returned.
Maybe a name could be "hotplug_added_devices" ?
> > > +{
> > > + int res = 0;
> > > + while (*l1) {
Maybe
for (; *l1; ++l1) {
and remove l1++ below?
> > > + gboolean found = 0;
use FALSE? I would persoanally use bool.
> > > + uint32_t n = 0;
> > > + UsbDeviceInformation info1;
> > > + while (!found) {
Maybe
for (libusb_device **l = l2; *l; ++l) {
and remove n above and n++ below ??
> > > + found = is_same_libusb_dev(*l1, &info1, l2[n]);
> > > + if (l2[n] == NULL) {
> > > + break;
> > > + }
Maybe this check should be done before the is_same_libusb_dev call
so is_same_libusb_dev won't have to deal with NULL pointers?
I understand that potentially is_same_libusb_dev is not called
so info1 is not initialized but looks like is_same_libusb_dev is
doing 2 things instead of one as Victor mentioned. And more bad
is doing 2 things just for debug sake.
IMHO all that additional computation in some part of code to make
other code happy make stuff confusing, it's fine now but the code
is less maintainable, when in the future we would maintain or when
new people came the code won't make much sense.
> > > + n++;
> > > + }
> > > + if (!found) {
> > > + SPICE_DEBUG("%s %04X:%04X at %d:%d", value ? "adding" :
> > > "removing",
> > > + info1.vid, info1.pid, info1.bus, info1.address);
info1 is used only here for debugging but compute for all l2 items.
> > > + hotplug_callback(NULL, *l1,
> > > + value ? LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED :
> > > LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT,
> > > + be);
> > > + res++;
> > > + }
> > > + l1++;
> > > + }
> > > + return res;
> > > +}
> > > +
> > > +static LRESULT subclass_proc(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM
> > > lParam,
> > > + UINT_PTR uIdSubclass, DWORD_PTR dwRefData)
> > > +{
> > > + SpiceUsbBackend *be = (SpiceUsbBackend *)dwRefData;
> > > + if (uMsg == WM_DEVICECHANGE && !be->redirecting) {
> > > + libusb_device **new_list = NULL;
> > > + libusb_get_device_list(be->libusb_context, &new_list);
> > > + if (new_list) {
> > > + int res = compare_dev_list(be, be->libusb_device_list,
> > > new_list, FALSE);
> > > + res += compare_dev_list(be, new_list,
> > > be->libusb_device_list, TRUE);
> > > + if (res) {
> > > + /* there were changes in device tree */
> > > + libusb_free_device_list(be->libusb_device_list, TRUE);
> > > + be->libusb_device_list = new_list;
> > > + } else {
> > > + libusb_free_device_list(new_list, TRUE);
> > > + }
> > > + } else {
> > > + g_warn_if_reached();
> > > + }
> > > + }
> > > + return DefSubclassProc(hWnd, uMsg, wParam, lParam);
> > > +}
> > > +
> > > +static void disable_hotplug_support(SpiceUsbBackend *be)
> > > +{
> > > + if (be->hWnd) {
> > > + DestroyWindow(be->hWnd);
> > > + be->hWnd = NULL;
> > > + }
> > > + if (be->libusb_device_list) {
> > > + libusb_free_device_list(be->libusb_device_list, TRUE);
> > > + be->libusb_device_list = NULL;
> > > + }
> > > +}
> > > +
> > > +static int enable_hotplug_support(SpiceUsbBackend *be, const char
> > > **error_on_enable)
> > > +{
> > > + long win_err;
> > > + libusb_device **dev_list = NULL;
> > > + libusb_device *dummy = NULL;
> > > +
> > > + libusb_get_device_list(be->libusb_context, &dev_list);
> > > + if (!dev_list) {
> > > + *error_on_enable = "Getting device list";
> > > + goto error;
> > > + }
> > > + be->hWnd = CreateWindow("Static", NULL, 0, 0, 0, 0, 0, NULL, NULL,
> > > NULL, NULL);
> >
> > Not sure if can be important but it was G_UDEV_CLIENT_WINCLASS_NAME before,
> >
> > #define G_UDEV_CLIENT_WINCLASS_NAME TEXT("G_UDEV_CLIENT")
> >
> > and I notice that you dropped RegisterClass call, not needed
> > anymore?
> >
> >
>
> 'Static' is standard class, starting from XP there is easy way to work
> without registering own class.
>
I think static was present in Windows 3.1 too :-)
Make sense to me, but a small comment won't hurt, I too stopped here and
thought "why static" ?
> > > + if (!be->hWnd) {
> > > + *error_on_enable = "CreateWindow";
> > > + goto error;
> > > + }
> > > + if (!SetWindowSubclass(be->hWnd, subclass_proc, 0, (DWORD_PTR)be)) {
> > > + *error_on_enable = "SetWindowSubclass";
> > > + goto error;
> > > + }
> > > + be->hotplug_handle = 1;
> > > + be->libusb_device_list = dev_list;
> > > +
> > > + compare_dev_list(be, be->libusb_device_list, &dummy, TRUE);
> >
> > What's the point of this function call? to call
> > hotplug_callback() on dev_list? You might as well just do it here
> > with a comment
> >
> The point is that this takes one line of code, just call
> hotplug_callback takes more.
> Procedures exist for reuse, I think
>
A
hotplug_added_devices(be, be->libusb_device_list, &empty_device_list, TRUE);
would be more readable.
> > > +
> > > + return LIBUSB_SUCCESS;
> > > +error:
> > > + win_err = GetLastError();
> > > + if (!win_err) {
> > > + win_err = -1;
> > > + }
> > > + g_warning("%s failed: %ld", *error_on_enable, win_err);
> > > + if (dev_list) {
> > > + libusb_free_device_list(dev_list, TRUE);
> > > + }
> > > + return win_err;
> > > +}
> > > +
> > > +static void set_redirecting(SpiceUsbBackend *be, gboolean on)
> > > +{
> > > + gboolean no_redir;
> > > + if (on) {
> > > + g_atomic_int_inc(&be->redirecting);
> > > + } else {
> > > + no_redir = g_atomic_int_dec_and_test(&be->redirecting);
> >
> > You can move declaration of no_redir here
> >
> > > + if (no_redir && be->hWnd) {
> > > + PostMessage(be->hWnd, WM_DEVICECHANGE, 0, 0);
> > > + }
> > > + }
> > > +}
> > > +
> > > +#else
> > > +/* Linux-specific: use hot callback from libusb */
> > > +
> > > +static void disable_hotplug_support(SpiceUsbBackend *be)
> > > +{
> > > + libusb_hotplug_deregister_callback(be->libusb_context,
> > > be->hotplug_handle);
> > > +}
> > > +
> > > +static int enable_hotplug_support(SpiceUsbBackend *be, const char
> > > **error_on_enable)
> > > +{
> > > + int rc = 0;
> > > + rc = libusb_hotplug_register_callback(be->libusb_context,
> > > + LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED |
> > > LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT,
> > > + LIBUSB_HOTPLUG_ENUMERATE, LIBUSB_HOTPLUG_MATCH_ANY,
> > > + LIBUSB_HOTPLUG_MATCH_ANY, LIBUSB_HOTPLUG_MATCH_ANY,
> > > + hotplug_callback, be, &be->hotplug_handle);
> > > + *error_on_enable = libusb_strerror(rc);
> > > + return rc;
> > > +}
> > > +
> > > +#define set_redirecting(...)
> >
> > ok
> >
Usually I prefer static inline empty function for type safety but
fine with the macro. The empty expansion could be a problem, some
compilers for instance give a warning if they find 2 semicolon in
a row.
> > > +
> > > +#endif
> > > +
> > > /* lock functions for usbredirhost and usbredirparser */
> > > static void *usbredir_alloc_lock(void) {
> > > GMutex *mutex;
> > > @@ -131,41 +359,6 @@ gboolean
> > > spice_usb_backend_device_isoch(SpiceUsbBackendDevice *dev)
> > > return isoc_found;
> > > }
> > >
> > > -static gboolean fill_usb_info(SpiceUsbBackendDevice *bdev)
> > > -{
> > > - UsbDeviceInformation *info = &bdev->device_info;
> > > -
> > > - struct libusb_device_descriptor desc;
> > > - libusb_device *libdev = bdev->libusb_device;
> > > - libusb_get_device_descriptor(libdev, &desc);
> > > - info->bus = libusb_get_bus_number(libdev);
> > > - info->address = libusb_get_device_address(libdev);
> > > - if (info->address == 0xff || /* root hub (HCD) */
> > > - info->address <= 1 || /* root hub or bad address */
> > > - (desc.bDeviceClass == LIBUSB_CLASS_HUB) /*hub*/) {
> > > - return FALSE;
> > > - }
> > > -
> > > - info->vid = desc.idVendor;
> > > - info->pid = desc.idProduct;
> > > - info->class = desc.bDeviceClass;
> > > - info->subclass = desc.bDeviceSubClass;
> > > - info->protocol = desc.bDeviceProtocol;
> > > -
> > > - return TRUE;
> > > -}
> > > -
> > > -static SpiceUsbBackendDevice *allocate_backend_device(libusb_device
> > > *libdev)
> > > -{
> > > - SpiceUsbBackendDevice *dev = g_new0(SpiceUsbBackendDevice, 1);
> > > - dev->ref_count = 1;
> > > - dev->libusb_device = libdev;
> > > - if (!fill_usb_info(dev)) {
> > > - g_clear_pointer(&dev, g_free);
> > > - }
> > > - return dev;
> > > -}
> > > -
> > > static gboolean is_channel_ready(SpiceUsbredirChannel *channel)
> > > {
> > > return spice_channel_get_state(SPICE_CHANNEL(channel)) ==
> > > SPICE_CHANNEL_STATE_READY;
> > > @@ -237,31 +430,11 @@ void
> > > spice_usb_backend_interrupt_event_handler(SpiceUsbBackend *be)
> > > }
> > > }
> > >
> > > -static int LIBUSB_CALL hotplug_callback(libusb_context *ctx,
> > > - libusb_device *device,
> > > - libusb_hotplug_event event,
> > > - void *user_data)
> > > -{
> > > - SpiceUsbBackend *be = (SpiceUsbBackend *)user_data;
> > > - if (be->hotplug_callback) {
> > > - SpiceUsbBackendDevice *dev;
> > > - gboolean val = event == LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED;
> > > - dev = allocate_backend_device(device);
> > > - if (dev) {
> > > - SPICE_DEBUG("created dev %p, usblib dev %p", dev, device);
> > > - libusb_ref_device(device);
> > > - be->hotplug_callback(be->hotplug_user_data, dev, val);
> > > - spice_usb_backend_device_unref(dev);
> > > - }
> > > - }
> > > - return 0;
> > > -}
> > > -
> > > void spice_usb_backend_deregister_hotplug(SpiceUsbBackend *be)
> > > {
> > > g_return_if_fail(be != NULL);
> > > if (be->hotplug_handle) {
> > > - libusb_hotplug_deregister_callback(be->libusb_context,
> > > be->hotplug_handle);
> > > + disable_hotplug_support(be);
> > > be->hotplug_handle = 0;
> > > }
> > > be->hotplug_callback = NULL;
> > > @@ -272,17 +445,15 @@ gboolean
> > > spice_usb_backend_register_hotplug(SpiceUsbBackend *be,
> > > usb_hot_plug_callback proc)
> > > {
> > > int rc;
> > > + const char *desc;
> > > g_return_val_if_fail(be != NULL, FALSE);
> > >
> > > be->hotplug_callback = proc;
> > > be->hotplug_user_data = user_data;
> > > - rc = libusb_hotplug_register_callback(be->libusb_context,
> > > - LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED |
> > > LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT,
> > > - LIBUSB_HOTPLUG_ENUMERATE, LIBUSB_HOTPLUG_MATCH_ANY,
> > > - LIBUSB_HOTPLUG_MATCH_ANY, LIBUSB_HOTPLUG_MATCH_ANY,
> > > - hotplug_callback, be, &be->hotplug_handle);
> > > +
> > > + rc = enable_hotplug_support(be, &desc);
> > > +
> > > if (rc != LIBUSB_SUCCESS) {
> > > - const char *desc = libusb_strerror(rc);
> > > g_warning("Error initializing USB hotplug support: %s [%i]",
> > > desc, rc);
> > > be->hotplug_callback = NULL;
> > > return FALSE;
> > > @@ -301,45 +472,6 @@ void spice_usb_backend_delete(SpiceUsbBackend *be)
> > > SPICE_DEBUG("%s <<", __FUNCTION__);
> > > }
> > >
> > > -SpiceUsbBackendDevice
> > > **spice_usb_backend_get_device_list(SpiceUsbBackend *be)
> > > -{
> > > - LOUD_DEBUG("%s >>", __FUNCTION__);
> > > - libusb_device **devlist = NULL, **dev;
> > > - SpiceUsbBackendDevice *d, **list;
> > > -
> > > - int n = 0, index;
> > > -
> > > - if (be && be->libusb_context) {
> > > - libusb_get_device_list(be->libusb_context, &devlist);
> > > - }
> > > -
> > > - /* add all the libusb device that not present in our list */
> > > - for (dev = devlist; dev && *dev; dev++) {
> > > - n++;
> > > - }
> > > -
> > > - list = g_new0(SpiceUsbBackendDevice*, n + 1);
> > > -
> > > - index = 0;
> > > -
> > > - for (dev = devlist; dev && *dev; dev++) {
> > > - d = allocate_backend_device(*dev);
> > > - if (!d) {
> > > - libusb_unref_device(*dev);
> > > - } else {
> > > - SPICE_DEBUG("created dev %p, usblib dev %p", d, *dev);
> > > - list[index++] = d;
> > > - }
> > > - }
> > > -
> > > - if (devlist) {
> > > - libusb_free_device_list(devlist, 0);
> > > - }
> > > -
> > > - LOUD_DEBUG("%s <<", __FUNCTION__);
> > > - return list;
> > > -}
> > > -
> > > const UsbDeviceInformation*
> > > spice_usb_backend_device_get_info(SpiceUsbBackendDevice *dev)
> > > {
> > > return &dev->device_info;
> > > @@ -350,18 +482,6 @@ gconstpointer
> > > spice_usb_backend_device_get_libdev(SpiceUsbBackendDevice *dev)
> > > return dev->libusb_device;
> > > }
> > >
> > > -void spice_usb_backend_free_device_list(SpiceUsbBackendDevice **devlist)
> > > -{
> > > - LOUD_DEBUG("%s >>", __FUNCTION__);
> > > - SpiceUsbBackendDevice **dev;
> > > - for (dev = devlist; *dev; dev++) {
> > > - SpiceUsbBackendDevice *d = *dev;
> > > - spice_usb_backend_device_unref(d);
> > > - }
> > > - g_free(devlist);
> > > - LOUD_DEBUG("%s <<", __FUNCTION__);
> > > -}
> > > -
> > > SpiceUsbBackendDevice
> > > *spice_usb_backend_device_ref(SpiceUsbBackendDevice *dev)
> > > {
> > > LOUD_DEBUG("%s >> %p", __FUNCTION__, dev);
> > > @@ -529,12 +649,23 @@ gboolean
> > > spice_usb_backend_channel_attach(SpiceUsbBackendChannel *ch,
> > > SpiceUsbBackendDevice *dev,
> > > GError **error)
> > > {
> > > + int rc;
> > > SPICE_DEBUG("%s >> ch %p, dev %p (was attached %p)", __FUNCTION__,
> > > ch, dev, ch->attached);
> > >
> > > g_return_val_if_fail(dev != NULL, FALSE);
> > >
> > > libusb_device_handle *handle = NULL;
> > > - int rc = libusb_open(dev->libusb_device, &handle);
> > > +
> > > + /*
> > > + Under Windows we need to avoid updating
> > > + list of devices when we are acquiring the device
> > > + */
> >
> > I'd rather have
> >
> > /* start comment
> > * end comment */
> >
> > but I don't mind much.
> >
> > Apart from those comments, not much to add.
> >
> > Victor
> >
> > > + set_redirecting(ch->backend, TRUE);
> > > +
> > > + rc = libusb_open(dev->libusb_device, &handle);
> > > +
> > > + set_redirecting(ch->backend, FALSE);
> > > +
> > > if (rc) {
> > > const char *desc = libusb_strerror(rc);
> > > g_set_error(error, SPICE_CLIENT_ERROR,
> > > SPICE_CLIENT_ERROR_FAILED,
> > > @@ -582,6 +713,7 @@ SpiceUsbBackendChannel
> > > *spice_usb_backend_channel_new(SpiceUsbBackend *be,
> > > SPICE_DEBUG("%s >>", __FUNCTION__);
> > > ch->user_data = SPICE_USBREDIR_CHANNEL(user_data);
> > > if (be->libusb_context) {
> > > + ch->backend = be;
> > > ch->usbredirhost = usbredirhost_open_full(
> > > be->libusb_context,
> > > NULL,
> > > diff --git a/src/usb-backend.h b/src/usb-backend.h
> > > index cbb73c2..6da3981 100644
> > > --- a/src/usb-backend.h
> > > +++ b/src/usb-backend.h
> > > @@ -56,14 +56,6 @@ enum {
> > > SpiceUsbBackend *spice_usb_backend_new(GError **error);
> > > void spice_usb_backend_delete(SpiceUsbBackend *context);
> > >
> > > -/*
> > > -returns newly-allocated null-terminated list of
> > > -SpiceUsbBackendDevice pointers.
> > > -The caller must call spice_usb_backend_free_device_list
> > > -after it finishes list processing
> > > -*/
> > > -SpiceUsbBackendDevice
> > > **spice_usb_backend_get_device_list(SpiceUsbBackend *backend);
> > > -void spice_usb_backend_free_device_list(SpiceUsbBackendDevice
> > > **devlist);
> > > gboolean spice_usb_backend_handle_events(SpiceUsbBackend *be);
> > > void spice_usb_backend_interrupt_event_handler(SpiceUsbBackend *be);
> > > gboolean spice_usb_backend_register_hotplug(SpiceUsbBackend *be,
> > > diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c
> > > index 9aba275..9300ad2 100644
> > > --- a/src/usb-device-manager.c
> > > +++ b/src/usb-device-manager.c
> > > @@ -29,7 +29,6 @@
> > > #ifdef G_OS_WIN32
> > > #include <windows.h>
> > > #include "usbdk_api.h"
> > > -#include "win-usb-dev.h"
> > > #endif
> > >
> > > #include "channel-usbredir-priv.h"
> > > @@ -101,12 +100,10 @@ struct _SpiceUsbDeviceManagerPrivate {
> > > struct usbredirfilter_rule *redirect_on_connect_rules;
> > > int auto_conn_filter_rules_count;
> > > int redirect_on_connect_rules_count;
> > > + gboolean redirecting;
> > > #ifdef G_OS_WIN32
> > > - GUdevClient *udev;
> > > usbdk_api_wrapper *usbdk_api;
> > > HANDLE usbdk_hider_handle;
> > > -#else
> > > - gboolean redirecting; /* Handled by GUdevClient in the gudev case */
> > > #endif
> > > GPtrArray *devices;
> > > GPtrArray *channels;
> > > @@ -139,16 +136,9 @@ static void channel_destroy(SpiceSession *session,
> > > SpiceChannel *channel,
> > > gpointer user_data);
> > > static void channel_event(SpiceChannel *channel, SpiceChannelEvent
> > > event,
> > > gpointer user_data);
> > > -#ifdef G_OS_WIN32
> > > -static void spice_usb_device_manager_uevent_cb(GUdevClient *client,
> > > - SpiceUsbBackendDevice
> > > *udevice,
> > > - gboolean add,
> > > - gpointer
> > > user_data);
> > > -#else
> > > static void spice_usb_device_manager_hotplug_cb(void *user_data,
> > > SpiceUsbBackendDevice
> > > *dev,
> > > gboolean added);
> > > -#endif
> > > static void spice_usb_device_manager_check_redir_on_connect(
> > > SpiceUsbDeviceManager *self, SpiceChannel *channel);
> > >
> > > @@ -190,11 +180,7 @@ G_DEFINE_BOXED_TYPE(SpiceUsbDevice,
> > > spice_usb_device,
> > > static void
> > > _set_redirecting(SpiceUsbDeviceManager *self, gboolean is_redirecting)
> > > {
> > > -#ifdef G_OS_WIN32
> > > - g_object_set(self->priv->udev, "redirecting", is_redirecting, NULL);
> > > -#else
> > > self->priv->redirecting = is_redirecting;
> > > -#endif
> > > }
> > >
> > > #else
> > > @@ -215,10 +201,6 @@ gboolean
> > > spice_usb_device_manager_is_redirecting(SpiceUsbDeviceManager *self)
> > > {
> > > #ifndef USE_USBREDIR
> > > return FALSE;
> > > -#elif defined(G_OS_WIN32)
> > > - gboolean redirecting;
> > > - g_object_get(self->priv->udev, "redirecting", &redirecting, NULL);
> > > - return redirecting;
> > > #else
> > > return self->priv->redirecting;
> > > #endif
> > > @@ -267,29 +249,18 @@ static gboolean
> > > spice_usb_device_manager_initable_init(GInitable *initable,
> > > GList *list;
> > > GList *it;
> > >
> > > - /* Start listening for usb devices plug / unplug */
> > > -#ifdef G_OS_WIN32
> > > - priv->udev = g_udev_client_new();
> > > - if (priv->udev == NULL) {
> > > - g_warning("Error initializing GUdevClient");
> > > - return FALSE;
> > > - }
> > > - priv->context = g_udev_client_get_context(priv->udev);
> > > - g_signal_connect(G_OBJECT(priv->udev), "uevent",
> > > - G_CALLBACK(spice_usb_device_manager_uevent_cb),
> > > self);
> > > - /* Do coldplug (detection of already connected devices) */
> > > - g_udev_client_report_devices(priv->udev);
> > > -#else
> > > /* Initialize libusb */
> > > priv->context = spice_usb_backend_new(err);
> > > if (!priv->context) {
> > > return FALSE;
> > > }
> > >
> > > + /* Start listening for usb devices plug / unplug */
> > > if (!spice_usb_backend_register_hotplug(priv->context, self,
> > > spice_usb_device_manager_hotplug_cb))
> > > {
> > > return FALSE;
> > > }
> > > +#ifndef G_OS_WIN32
> > > spice_usb_device_manager_start_event_listening(self, NULL);
> > > #endif
> > >
> > > @@ -324,8 +295,9 @@ static void spice_usb_device_manager_dispose(GObject
> > > *gobject)
> > > g_warn_if_reached();
> > > g_atomic_int_set(&priv->event_thread_run, FALSE);
> > > }
> > > - spice_usb_backend_deregister_hotplug(priv->context);
> > > #endif
> > > + spice_usb_backend_deregister_hotplug(priv->context);
> > > +
> > > if (priv->event_thread) {
> > > g_warn_if_fail(g_atomic_int_get(&priv->event_thread_run) ==
> > > FALSE);
> > > g_atomic_int_set(&priv->event_thread_run, FALSE);
> > > @@ -350,14 +322,10 @@ static void
> > > spice_usb_device_manager_finalize(GObject *gobject)
> > > if (priv->devices) {
> > > g_ptr_array_unref(priv->devices);
> > > }
> > > -#ifdef G_OS_WIN32
> > > - g_clear_object(&priv->udev);
> > > -#endif
> > > g_return_if_fail(priv->event_thread == NULL);
> > > -#ifndef G_OS_WIN32
> > > - if (priv->context)
> > > + if (priv->context) {
> > > spice_usb_backend_delete(priv->context);
> > > -#endif
> > > + }
> > > free(priv->auto_conn_filter_rules);
> > > free(priv->redirect_on_connect_rules);
> > > #ifdef G_OS_WIN32
> > > @@ -891,20 +859,6 @@ static void
> > > spice_usb_device_manager_remove_dev(SpiceUsbDeviceManager *self,
> > > spice_usb_device_unref(device);
> > > }
> > >
> > > -#ifdef G_OS_WIN32
> > > -static void spice_usb_device_manager_uevent_cb(GUdevClient *client,
> > > - SpiceUsbBackendDevice
> > > *bdev,
> > > - gboolean add,
> > > - gpointer
> > > user_data)
> > > -{
> > > - SpiceUsbDeviceManager *self = SPICE_USB_DEVICE_MANAGER(user_data);
> > > -
> > > - if (add)
> > > - spice_usb_device_manager_add_dev(self, bdev);
> > > - else
> > > - spice_usb_device_manager_remove_dev(self, bdev);
> > > -}
> > > -#else
> > > struct hotplug_idle_cb_args {
> > > SpiceUsbDeviceManager *self;
> > > SpiceUsbBackendDevice *device;
> > > @@ -940,7 +894,6 @@ static void spice_usb_device_manager_hotplug_cb(void
> > > *user_data,
> > > args->added = added;
> > > g_idle_add(spice_usb_device_manager_hotplug_idle_cb, args);
> > > }
> > > -#endif // USE_USBREDIR
> > >
> > > static void spice_usb_device_manager_channel_connect_cb(
> > > GObject *gobject, GAsyncResult *channel_res, gpointer user_data)
> > > diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c
> > > deleted file mode 100644
> > > index c2a5115..0000000
> > > --- a/src/win-usb-dev.c
> > > +++ /dev/null
> > > @@ -1,436 +0,0 @@
> > > -/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> > > -/*
> > > - Copyright (C) 2012 Red Hat, Inc.
> > > -
> > > - Red Hat Authors:
> > > - Arnon Gilboa <agilboa at redhat.com>
> > > - Uri Lublin <uril at redhat.com>
> > > -
> > > - This library is free software; you can redistribute it and/or
> > > - modify it under the terms of the GNU Lesser General Public
> > > - License as published by the Free Software Foundation; either
> > > - version 2.1 of the License, or (at your option) any later version.
> > > -
> > > - This library is distributed in the hope that it will be useful,
> > > - but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > > - Lesser General Public License for more details.
> > > -
> > > - You should have received a copy of the GNU Lesser General Public
> > > - License along with this library; if not, see
> > > <http://www.gnu.org/licenses/>.
> > > -*/
> > > -
> > > -#include "config.h"
> > > -
> > > -#include <windows.h>
> > > -#include "win-usb-dev.h"
> > > -#include "spice-marshal.h"
> > > -#include "spice-util.h"
> > > -
> > > -enum {
> > > - PROP_0,
> > > - PROP_REDIRECTING
> > > -};
> > > -
> > > -struct _GUdevClientPrivate {
> > > - SpiceUsbBackend *ctx;
> > > - GList *udev_list;
> > > - HWND hwnd;
> > > - gboolean redirecting;
> > > -};
> > > -
> > > -#define G_UDEV_CLIENT_WINCLASS_NAME TEXT("G_UDEV_CLIENT")
> > > -
> > > -static void g_udev_client_initable_iface_init(GInitableIface *iface);
> > > -
> > > -G_DEFINE_TYPE_WITH_CODE(GUdevClient, g_udev_client, G_TYPE_OBJECT,
> > > - G_ADD_PRIVATE(GUdevClient)
> > > - G_IMPLEMENT_INTERFACE(G_TYPE_INITABLE,
> > > g_udev_client_initable_iface_init))
> > > -
> > > -enum
> > > -{
> > > - UEVENT_SIGNAL,
> > > - LAST_SIGNAL,
> > > -};
> > > -
> > > -static guint signals[LAST_SIGNAL] = { 0, };
> > > -static GUdevClient *singleton = NULL;
> > > -
> > > -static LRESULT CALLBACK wnd_proc(HWND hwnd, UINT message, WPARAM wparam,
> > > LPARAM lparam);
> > > -
> > > -//uncomment to debug gudev device lists.
> > > -//#define DEBUG_GUDEV_DEVICE_LISTS
> > > -
> > > -#ifdef DEBUG_GUDEV_DEVICE_LISTS
> > > -static void g_udev_device_print_list(GList *l, const gchar *msg);
> > > -#else
> > > -static void g_udev_device_print_list(GList *l, const gchar *msg) {}
> > > -#endif
> > > -static void g_udev_device_print(SpiceUsbBackendDevice *dev, const gchar
> > > *msg);
> > > -
> > > -GQuark g_udev_client_error_quark(void)
> > > -{
> > > - return g_quark_from_static_string("win-gudev-client-error-quark");
> > > -}
> > > -
> > > -GUdevClient *g_udev_client_new(void)
> > > -{
> > > - if (singleton != NULL)
> > > - return g_object_ref(singleton);
> > > -
> > > - singleton = g_initable_new(G_UDEV_TYPE_CLIENT, NULL, NULL, NULL);
> > > - return singleton;
> > > -}
> > > -
> > > -SpiceUsbBackend *g_udev_client_get_context(GUdevClient *client)
> > > -{
> > > - return client->priv->ctx;
> > > -}
> > > -
> > > -/*
> > > - * devs [in,out] an empty devs list in, full devs list out
> > > - * Returns: number-of-devices, or a negative value on error.
> > > - */
> > > -static ssize_t
> > > -g_udev_client_list_devices(GUdevClient *self, GList **devs,
> > > - GError **err, const gchar *name)
> > > -{
> > > - SpiceUsbBackendDevice **lusb_list, **dev;
> > > - GUdevClientPrivate *priv;
> > > - ssize_t n;
> > > -
> > > - g_return_val_if_fail(G_UDEV_IS_CLIENT(self), -1);
> > > - g_return_val_if_fail(devs != NULL, -2);
> > > -
> > > - priv = self->priv;
> > > -
> > > - g_return_val_if_fail(self->priv->ctx != NULL, -3);
> > > -
> > > - lusb_list = spice_usb_backend_get_device_list(priv->ctx);
> > > - if (!lusb_list) {
> > > - g_set_error(err, G_UDEV_CLIENT_ERROR,
> > > G_UDEV_CLIENT_LIBUSB_FAILED,
> > > - "%s: Error getting USB device list", name);
> > > - return -4;
> > > - }
> > > -
> > > - n = 0;
> > > - for (dev = lusb_list; *dev; dev++) {
> > > - *devs = g_list_prepend(*devs,
> > > spice_usb_backend_device_ref(*dev));
> > > - n++;
> > > - }
> > > - spice_usb_backend_free_device_list(lusb_list);
> > > -
> > > - return n;
> > > -}
> > > -
> > > -static void unreference_backend_device(gpointer data)
> > > -{
> > > - spice_usb_backend_device_unref((SpiceUsbBackendDevice *)data);
> > > -}
> > > -
> > > -static void g_udev_client_free_device_list(GList **devs)
> > > -{
> > > - g_return_if_fail(devs != NULL);
> > > - if (*devs) {
> > > - g_list_free_full(*devs, unreference_backend_device);
> > > - *devs = NULL;
> > > - }
> > > -}
> > > -
> > > -
> > > -static gboolean
> > > -g_udev_client_initable_init(GInitable *initable, GCancellable
> > > *cancellable,
> > > - GError **err)
> > > -{
> > > - GUdevClient *self;
> > > - GUdevClientPrivate *priv;
> > > - WNDCLASS wcls;
> > > -
> > > - g_return_val_if_fail(G_UDEV_IS_CLIENT(initable), FALSE);
> > > - g_return_val_if_fail(cancellable == NULL, FALSE);
> > > -
> > > - self = G_UDEV_CLIENT(initable);
> > > - priv = self->priv;
> > > -
> > > - priv->ctx = spice_usb_backend_new(err);
> > > - if (!priv->ctx) {
> > > - return FALSE;
> > > - }
> > > -
> > > -#ifdef G_OS_WIN32
> > > -#if LIBUSB_API_VERSION >= 0x01000106
> > > - libusb_set_option(priv->ctx, LIBUSB_OPTION_USE_USBDK);
> > > -#endif
> > > -#endif
> > > -
> > > - /* get initial device list */
> > > - if (g_udev_client_list_devices(self, &priv->udev_list, err,
> > > __FUNCTION__) < 0) {
> > > - goto g_udev_client_init_failed;
> > > - }
> > > -
> > > - g_udev_device_print_list(priv->udev_list, "init: first list is: ");
> > > -
> > > - /* create a hidden window */
> > > - memset(&wcls, 0, sizeof(wcls));
> > > - wcls.lpfnWndProc = wnd_proc;
> > > - wcls.lpszClassName = G_UDEV_CLIENT_WINCLASS_NAME;
> > > - if (!RegisterClass(&wcls)) {
> > > - DWORD e = GetLastError();
> > > - g_warning("RegisterClass failed , %ld", (long)e);
> > > - g_set_error(err, G_UDEV_CLIENT_ERROR,
> > > G_UDEV_CLIENT_WINAPI_FAILED,
> > > - "RegisterClass failed: %ld", (long)e);
> > > - goto g_udev_client_init_failed;
> > > - }
> > > - priv->hwnd = CreateWindow(G_UDEV_CLIENT_WINCLASS_NAME,
> > > - NULL, 0, 0, 0, 0, 0, NULL, NULL, NULL,
> > > NULL);
> > > - if (!priv->hwnd) {
> > > - DWORD e = GetLastError();
> > > - g_warning("CreateWindow failed: %ld", (long)e);
> > > - g_set_error(err, G_UDEV_CLIENT_ERROR,
> > > G_UDEV_CLIENT_LIBUSB_FAILED,
> > > - "CreateWindow failed: %ld", (long)e);
> > > - goto g_udev_client_init_failed_unreg;
> > > - }
> > > -
> > > - return TRUE;
> > > -
> > > - g_udev_client_init_failed_unreg:
> > > - UnregisterClass(G_UDEV_CLIENT_WINCLASS_NAME, NULL);
> > > - g_udev_client_init_failed:
> > > - return FALSE;
> > > -}
> > > -
> > > -static void g_udev_client_initable_iface_init(GInitableIface *iface)
> > > -{
> > > - iface->init = g_udev_client_initable_init;
> > > -}
> > > -
> > > -static void report_one_device(gpointer data, gpointer self)
> > > -{
> > > - g_signal_emit(self, signals[UEVENT_SIGNAL], 0, data, TRUE);
> > > -}
> > > -
> > > -void g_udev_client_report_devices(GUdevClient *self)
> > > -{
> > > - g_list_foreach(self->priv->udev_list, report_one_device, self);
> > > -}
> > > -
> > > -static void g_udev_client_init(GUdevClient *self)
> > > -{
> > > - self->priv = g_udev_client_get_instance_private(self);
> > > -}
> > > -
> > > -static void g_udev_client_finalize(GObject *gobject)
> > > -{
> > > - GUdevClient *self = G_UDEV_CLIENT(gobject);
> > > - GUdevClientPrivate *priv = self->priv;
> > > -
> > > - singleton = NULL;
> > > - DestroyWindow(priv->hwnd);
> > > - UnregisterClass(G_UDEV_CLIENT_WINCLASS_NAME, NULL);
> > > - g_udev_client_free_device_list(&priv->udev_list);
> > > -
> > > - /* free backend context */
> > > - g_warn_if_fail(priv->ctx != NULL);
> > > - spice_usb_backend_delete(priv->ctx);
> > > -
> > > - /* Chain up to the parent class */
> > > - if (G_OBJECT_CLASS(g_udev_client_parent_class)->finalize)
> > > - G_OBJECT_CLASS(g_udev_client_parent_class)->finalize(gobject);
> > > -}
> > > -
> > > -static void g_udev_client_get_property(GObject *gobject,
> > > - guint prop_id,
> > > - GValue *value,
> > > - GParamSpec *pspec)
> > > -{
> > > - GUdevClient *self = G_UDEV_CLIENT(gobject);
> > > - GUdevClientPrivate *priv = self->priv;
> > > -
> > > - switch (prop_id) {
> > > - case PROP_REDIRECTING:
> > > - g_value_set_boolean(value, priv->redirecting);
> > > - break;
> > > - default:
> > > - G_OBJECT_WARN_INVALID_PROPERTY_ID(gobject, prop_id, pspec);
> > > - break;
> > > - }
> > > -}
> > > -
> > > -static void handle_dev_change(GUdevClient *self);
> > > -
> > > -static void g_udev_client_set_property(GObject *gobject,
> > > - guint prop_id,
> > > - const GValue *value,
> > > - GParamSpec *pspec)
> > > -{
> > > - GUdevClient *self = G_UDEV_CLIENT(gobject);
> > > - GUdevClientPrivate *priv = self->priv;
> > > - gboolean old_val;
> > > -
> > > - switch (prop_id) {
> > > - case PROP_REDIRECTING:
> > > - old_val = priv->redirecting;
> > > - priv->redirecting = g_value_get_boolean(value);
> > > - if (old_val && !priv->redirecting) {
> > > - /* This is a redirection completion case.
> > > - Inject hotplug event in case we missed device changes
> > > - during redirection processing. */
> > > - handle_dev_change(self);
> > > - }
> > > - break;
> > > - default:
> > > - G_OBJECT_WARN_INVALID_PROPERTY_ID(gobject, prop_id, pspec);
> > > - break;
> > > - }
> > > -}
> > > -
> > > -static void g_udev_client_class_init(GUdevClientClass *klass)
> > > -{
> > > - GObjectClass *gobject_class = G_OBJECT_CLASS(klass);
> > > - GParamSpec *pspec;
> > > -
> > > - gobject_class->finalize = g_udev_client_finalize;
> > > - gobject_class->get_property = g_udev_client_get_property;
> > > - gobject_class->set_property = g_udev_client_set_property;
> > > -
> > > - signals[UEVENT_SIGNAL] =
> > > - g_signal_new("uevent",
> > > - G_OBJECT_CLASS_TYPE(klass),
> > > - G_SIGNAL_RUN_FIRST,
> > > - G_STRUCT_OFFSET(GUdevClientClass, uevent),
> > > - NULL, NULL,
> > > - g_cclosure_user_marshal_VOID__POINTER_BOOLEAN,
> > > - G_TYPE_NONE,
> > > - 2,
> > > - G_TYPE_POINTER,
> > > - G_TYPE_BOOLEAN);
> > > -
> > > - /**
> > > - * GUdevClient::redirecting:
> > > - *
> > > - * This property indicates when a redirection operation
> > > - * is in progress on a device. It's set back to FALSE
> > > - * once the device is fully redirected to the guest.
> > > - */
> > > - pspec = g_param_spec_boolean("redirecting", "Redirecting",
> > > - "USB redirection operation is in
> > > progress",
> > > - FALSE,
> > > - G_PARAM_READWRITE |
> > > G_PARAM_STATIC_STRINGS);
> > > -
> > > - g_object_class_install_property(gobject_class, PROP_REDIRECTING,
> > > pspec);
> > > -}
> > > -
> > > -/* comparing bus:addr and vid:pid */
> > > -static gint compare_usb_devices(gconstpointer a, gconstpointer b)
> > > -{
> > > - const UsbDeviceInformation *a_info, *b_info;
> > > - gboolean same_bus, same_addr, same_vid, same_pid;
> > > - a_info = spice_usb_backend_device_get_info((SpiceUsbBackendDevice
> > > *)a);
> > > - b_info = spice_usb_backend_device_get_info((SpiceUsbBackendDevice
> > > *)b);
> > > -
> > > -
> > > - same_bus = (a_info->bus == b_info->bus);
> > > - same_addr = (a_info->address == b_info->address);
> > > - same_vid = (a_info->vid == b_info->vid);
> > > - same_pid = (a_info->pid == b_info->pid);
> > > -
> > > - return (same_bus && same_addr && same_vid && same_pid) ? 0 : -1;
> > > -}
> > > -
> > > -static void update_device_list(GUdevClient *self, GList
> > > *new_device_list)
> > > -{
> > > - GList *dev;
> > > -
> > > - for (dev = g_list_first(new_device_list); dev != NULL; dev =
> > > g_list_next(dev)) {
> > > - GList *found = g_list_find_custom(self->priv->udev_list,
> > > dev->data, compare_usb_devices);
> > > - if (found != NULL) {
> > > - /* keep old existing libusb_device in the new list, when
> > > - usb-dev-manager will maintain its own list of
> > > libusb_device,
> > > - these lists will be completely consistent */
> > > - SpiceUsbBackendDevice *temp = found->data;
> > > - found->data = dev->data;
> > > - dev->data = temp;
> > > - }
> > > - }
> > > -
> > > - g_udev_client_free_device_list(&self->priv->udev_list);
> > > - self->priv->udev_list = new_device_list;
> > > -}
> > > -
> > > -static void notify_dev_state_change(GUdevClient *self,
> > > - GList *old_list,
> > > - GList *new_list,
> > > - gboolean add)
> > > -{
> > > - GList *dev;
> > > -
> > > - for (dev = g_list_first(old_list); dev != NULL; dev =
> > > g_list_next(dev)) {
> > > - GList *found = g_list_find_custom(new_list, dev->data,
> > > compare_usb_devices);
> > > - if (found == NULL) {
> > > - g_udev_device_print(dev->data, add ? "add" : "remove");
> > > - g_signal_emit(self, signals[UEVENT_SIGNAL], 0, dev->data,
> > > add);
> > > - }
> > > - }
> > > -}
> > > -
> > > -static void handle_dev_change(GUdevClient *self)
> > > -{
> > > - GUdevClientPrivate *priv = self->priv;
> > > - GError *err = NULL;
> > > - GList *now_devs = NULL;
> > > -
> > > - if (priv->redirecting == TRUE) {
> > > - /* On Windows, querying USB device list may return inconsistent
> > > results
> > > - if performed in parallel to redirection flow.
> > > - A simulated hotplug event will be injected after redirection
> > > - completion in order to process real device list changes that
> > > may
> > > - had taken place during redirection process. */
> > > - return;
> > > - }
> > > -
> > > - if(g_udev_client_list_devices(self, &now_devs, &err, __FUNCTION__) <
> > > 0) {
> > > - g_warning("could not retrieve device list");
> > > - return;
> > > - }
> > > -
> > > - g_udev_device_print_list(now_devs, "handle_dev_change: current
> > > list:");
> > > - g_udev_device_print_list(priv->udev_list, "handle_dev_change:
> > > previous list:");
> > > -
> > > - /* Unregister devices that are not present anymore */
> > > - notify_dev_state_change(self, priv->udev_list, now_devs, FALSE);
> > > -
> > > - /* Register newly inserted devices */
> > > - notify_dev_state_change(self, now_devs, priv->udev_list, TRUE);
> > > -
> > > - /* keep most recent info: free previous list, and keep current list
> > > */
> > > - update_device_list(self, now_devs);
> > > -}
> > > -
> > > -static LRESULT CALLBACK wnd_proc(HWND hwnd, UINT message, WPARAM wparam,
> > > LPARAM lparam)
> > > -{
> > > - /* Only DBT_DEVNODES_CHANGED recieved */
> > > - if (message == WM_DEVICECHANGE) {
> > > - handle_dev_change(singleton);
> > > - }
> > > - return DefWindowProc(hwnd, message, wparam, lparam);
> > > -}
> > > -
> > > -#ifdef DEBUG_GUDEV_DEVICE_LISTS
> > > -static void g_udev_device_print_list(GList *l, const gchar *msg)
> > > -{
> > > - GList *it;
> > > -
> > > - for (it = g_list_first(l); it != NULL; it=g_list_next(it)) {
> > > - g_udev_device_print(it->data, msg);
> > > - }
> > > -}
> > > -#endif
> > > -
> > > -static void g_udev_device_print(SpiceUsbBackendDevice *dev, const gchar
> > > *msg)
> > > -{
> > > - const UsbDeviceInformation *info =
> > > spice_usb_backend_device_get_info(dev);
> > > -
> > > - SPICE_DEBUG("%s: %d.%d 0x%04x:0x%04x class %d", msg,
> > > - info->bus, info->address,
> > > - info->vid, info->pid, info->class);
> > > -}
> > > diff --git a/src/win-usb-dev.h b/src/win-usb-dev.h
> > > deleted file mode 100644
> > > index fdfe73a..0000000
> > > --- a/src/win-usb-dev.h
> > > +++ /dev/null
> > > @@ -1,84 +0,0 @@
> > > -/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> > > -/*
> > > - Copyright (C) 2012 Red Hat, Inc.
> > > -
> > > - Red Hat Authors:
> > > - Arnon Gilboa <agilboa at redhat.com>
> > > - Uri Lublin <uril at redhat.com>
> > > -
> > > - This library is free software; you can redistribute it and/or
> > > - modify it under the terms of the GNU Lesser General Public
> > > - License as published by the Free Software Foundation; either
> > > - version 2.1 of the License, or (at your option) any later version.
> > > -
> > > - This library is distributed in the hope that it will be useful,
> > > - but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > > - Lesser General Public License for more details.
> > > -
> > > - You should have received a copy of the GNU Lesser General Public
> > > - License along with this library; if not, see
> > > <http://www.gnu.org/licenses/>.
> > > -*/
> > > -#ifndef __WIN_USB_DEV_H__
> > > -#define __WIN_USB_DEV_H__
> > > -
> > > -#include <gio/gio.h>
> > > -#include "usb-backend.h"
> > > -
> > > -G_BEGIN_DECLS
> > > -
> > > -/* GUdevClient */
> > > -
> > > -#define G_UDEV_TYPE_CLIENT (g_udev_client_get_type())
> > > -#define G_UDEV_CLIENT(o) (G_TYPE_CHECK_INSTANCE_CAST((o),
> > > G_UDEV_TYPE_CLIENT, GUdevClient))
> > > -#define G_UDEV_CLIENT_CLASS(k) (G_TYPE_CHECK_CLASS_CAST((k),
> > > G_UDEV_TYPE_CLIENT, GUdevClientClass))
> > > -#define G_UDEV_IS_CLIENT(o) (G_TYPE_CHECK_INSTANCE_TYPE((o),
> > > G_UDEV_TYPE_CLIENT))
> > > -#define G_UDEV_IS_CLIENT_CLASS(k) (G_TYPE_CHECK_CLASS_TYPE((k),
> > > G_UDEV_TYPE_CLIENT))
> > > -#define G_UDEV_CLIENT_GET_CLASS(o) (G_TYPE_INSTANCE_GET_CLASS((o),
> > > G_UDEV_TYPE_CLIENT, GUdevClientClass))
> > > -
> > > -typedef struct _GUdevClient GUdevClient;
> > > -typedef struct _GUdevClientClass GUdevClientClass;
> > > -typedef struct _GUdevClientPrivate GUdevClientPrivate;
> > > -
> > > -struct _GUdevClient
> > > -{
> > > - GObject parent;
> > > -
> > > - GUdevClientPrivate *priv;
> > > -};
> > > -
> > > -struct _GUdevClientClass
> > > -{
> > > - GObjectClass parent_class;
> > > -
> > > - /* signals */
> > > - void (*uevent)(GUdevClient *client, SpiceUsbBackendDevice *device,
> > > gboolean add);
> > > -};
> > > -
> > > -GType g_udev_client_get_type(void) G_GNUC_CONST;
> > > -GUdevClient *g_udev_client_new(void);
> > > -SpiceUsbBackend *g_udev_client_get_context(GUdevClient *client);
> > > -void g_udev_client_report_devices(GUdevClient *client);
> > > -
> > > -GQuark g_udev_client_error_quark(void);
> > > -#define G_UDEV_CLIENT_ERROR g_udev_client_error_quark()
> > > -
> > > -/**
> > > - * GUdevClientError:
> > > - * @G_UDEV_CLIENT_ERROR_FAILED: generic error code
> > > - * @G_UDEV_CLIENT_LIBUSB_FAILED: a libusb call failed
> > > - * @G_UDEV_CLIENT_WINAPI_FAILED: a winapi call failed
> > > - *
> > > - * Error codes returned by spice-client API.
> > > - */
> > > -typedef enum
> > > -{
> > > - G_UDEV_CLIENT_ERROR_FAILED = 1,
> > > - G_UDEV_CLIENT_LIBUSB_FAILED,
> > > - G_UDEV_CLIENT_WINAPI_FAILED
> > > -} GUdevClientError;
> > > -
> > > -
> > > -G_END_DECLS
> > > -
> > > -#endif /* __WIN_USB_DEV_H__ */
> > > --
> > > 2.17.1
> > >
> > > _______________________________________________
> > > Spice-devel mailing list
> > > Spice-devel at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
More information about the Spice-devel
mailing list