[Spice-devel] [spice-gtk v2 12/13] usb-redir: use persistent libusb_device under Windows also
Unify SpiceUsbDeviceInfo for Linux and Windows by using persistent libusb_device. Probably there was some problem doing it long time ago with winusb, but at least with existing libusb and UsbDk the the libusb device can be accessed as long as it is referenced. Signed-off-by: Yuri Benditovich --- src/usb-device-manager.c | 105 --- 1 file changed, 105 deletions(-) diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c index a163501..621e303 100644 --- a/src/usb-device-manager.c +++ b/src/usb-device-manager.c @@ -136,11 +136,7 @@ typedef struct _SpiceUsbDeviceInfo { guint16 vid; guint16 pid; gboolean isochronous; -#ifdef G_OS_WIN32 -guint8 state; -#else libusb_device *libdev; -#endif gintref; } SpiceUsbDeviceInfo; @@ -758,13 +754,11 @@ gconstpointer spice_usb_device_get_libusb_device(const SpiceUsbDevice *device G_GNUC_UNUSED) { #ifdef USE_USBREDIR -#ifndef G_OS_WIN32 const SpiceUsbDeviceInfo *info = (const SpiceUsbDeviceInfo *)device; g_return_val_if_fail(info != NULL, FALSE); return info->libdev; -#endif #endif return NULL; } @@ -886,17 +880,6 @@ spice_usb_device_manager_device_match(SpiceUsbDeviceManager *self, SpiceUsbDevic spice_usb_device_get_devaddr(device) == address); } -#ifdef G_OS_WIN32 -static gboolean -spice_usb_device_manager_libdev_match(SpiceUsbDeviceManager *self, libusb_device *libdev, - const int bus, const int address) -{ -/* match functions for Linux/UsbDk -- match by bus.addr */ -return (libusb_get_bus_number(libdev) == bus && -libusb_get_device_address(libdev) == address); -} -#endif - static SpiceUsbDevice* spice_usb_device_manager_find_device(SpiceUsbDeviceManager *self, const int bus, const int address) @@ -1150,10 +1133,6 @@ static void spice_usb_device_manager_check_redir_on_connect( continue; libdev = spice_usb_device_manager_device_to_libdev(self, device); -#ifdef G_OS_WIN32 -if (libdev == NULL) -continue; -#endif if (usbredirhost_check_device_filter( priv->redirect_on_connect_rules, priv->redirect_on_connect_rules_count, @@ -1258,10 +1237,6 @@ GPtrArray* spice_usb_device_manager_get_devices_with_filter( if (rules) { libusb_device *libdev = spice_usb_device_manager_device_to_libdev(self, device); -#ifdef G_OS_WIN32 -if (libdev == NULL) -continue; -#endif if (usbredirhost_check_device_filter(rules, count, libdev, 0) != 0) continue; } @@ -1353,24 +1328,6 @@ _spice_usb_device_manager_connect_device_async(SpiceUsbDeviceManager *self, continue; /* Skip already used channels */ libdev = spice_usb_device_manager_device_to_libdev(self, device); -#ifdef G_OS_WIN32 -if (libdev == NULL) { -/* Most likely, the device was plugged out at driver installation - * time, and its remove-device event was ignored. - * So remove the device now - */ -SPICE_DEBUG("libdev does not exist for %p -- removing", device); -spice_usb_device_ref(device); -g_ptr_array_remove(priv->devices, device); -g_signal_emit(self, signals[DEVICE_REMOVED], 0, device); -spice_usb_device_unref(device); -g_task_return_new_error(task, -SPICE_CLIENT_ERROR, -SPICE_CLIENT_ERROR_FAILED, -_("Device was not found")); -goto done; -} -#endif spice_usbredir_channel_connect_device_async(channel, libdev, device, @@ -1642,13 +1599,6 @@ spice_usb_device_manager_can_redirect_device(SpiceUsbDeviceManager *self, libusb_device *libdev; libdev = spice_usb_device_manager_device_to_libdev(self, device); -#ifdef G_OS_WIN32 -if (libdev == NULL) { -g_set_error_literal(err, SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED, -_("Some USB devices were not found")); -return FALSE; -} -#endif filter_ok = (usbredirhost_check_device_filter( guest_filter_rules, guest_filter_rules_count, libdev, 0) == 0); @@ -1799,9 +1749,7 @@ static SpiceUsbDeviceInfo *spice_usb_device_new(libusb_device *libdev) info->pid = pid; info->ref = 1; info->isochronous = probe_isochronous_endpoint(libdev); -#ifndef G_OS_WIN32 info->libdev = libusb_ref_device(libdev); -#endif return info; } @@ -1937,14 +1885,11 @@ static void spice_usb_device_unref(SpiceUsbDevice *device) ref_count_is_0 =
[Spice-devel] [spice-gtk v2 09/13] win-usb-dev: do not allocate memory for hub devices
When processing list of USB devices, avoid allocating memory for devices which later will be skipped. Use existing libusb struct first to check whether the device shall be excluded. Signed-off-by: Yuri Benditovich --- src/win-usb-dev.c | 27 --- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c index bb11e8b..b3b2ed8 100644 --- a/src/win-usb-dev.c +++ b/src/win-usb-dev.c @@ -97,7 +97,7 @@ static void g_udev_device_print_list(GList *l, const gchar *msg) {} #endif static void g_udev_device_print(GUdevDevice *udev, const gchar *msg); -static gboolean g_udev_skip_search(GUdevDevice *udev); +static gboolean g_udev_skip_search(libusb_device *dev); GQuark g_udev_client_error_quark(void) { @@ -152,13 +152,12 @@ g_udev_client_list_devices(GUdevClient *self, GList **devs, n = 0; for (dev = lusb_list; *dev; dev++) { +if (g_udev_skip_search(*dev)) { +continue; +} udevinfo = g_new0(GUdevDeviceInfo, 1); get_usb_dev_info(*dev, udevinfo); udevice = g_udev_device_new(udevinfo); -if (g_udev_skip_search(udevice)) { -g_object_unref(udevice); -continue; -} *devs = g_list_prepend(*devs, udevice); n++; } @@ -549,19 +548,17 @@ static void g_udev_device_print(GUdevDevice *udev, const gchar *msg) udevinfo->vid, udevinfo->pid, udevinfo->class); } -static gboolean g_udev_skip_search(GUdevDevice *udev) +static gboolean g_udev_skip_search(libusb_device *dev) { -GUdevDeviceInfo* udevinfo; gboolean skip; +uint8_t addr = libusb_get_device_address(dev); +struct libusb_device_descriptor desc; -g_return_val_if_fail(G_UDEV_DEVICE(udev), FALSE); - -udevinfo = udev->priv->udevinfo; -g_return_val_if_fail(udevinfo != NULL, FALSE); +libusb_get_device_descriptor(dev, ); -skip = ((udevinfo->addr == 0xff) || /* root hub (HCD) */ -(udevinfo->addr == 1) || /* root hub addr */ -(udevinfo->class == LIBUSB_CLASS_HUB) || /* hub*/ -(udevinfo->addr == 0)); /* bad address */ +skip = ((addr == 0xff) || /* root hub (HCD) */ +(addr == 1) || /* root hub addr */ +(desc.bDeviceClass == LIBUSB_CLASS_HUB) || /* hub*/ +(addr == 0)); /* bad address */ return skip; } -- 2.17.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v2 07/13] usb-redir: discard cold-plug device list under Windows
Discard the optimization of initial device enumeration. Just after connection to 'udev' signal request to report all the devices one by one as if they are inserted. Further commits will remove device enumeration in usb-dev-manager completely. Signed-off-by: Yuri Benditovich --- src/usb-device-manager.c | 19 +++ src/win-usb-dev.c| 11 +++ src/win-usb-dev.h| 2 +- 3 files changed, 11 insertions(+), 21 deletions(-) diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c index a9f19bf..c5e662c 100644 --- a/src/usb-device-manager.c +++ b/src/usb-device-manager.c @@ -104,7 +104,6 @@ struct _SpiceUsbDeviceManagerPrivate { int redirect_on_connect_rules_count; #ifdef G_OS_WIN32 GUdevClient *udev; -libusb_device **coldplug_list; /* Avoid needless reprobing during init */ #else gboolean redirecting; /* Handled by GUdevClient in the gudev case */ libusb_hotplug_callback_handle hp_handle; @@ -307,15 +306,7 @@ static gboolean spice_usb_device_manager_initable_init(GInitable *initable, g_signal_connect(G_OBJECT(priv->udev), "uevent", G_CALLBACK(spice_usb_device_manager_uevent_cb), self); /* Do coldplug (detection of already connected devices) */ -libusb_get_device_list(priv->context, >coldplug_list); -list = g_udev_client_query_by_subsystem(priv->udev, "usb"); -for (it = g_list_first(list); it; it = g_list_next(it)) { -spice_usb_device_manager_add_udev(self, it->data); -g_object_unref(it->data); -} -g_list_free(list); -libusb_free_device_list(priv->coldplug_list, 1); -priv->coldplug_list = NULL; +g_udev_client_report_devices(priv->udev); #else rc = libusb_hotplug_register_callback(priv->context, LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED | LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT, @@ -1049,10 +1040,7 @@ static void spice_usb_device_manager_add_udev(SpiceUsbDeviceManager *self, return; } -if (priv->coldplug_list) -dev_list = priv->coldplug_list; -else -libusb_get_device_list(priv->context, _list); +libusb_get_device_list(priv->context, _list); for (i = 0; dev_list && dev_list[i]; i++) { if (spice_usb_device_manager_libdev_match(self, dev_list[i], bus, address)) { @@ -1067,8 +1055,7 @@ static void spice_usb_device_manager_add_udev(SpiceUsbDeviceManager *self, g_warning("Could not find USB device to add " DEV_ID_FMT, (guint) bus, (guint) address); -if (!priv->coldplug_list) -libusb_free_device_list(dev_list, 1); +libusb_free_device_list(dev_list, 1); } static void spice_usb_device_manager_remove_udev(SpiceUsbDeviceManager *self, diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c index 6273cbe..50db4e5 100644 --- a/src/win-usb-dev.c +++ b/src/win-usb-dev.c @@ -247,11 +247,14 @@ static void g_udev_client_initable_iface_init(GInitableIface *iface) iface->init = g_udev_client_initable_init; } -GList *g_udev_client_query_by_subsystem(GUdevClient *self, const gchar *subsystem) +static void report_one_device(gpointer data, gpointer self) { -GList *l = g_list_copy(self->priv->udev_list); -g_list_foreach(l, (GFunc)g_object_ref, NULL); -return l; +g_signal_emit(self, signals[UEVENT_SIGNAL], 0, "add", data); +} + +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) diff --git a/src/win-usb-dev.h b/src/win-usb-dev.h index b7b7eda..bca8285 100644 --- a/src/win-usb-dev.h +++ b/src/win-usb-dev.h @@ -81,7 +81,7 @@ struct _GUdevClientClass GType g_udev_client_get_type(void) G_GNUC_CONST; GUdevClient *g_udev_client_new(void); libusb_context *g_udev_client_get_context(GUdevClient *client); -GList *g_udev_client_query_by_subsystem(GUdevClient *client, const gchar *subsystem); +void g_udev_client_report_devices(GUdevClient *client); GType g_udev_device_get_type(void) G_GNUC_CONST; const gchar *g_udev_device_get_property(GUdevDevice *udev, const gchar *property); -- 2.17.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v2 03/13] usb-redir: reuse libusb context under Windows
Do not create own libusb context in usb-device-manager. Reuse existing context created by win-usb-dev instead, this will later allow usb-device-manager to use libusb devices enumerated in win-usb-dev. Signed-off-by: Yuri Benditovich --- src/usb-device-manager.c | 11 +-- src/win-usb-dev.c| 4 src/win-usb-dev.h| 1 + 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c index 6435be8..debba4d 100644 --- a/src/usb-device-manager.c +++ b/src/usb-device-manager.c @@ -282,8 +282,9 @@ static gboolean spice_usb_device_manager_initable_init(GInitable *initable, SpiceUsbDeviceManagerPrivate *priv = self->priv; GList *list; GList *it; -int rc; +#ifndef G_OS_WIN32 +int rc; /* Initialize libusb */ rc = libusb_init(>context); if (rc < 0) { @@ -293,11 +294,6 @@ static gboolean spice_usb_device_manager_initable_init(GInitable *initable, "Error initializing USB support: %s [%i]", desc, rc); return FALSE; } - -#ifdef G_OS_WIN32 -#if LIBUSB_API_VERSION >= 0x01000106 -libusb_set_option(priv->context, LIBUSB_OPTION_USE_USBDK); -#endif #endif /* Start listening for usb devices plug / unplug */ @@ -307,6 +303,7 @@ static gboolean spice_usb_device_manager_initable_init(GInitable *initable, 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) */ @@ -402,8 +399,10 @@ static void spice_usb_device_manager_finalize(GObject *gobject) g_clear_object(>udev); #endif g_return_if_fail(priv->event_thread == NULL); +#ifndef G_OS_WIN32 if (priv->context) libusb_exit(priv->context); +#endif free(priv->auto_conn_filter_rules); free(priv->redirect_on_connect_rules); #ifdef G_OS_WIN32 diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c index d3ff55a..b08bf02 100644 --- a/src/win-usb-dev.c +++ b/src/win-usb-dev.c @@ -113,6 +113,10 @@ GUdevClient *g_udev_client_new(void) return singleton; } +libusb_context *g_udev_client_get_context(GUdevClient *client) +{ +return client->priv->ctx; +} /* * devs [in,out] an empty devs list in, full devs list out diff --git a/src/win-usb-dev.h b/src/win-usb-dev.h index 0f34a01..f3c7466 100644 --- a/src/win-usb-dev.h +++ b/src/win-usb-dev.h @@ -80,6 +80,7 @@ struct _GUdevClientClass GType g_udev_client_get_type(void) G_GNUC_CONST; GUdevClient *g_udev_client_new(void); +libusb_context *g_udev_client_get_context(GUdevClient *client); GList *g_udev_client_query_by_subsystem(GUdevClient *client, const gchar *subsystem); GType g_udev_device_get_type(void) G_GNUC_CONST; -- 2.17.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v2 08/13] usb-redir: change signal prototype of win-usb-dev
Changing signal definition from (boxed-boxed) to (pointer,int). There is no need for additional referencing of GUdevDevice object before signal callback. Second parameter (action) is FALSE for device removal and TRUE for device addition. Signed-off-by: Yuri Benditovich --- src/spice-marshal.txt| 1 + src/usb-device-manager.c | 8 src/win-usb-dev.c| 18 +- src/win-usb-dev.h| 2 +- 4 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/spice-marshal.txt b/src/spice-marshal.txt index cf35790..46be405 100644 --- a/src/spice-marshal.txt +++ b/src/spice-marshal.txt @@ -5,6 +5,7 @@ VOID:UINT,UINT,UINT,UINT VOID:INT,INT,INT,INT,POINTER VOID:INT,INT,INT,INT,INT,POINTER VOID:POINTER,INT +VOID:POINTER,BOOLEAN BOOLEAN:POINTER,UINT BOOLEAN:UINT VOID:UINT,POINTER,UINT diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c index c5e662c..c99d359 100644 --- a/src/usb-device-manager.c +++ b/src/usb-device-manager.c @@ -153,8 +153,8 @@ 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, - const gchar *action, GUdevDevice *udevice, + gboolean add, gpointer user_data); static void spice_usb_device_manager_add_udev(SpiceUsbDeviceManager *self, GUdevDevice*udev); @@ -1070,15 +1070,15 @@ static void spice_usb_device_manager_remove_udev(SpiceUsbDeviceManager *self, } static void spice_usb_device_manager_uevent_cb(GUdevClient *client, - const gchar *action, GUdevDevice *udevice, + gboolean add, gpointer user_data) { SpiceUsbDeviceManager *self = SPICE_USB_DEVICE_MANAGER(user_data); -if (g_str_equal(action, "add")) +if (add) spice_usb_device_manager_add_udev(self, udevice); -else if (g_str_equal (action, "remove")) +else spice_usb_device_manager_remove_udev(self, udevice); } #else diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c index 50db4e5..bb11e8b 100644 --- a/src/win-usb-dev.c +++ b/src/win-usb-dev.c @@ -249,7 +249,7 @@ static void g_udev_client_initable_iface_init(GInitableIface *iface) static void report_one_device(gpointer data, gpointer self) { -g_signal_emit(self, signals[UEVENT_SIGNAL], 0, "add", data); +g_signal_emit(self, signals[UEVENT_SIGNAL], 0, data, TRUE); } void g_udev_client_report_devices(GUdevClient *self) @@ -342,11 +342,11 @@ static void g_udev_client_class_init(GUdevClientClass *klass) G_SIGNAL_RUN_FIRST, G_STRUCT_OFFSET(GUdevClientClass, uevent), NULL, NULL, - g_cclosure_user_marshal_VOID__BOXED_BOXED, + g_cclosure_user_marshal_VOID__POINTER_BOOLEAN, G_TYPE_NONE, 2, - G_TYPE_STRING, - G_UDEV_TYPE_DEVICE); + G_TYPE_POINTER, + G_TYPE_BOOLEAN); /** * GUdevClient::redirecting: @@ -408,15 +408,15 @@ static gint gudev_devices_differ(gconstpointer a, gconstpointer b) static void notify_dev_state_change(GUdevClient *self, GList *old_list, GList *new_list, -const gchar *action) +gboolean add) { GList *dev; for (dev = g_list_first(old_list); dev != NULL; dev = g_list_next(dev)) { if (g_list_find_custom(new_list, dev->data, gudev_devices_differ) == NULL) { /* Found a device that changed its state */ -g_udev_device_print(dev->data, action); -g_signal_emit(self, signals[UEVENT_SIGNAL], 0, action, dev->data); +g_udev_device_print(dev->data, add ? "add" : "remove"); +g_signal_emit(self, signals[UEVENT_SIGNAL], 0, dev->data, add); } } } @@ -445,10 +445,10 @@ static void handle_dev_change(GUdevClient *self) 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, "remove"); +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, "add"); +notify_dev_state_change(self, now_devs,
[Spice-devel] [spice-gtk v2 06/13] win-usb-dev: remove unused procedure
Remove unused g_udev_device_get_sysfs_attr. Signed-off-by: Yuri Benditovich --- src/win-usb-dev.c | 18 -- src/win-usb-dev.h | 1 - 2 files changed, 19 deletions(-) diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c index 52e61a8..6273cbe 100644 --- a/src/win-usb-dev.c +++ b/src/win-usb-dev.c @@ -521,24 +521,6 @@ const gchar *g_udev_device_get_property(GUdevDevice *udev, const gchar *property return NULL; } -const gchar *g_udev_device_get_sysfs_attr(GUdevDevice *udev, const gchar *attr) -{ -GUdevDeviceInfo* udevinfo; - -g_return_val_if_fail(G_UDEV_DEVICE(udev), NULL); -g_return_val_if_fail(attr != NULL, NULL); - -udevinfo = udev->priv->udevinfo; -g_return_val_if_fail(udevinfo != NULL, NULL); - - -if (g_strcmp0(attr, "bDeviceClass") == 0) { -return udevinfo->sclass; -} -g_warn_if_reached(); -return NULL; -} - #ifdef DEBUG_GUDEV_DEVICE_LISTS static void g_udev_device_print_list(GList *l, const gchar *msg) { diff --git a/src/win-usb-dev.h b/src/win-usb-dev.h index f3c7466..b7b7eda 100644 --- a/src/win-usb-dev.h +++ b/src/win-usb-dev.h @@ -85,7 +85,6 @@ GList *g_udev_client_query_by_subsystem(GUdevClient *client, const gchar *subsys GType g_udev_device_get_type(void) G_GNUC_CONST; const gchar *g_udev_device_get_property(GUdevDevice *udev, const gchar *property); -const gchar *g_udev_device_get_sysfs_attr(GUdevDevice *udev, const gchar *attr); GQuark g_udev_client_error_quark(void); #define G_UDEV_CLIENT_ERROR g_udev_client_error_quark() -- 2.17.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v2 02/13] usb-redir: remove unused 'subsystem' parameter
Removing unused parameter for GUdevClient constructor. This code is used only in build for Windows. Signed-off-by: Yuri Benditovich --- src/usb-device-manager.c | 5 + src/win-usb-dev.c| 2 +- src/win-usb-dev.h| 2 +- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c index 6a36cfa..6435be8 100644 --- a/src/usb-device-manager.c +++ b/src/usb-device-manager.c @@ -283,9 +283,6 @@ static gboolean spice_usb_device_manager_initable_init(GInitable *initable, GList *list; GList *it; int rc; -#ifdef G_OS_WIN32 -const gchar *const subsystems[] = {"usb", NULL}; -#endif /* Initialize libusb */ rc = libusb_init(>context); @@ -305,7 +302,7 @@ static gboolean spice_usb_device_manager_initable_init(GInitable *initable, /* Start listening for usb devices plug / unplug */ #ifdef G_OS_WIN32 -priv->udev = g_udev_client_new(subsystems); +priv->udev = g_udev_client_new(); if (priv->udev == NULL) { g_warning("Error initializing GUdevClient"); return FALSE; diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c index 964719c..d3ff55a 100644 --- a/src/win-usb-dev.c +++ b/src/win-usb-dev.c @@ -104,7 +104,7 @@ GQuark g_udev_client_error_quark(void) return g_quark_from_static_string("win-gudev-client-error-quark"); } -GUdevClient *g_udev_client_new(const gchar* const *subsystems) +GUdevClient *g_udev_client_new(void) { if (singleton != NULL) return g_object_ref(singleton); diff --git a/src/win-usb-dev.h b/src/win-usb-dev.h index 7f40197..0f34a01 100644 --- a/src/win-usb-dev.h +++ b/src/win-usb-dev.h @@ -79,7 +79,7 @@ struct _GUdevClientClass }; GType g_udev_client_get_type(void) G_GNUC_CONST; -GUdevClient *g_udev_client_new(const gchar* const *subsystems); +GUdevClient *g_udev_client_new(void); GList *g_udev_client_query_by_subsystem(GUdevClient *client, const gchar *subsystem); GType g_udev_device_get_type(void) G_GNUC_CONST; -- 2.17.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v2 05/13] win-usb-dev: strict comparison of USB devices
If on device change the new device has the same bus:address as existing device, win-usb-dev does not emit signal of device change (for example, when due to some reason the 'redirecting' property is set for long time and during this time one of devices is changed). Make device comparison more strict: check not only bus:addr, but also vid:pid. Signed-off-by: Yuri Benditovich --- src/win-usb-dev.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c index b08bf02..52e61a8 100644 --- a/src/win-usb-dev.c +++ b/src/win-usb-dev.c @@ -385,20 +385,21 @@ static gboolean get_usb_dev_info(libusb_device *dev, GUdevDeviceInfo *udevinfo) return TRUE; } -/* Only bus:addr are compared */ +/* comparing bus:addr and vid:pid */ static gint gudev_devices_differ(gconstpointer a, gconstpointer b) { GUdevDeviceInfo *ai, *bi; -gboolean same_bus; -gboolean same_addr; +gboolean same_bus, same_addr, same_vid, same_pid; ai = G_UDEV_DEVICE(a)->priv->udevinfo; bi = G_UDEV_DEVICE(b)->priv->udevinfo; same_bus = (ai->bus == bi->bus); same_addr = (ai->addr == bi->addr); +same_vid = (ai->vid == bi->vid); +same_pid = (ai->pid == bi->pid); -return (same_bus && same_addr) ? 0 : -1; +return (same_bus && same_addr && same_vid && same_pid) ? 0 : -1; } static void notify_dev_state_change(GUdevClient *self, -- 2.17.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v2 10/13] win-usb-dev: maintain list of libusb devices
Change internal device list to maintain libusb devices instead of GUdevDevice objects. Create temporary GUdevDevice object only for indication to usb-dev-manager. Signed-off-by: Yuri Benditovich --- src/win-usb-dev.c | 80 ++- 1 file changed, 51 insertions(+), 29 deletions(-) diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c index b3b2ed8..0b87e75 100644 --- a/src/win-usb-dev.c +++ b/src/win-usb-dev.c @@ -95,7 +95,7 @@ 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(GUdevDevice *udev, const gchar *msg); +static void g_udev_device_print(libusb_device *dev, const gchar *msg); static gboolean g_udev_skip_search(libusb_device *dev); @@ -129,8 +129,6 @@ g_udev_client_list_devices(GUdevClient *self, GList **devs, gssize rc; libusb_device **lusb_list, **dev; GUdevClientPrivate *priv; -GUdevDeviceInfo *udevinfo; -GUdevDevice *udevice; ssize_t n; g_return_val_if_fail(G_UDEV_IS_CLIENT(self), -1); @@ -155,10 +153,7 @@ g_udev_client_list_devices(GUdevClient *self, GList **devs, if (g_udev_skip_search(*dev)) { continue; } -udevinfo = g_new0(GUdevDeviceInfo, 1); -get_usb_dev_info(*dev, udevinfo); -udevice = g_udev_device_new(udevinfo); -*devs = g_list_prepend(*devs, udevice); +*devs = g_list_prepend(*devs, libusb_ref_device(*dev)); n++; } libusb_free_device_list(lusb_list, 1); @@ -166,11 +161,17 @@ g_udev_client_list_devices(GUdevClient *self, GList **devs, return n; } +static void unreference_libusb_device(gpointer data) +{ +libusb_unref_device((libusb_device *)data); +} + static void g_udev_client_free_device_list(GList **devs) { g_return_if_fail(devs != NULL); if (*devs) { -g_list_free_full(*devs, g_object_unref); +/* avoiding casting of imported procedure to GDestroyNotify */ +g_list_free_full(*devs, unreference_libusb_device); *devs = NULL; } } @@ -246,9 +247,22 @@ static void g_udev_client_initable_iface_init(GInitableIface *iface) iface->init = g_udev_client_initable_init; } +static void g_udev_notify_device(GUdevClient *self, libusb_device *dev, gboolean add) +{ +GUdevDeviceInfo *udevinfo; +GUdevDevice *udevice; +udevinfo = g_new0(GUdevDeviceInfo, 1); +if (get_usb_dev_info(dev, udevinfo)) { +udevice = g_udev_device_new(udevinfo); +g_signal_emit(self, signals[UEVENT_SIGNAL], 0, udevice, add); +} else { +g_free(udevinfo); +} +} + static void report_one_device(gpointer data, gpointer self) { -g_signal_emit(self, signals[UEVENT_SIGNAL], 0, data, TRUE); +g_udev_notify_device(self, data, TRUE); } void g_udev_client_report_devices(GUdevClient *self) @@ -388,18 +402,20 @@ static gboolean get_usb_dev_info(libusb_device *dev, GUdevDeviceInfo *udevinfo) } /* comparing bus:addr and vid:pid */ -static gint gudev_devices_differ(gconstpointer a, gconstpointer b) +static gint compare_libusb_devices(gconstpointer a, gconstpointer b) { -GUdevDeviceInfo *ai, *bi; +libusb_device *a_dev = (libusb_device *)a; +libusb_device *b_dev = (libusb_device *)b; +struct libusb_device_descriptor a_desc, b_desc; gboolean same_bus, same_addr, same_vid, same_pid; -ai = G_UDEV_DEVICE(a)->priv->udevinfo; -bi = G_UDEV_DEVICE(b)->priv->udevinfo; +libusb_get_device_descriptor(a_dev, _desc); +libusb_get_device_descriptor(b_dev, _desc); -same_bus = (ai->bus == bi->bus); -same_addr = (ai->addr == bi->addr); -same_vid = (ai->vid == bi->vid); -same_pid = (ai->pid == bi->pid); +same_bus = (libusb_get_bus_number(a_dev) == libusb_get_bus_number(b_dev)); +same_addr = (libusb_get_device_address(a_dev) == libusb_get_device_address(b_dev)); +same_vid = (a_desc.idVendor == b_desc.idVendor); +same_pid = (a_desc.idProduct == b_desc.idProduct); return (same_bus && same_addr && same_vid && same_pid) ? 0 : -1; } @@ -412,10 +428,17 @@ static void notify_dev_state_change(GUdevClient *self, GList *dev; for (dev = g_list_first(old_list); dev != NULL; dev = g_list_next(dev)) { -if (g_list_find_custom(new_list, dev->data, gudev_devices_differ) == NULL) { -/* Found a device that changed its state */ +GList *found = g_list_find_custom(new_list, dev->data, compare_libusb_devices); +if (found == NULL) { g_udev_device_print(dev->data, add ? "add" : "remove"); -g_signal_emit(self, signals[UEVENT_SIGNAL], 0, dev->data, add); +g_udev_notify_device(self, dev->data, add); +} else if (add) { +/* keep old existing libusb_device in the new list, when + usb-dev-manager will maintain its own list of libusb_device, + these lists
[Spice-devel] [spice-gtk v2 00/13] use persistent libusb_device on Windows
This series unifies part of USB redirection code for Windows making it the same as for Linux by using persistent libusb_device also on Windows. Changes from v1: minor fixes per v1 review rebase to latest master branch Yuri Benditovich (13): usb-redir: replace USE_GUDEV with G_OS_WIN32 usb-redir: remove unused 'subsystem' parameter usb-redir: reuse libusb context under Windows usb-redir: do not add device if one with the same bus:addr exists win-usb-dev: strict comparison of USB devices win-usb-dev: remove unused procedure usb-redir: discard cold-plug device list under Windows usb-redir: change signal prototype of win-usb-dev win-usb-dev: do not allocate memory for hub devices win-usb-dev: maintain list of libusb devices win-usb-dev: report libusb_device via signal usb-redir: use persistent libusb_device under Windows also win-usb-dev: remove unused code src/spice-marshal.txt| 1 + src/usb-device-manager.c | 267 ++- src/win-usb-dev.c| 254 ++--- src/win-usb-dev.h| 35 + 4 files changed, 110 insertions(+), 447 deletions(-) -- 2.17.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v2 13/13] win-usb-dev: remove unused code
Remove unused code related to GUdevDevice. Signed-off-by: Yuri Benditovich --- src/win-usb-dev.c | 74 --- src/win-usb-dev.h | 27 - 2 files changed, 101 deletions(-) diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c index 09bd031..7c21482 100644 --- a/src/win-usb-dev.c +++ b/src/win-usb-dev.c @@ -49,31 +49,6 @@ 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)) - -typedef struct _GUdevDeviceInfo GUdevDeviceInfo; - -struct _GUdevDeviceInfo { -guint16 bus; -guint16 addr; -guint16 vid; -guint16 pid; -guint16 class; -gchar sclass[4]; -gchar sbus[4]; -gchar saddr[4]; -gchar svid[8]; -gchar spid[8]; -}; - -struct _GUdevDevicePrivate -{ -/* FixMe: move above fields to this structure and access them directly */ -GUdevDeviceInfo *udevinfo; -}; - -G_DEFINE_TYPE_WITH_PRIVATE(GUdevDevice, g_udev_device, G_TYPE_OBJECT) - - enum { UEVENT_SIGNAL, @@ -447,55 +422,6 @@ static LRESULT CALLBACK wnd_proc(HWND hwnd, UINT message, WPARAM wparam, LPARAM return DefWindowProc(hwnd, message, wparam, lparam); } -/*** GUdevDevice ***/ - -static void g_udev_device_finalize(GObject *object) -{ -GUdevDevice *device = G_UDEV_DEVICE(object); - -g_free(device->priv->udevinfo); -if (G_OBJECT_CLASS(g_udev_device_parent_class)->finalize != NULL) -(* G_OBJECT_CLASS(g_udev_device_parent_class)->finalize)(object); -} - -static void g_udev_device_class_init(GUdevDeviceClass *klass) -{ -GObjectClass *gobject_class = (GObjectClass *) klass; - -gobject_class->finalize = g_udev_device_finalize; -} - -static void g_udev_device_init(GUdevDevice *device) -{ -device->priv = g_udev_device_get_instance_private(device); -} - -const gchar *g_udev_device_get_property(GUdevDevice *udev, const gchar *property) -{ -GUdevDeviceInfo* udevinfo; - -g_return_val_if_fail(G_UDEV_DEVICE(udev), NULL); -g_return_val_if_fail(property != NULL, NULL); - -udevinfo = udev->priv->udevinfo; -g_return_val_if_fail(udevinfo != NULL, NULL); - -if (g_strcmp0(property, "BUSNUM") == 0) { -return udevinfo->sbus; -} else if (g_strcmp0(property, "DEVNUM") == 0) { -return udevinfo->saddr; -} else if (g_strcmp0(property, "DEVTYPE") == 0) { -return "usb_device"; -} else if (g_strcmp0(property, "VID") == 0) { -return udevinfo->svid; -} else if (g_strcmp0(property, "PID") == 0) { -return udevinfo->spid; -} - -g_warn_if_reached(); -return NULL; -} - #ifdef DEBUG_GUDEV_DEVICE_LISTS static void g_udev_device_print_list(GList *l, const gchar *msg) { diff --git a/src/win-usb-dev.h b/src/win-usb-dev.h index b960dfc..b1c0e27 100644 --- a/src/win-usb-dev.h +++ b/src/win-usb-dev.h @@ -26,30 +26,6 @@ G_BEGIN_DECLS -/* GUdevDevice */ - -#define G_UDEV_TYPE_DEVICE (g_udev_device_get_type()) -#define G_UDEV_DEVICE(o) (G_TYPE_CHECK_INSTANCE_CAST((o), G_UDEV_TYPE_DEVICE, GUdevDevice)) -#define G_UDEV_DEVICE_CLASS(k) (G_TYPE_CHECK_CLASS_CAST((k), G_UDEV_TYPE_DEVICE, GUdevDeviceClass)) -#define G_UDEV_IS_DEVICE(o)(G_TYPE_CHECK_INSTANCE_TYPE ((o), G_UDEV_TYPE_DEVICE)) -#define G_UDEV_IS_DEVICE_CLASS(k) (G_TYPE_CHECK_CLASS_TYPE((k), G_UDEV_TYPE_DEVICE)) -#define G_UDEV_DEVICE_GET_CLASS(o) (G_TYPE_INSTANCE_GET_CLASS((o), G_UDEV_TYPE_DEVICE, GUdevDeviceClass)) - -typedef struct _GUdevDevice GUdevDevice; -typedef struct _GUdevDeviceClass GUdevDeviceClass; -typedef struct _GUdevDevicePrivate GUdevDevicePrivate; - -struct _GUdevDevice -{ - GObject parent; - GUdevDevicePrivate *priv; -}; - -struct _GUdevDeviceClass -{ - GObjectClass parent_class; -}; - /* GUdevClient */ #define G_UDEV_TYPE_CLIENT (g_udev_client_get_type()) @@ -83,9 +59,6 @@ GUdevClient *g_udev_client_new(void); libusb_context *g_udev_client_get_context(GUdevClient *client); void g_udev_client_report_devices(GUdevClient *client); -GType g_udev_device_get_type(void) G_GNUC_CONST; -const gchar *g_udev_device_get_property(GUdevDevice *udev, const gchar *property); - GQuark g_udev_client_error_quark(void); #define G_UDEV_CLIENT_ERROR g_udev_client_error_quark() -- 2.17.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v2 01/13] usb-redir: replace USE_GUDEV with G_OS_WIN32
Replacing USE_GUDEV with G_OS_WIN32 anywhere. GUDEV simulation is used only in Windows build. Signed-off-by: Yuri Benditovich --- src/usb-device-manager.c | 26 +++--- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c index 2578350..6a36cfa 100644 --- a/src/usb-device-manager.c +++ b/src/usb-device-manager.c @@ -29,11 +29,7 @@ #ifdef G_OS_WIN32 #include "usbdk_api.h" -#endif - -#if defined(G_OS_WIN32) #include "win-usb-dev.h" -#define USE_GUDEV /* win-usb-dev.h provides a fake gudev interface */ #endif #include "channel-usbredir-priv.h" @@ -106,7 +102,7 @@ struct _SpiceUsbDeviceManagerPrivate { struct usbredirfilter_rule *redirect_on_connect_rules; int auto_conn_filter_rules_count; int redirect_on_connect_rules_count; -#ifdef USE_GUDEV +#ifdef G_OS_WIN32 GUdevClient *udev; libusb_device **coldplug_list; /* Avoid needless reprobing during init */ #else @@ -156,7 +152,7 @@ static void channel_destroy(SpiceSession *session, SpiceChannel *channel, gpointer user_data); static void channel_event(SpiceChannel *channel, SpiceChannelEvent event, gpointer user_data); -#ifdef USE_GUDEV +#ifdef G_OS_WIN32 static void spice_usb_device_manager_uevent_cb(GUdevClient *client, const gchar *action, GUdevDevice *udevice, @@ -210,7 +206,7 @@ G_DEFINE_BOXED_TYPE(SpiceUsbDevice, spice_usb_device, static void _set_redirecting(SpiceUsbDeviceManager *self, gboolean is_redirecting) { -#ifdef USE_GUDEV +#ifdef G_OS_WIN32 g_object_set(self->priv->udev, "redirecting", is_redirecting, NULL); #else self->priv->redirecting = is_redirecting; @@ -235,7 +231,7 @@ gboolean spice_usb_device_manager_is_redirecting(SpiceUsbDeviceManager *self) { #ifdef USE_USBREDIR -#ifdef USE_GUDEV +#ifdef G_OS_WIN32 gboolean redirecting; g_object_get(self->priv->udev, "redirecting", , NULL); return redirecting; @@ -287,7 +283,7 @@ static gboolean spice_usb_device_manager_initable_init(GInitable *initable, GList *list; GList *it; int rc; -#ifdef USE_GUDEV +#ifdef G_OS_WIN32 const gchar *const subsystems[] = {"usb", NULL}; #endif @@ -308,7 +304,7 @@ static gboolean spice_usb_device_manager_initable_init(GInitable *initable, #endif /* Start listening for usb devices plug / unplug */ -#ifdef USE_GUDEV +#ifdef G_OS_WIN32 priv->udev = g_udev_client_new(subsystems); if (priv->udev == NULL) { g_warning("Error initializing GUdevClient"); @@ -366,7 +362,7 @@ static void spice_usb_device_manager_dispose(GObject *gobject) SpiceUsbDeviceManager *self = SPICE_USB_DEVICE_MANAGER(gobject); SpiceUsbDeviceManagerPrivate *priv = self->priv; -#ifndef USE_GUDEV +#ifndef G_OS_WIN32 if (priv->hp_handle) { spice_usb_device_manager_stop_event_listening(self); if (g_atomic_int_get(>event_thread_run)) { @@ -405,7 +401,7 @@ static void spice_usb_device_manager_finalize(GObject *gobject) g_ptr_array_unref(priv->devices); #ifdef USE_USBREDIR -#ifdef USE_GUDEV +#ifdef G_OS_WIN32 g_clear_object(>udev); #endif g_return_if_fail(priv->event_thread == NULL); @@ -737,7 +733,7 @@ static void spice_usb_device_manager_class_init(SpiceUsbDeviceManagerClass *klas /* -- */ /* gudev / libusb Helper functions*/ -#ifdef USE_GUDEV +#ifdef G_OS_WIN32 static gboolean spice_usb_device_manager_get_udev_bus_n_address( SpiceUsbDeviceManager *manager, GUdevDevice *udev, int *bus, int *address) @@ -927,7 +923,7 @@ spice_usb_device_manager_device_match(SpiceUsbDeviceManager *self, SpiceUsbDevic spice_usb_device_get_devaddr(device) == address); } -#ifdef USE_GUDEV +#ifdef G_OS_WIN32 static gboolean spice_usb_device_manager_libdev_match(SpiceUsbDeviceManager *self, libusb_device *libdev, const int bus, const int address) @@ -1027,7 +1023,7 @@ static void spice_usb_device_manager_remove_dev(SpiceUsbDeviceManager *self, spice_usb_device_unref(device); } -#ifdef USE_GUDEV +#ifdef G_OS_WIN32 static void spice_usb_device_manager_add_udev(SpiceUsbDeviceManager *self, GUdevDevice*udev) { -- 2.17.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v2 04/13] usb-redir: do not add device if one with the same bus:addr exists
In initial device enumeration hotplug notification can be called twice with the same libusb device. For details, see http://libusb.sourceforge.net/api-1.0/group__libusb__hotplug.html#ga00e0c69ddf1fb1b6774dc918192e8dc7 Filter out devices that already present in the list. Remove indentical call in spice_usb_device_manager_add_udev, which add devices under Windows. Signed-off-by: Yuri Benditovich --- src/usb-device-manager.c | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c index debba4d..a9f19bf 100644 --- a/src/usb-device-manager.c +++ b/src/usb-device-manager.c @@ -962,6 +962,17 @@ static void spice_usb_device_manager_add_dev(SpiceUsbDeviceManager *self, if (desc.bDeviceClass == LIBUSB_CLASS_HUB) return; +if (spice_usb_device_manager_find_device(self, +libusb_get_bus_number(libdev), +libusb_get_device_address(libdev))) { +SPICE_DEBUG("device not added %d:%d %04x:%04x", +libusb_get_bus_number(libdev), +libusb_get_device_address(libdev), +desc.idVendor, +desc.idProduct); +return; +} + device = (SpiceUsbDevice*)spice_usb_device_new(libdev); if (!device) return; @@ -1025,7 +1036,6 @@ static void spice_usb_device_manager_add_udev(SpiceUsbDeviceManager *self, { SpiceUsbDeviceManagerPrivate *priv = self->priv; libusb_device *libdev = NULL, **dev_list = NULL; -SpiceUsbDevice *device; const gchar *devtype; int i, bus, address; @@ -1039,16 +1049,6 @@ static void spice_usb_device_manager_add_udev(SpiceUsbDeviceManager *self, return; } -device = spice_usb_device_manager_find_device(self, bus, address); -if (device) { -SPICE_DEBUG("USB device 0x%04x:0x%04x at %d.%d already exists, ignored", -spice_usb_device_get_vid(device), -spice_usb_device_get_pid(device), -spice_usb_device_get_busnum(device), -spice_usb_device_get_devaddr(device)); -return; -} - if (priv->coldplug_list) dev_list = priv->coldplug_list; else -- 2.17.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [spice-gtk v2 11/13] win-usb-dev: report libusb_device via signal
Change the signal to report libusb_device instead of GUdevDevice object. This simplifies the callback procedure in usb-dev-manager and avoids additional reenumeration immediately after reenumeration that already done by win-usb-dev. Signed-off-by: Yuri Benditovich --- src/usb-device-manager.c | 81 +++- src/win-usb-dev.c| 55 +-- src/win-usb-dev.h| 2 +- 3 files changed, 9 insertions(+), 129 deletions(-) diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c index c99d359..a163501 100644 --- a/src/usb-device-manager.c +++ b/src/usb-device-manager.c @@ -153,11 +153,9 @@ 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, - GUdevDevice *udevice, + libusb_device *udevice, gboolean add, gpointer user_data); -static void spice_usb_device_manager_add_udev(SpiceUsbDeviceManager *self, - GUdevDevice*udev); #else static int spice_usb_device_manager_hotplug_cb(libusb_context *ctx, libusb_device*device, @@ -720,28 +718,6 @@ static void spice_usb_device_manager_class_init(SpiceUsbDeviceManagerClass *klas /* -- */ /* gudev / libusb Helper functions*/ -#ifdef G_OS_WIN32 -static gboolean spice_usb_device_manager_get_udev_bus_n_address( -SpiceUsbDeviceManager *manager, GUdevDevice *udev, -int *bus, int *address) -{ -const gchar *bus_str, *address_str; - -*bus = *address = 0; - - /* Linux or UsbDk backend on Windows*/ -bus_str = g_udev_device_get_property(udev, "BUSNUM"); -address_str = g_udev_device_get_property(udev, "DEVNUM"); - -if (bus_str) -*bus = atoi(bus_str); -if (address_str) -*address = atoi(address_str); - -return *bus && *address; -} -#endif - static gboolean spice_usb_device_manager_get_device_descriptor( libusb_device *libdev, struct libusb_device_descriptor *desc) @@ -1022,64 +998,19 @@ static void spice_usb_device_manager_remove_dev(SpiceUsbDeviceManager *self, } #ifdef G_OS_WIN32 -static void spice_usb_device_manager_add_udev(SpiceUsbDeviceManager *self, - GUdevDevice*udev) -{ -SpiceUsbDeviceManagerPrivate *priv = self->priv; -libusb_device *libdev = NULL, **dev_list = NULL; -const gchar *devtype; -int i, bus, address; - -devtype = g_udev_device_get_property(udev, "DEVTYPE"); -/* Check if this is a usb device (and not an interface) */ -if (!devtype || strcmp(devtype, "usb_device")) -return; - -if (!spice_usb_device_manager_get_udev_bus_n_address(self, udev, , )) { -g_warning("USB device without bus number or device address"); -return; -} - -libusb_get_device_list(priv->context, _list); - -for (i = 0; dev_list && dev_list[i]; i++) { -if (spice_usb_device_manager_libdev_match(self, dev_list[i], bus, address)) { -libdev = dev_list[i]; -break; -} -} - -if (libdev) -spice_usb_device_manager_add_dev(self, libdev); -else -g_warning("Could not find USB device to add " DEV_ID_FMT, - (guint) bus, (guint) address); - -libusb_free_device_list(dev_list, 1); -} - -static void spice_usb_device_manager_remove_udev(SpiceUsbDeviceManager *self, - GUdevDevice*udev) -{ -int bus, address; - -if (!spice_usb_device_manager_get_udev_bus_n_address(self, udev, , )) -return; - -spice_usb_device_manager_remove_dev(self, bus, address); -} - static void spice_usb_device_manager_uevent_cb(GUdevClient *client, - GUdevDevice *udevice, + libusb_device *dev, gboolean add, gpointer user_data) { SpiceUsbDeviceManager *self = SPICE_USB_DEVICE_MANAGER(user_data); if (add) -spice_usb_device_manager_add_udev(self, udevice); +spice_usb_device_manager_add_dev(self, dev); else -spice_usb_device_manager_remove_udev(self, udevice); +spice_usb_device_manager_remove_dev(self, +libusb_get_bus_number(dev), +libusb_get_device_address(dev)); } #else struct
Re: [Spice-devel] [spice-gtk v3] smartcard: Warn if multiple readers are detected
On Fri, Feb 22, 2019 at 11:40 AM Christophe Fergeau wrote: > > spice-server does not deal properly with multiple smartcard readers, > only the first one will be working. Add a warning when this happens to > make it easier to diagnose such issues. > > Signed-off-by: Christophe Fergeau ack > --- > src/smartcard-manager.c | 19 +++ > 1 file changed, 19 insertions(+) > > diff --git a/src/smartcard-manager.c b/src/smartcard-manager.c > index ceecfdc7..35bb2757 100644 > --- a/src/smartcard-manager.c > +++ b/src/smartcard-manager.c > @@ -389,6 +389,24 @@ typedef struct { > GError *err; > } SmartcardManagerInitArgs; > > + > +static void smartcard_reader_free(gpointer data) > +{ > +g_boxed_free(SPICE_TYPE_SMARTCARD_READER, data); > +} > + > +/* spice-server only supports one smartcard reader being in use */ > +static void smartcard_check_reader_count(void) > +{ > +GList *readers; > + > +readers = > spice_smartcard_manager_get_readers(spice_smartcard_manager_get()); > +if (g_list_length(readers) > 1) { > +g_warning("Multiple smartcard readers are plugged in, only the first > one will be shared with the VM"); > +} > +g_list_free_full(readers, smartcard_reader_free); > +} > + > static gboolean smartcard_manager_init(SmartcardManagerInitArgs *args) > { > gchar *emul_args = NULL; > @@ -442,6 +460,7 @@ init: > "Failed to initialize smartcard"); > goto end; > } > +smartcard_check_reader_count(); > > retval = TRUE; > > -- > 2.20.1 > > ___ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/spice-devel -- Marc-André Lureau ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk v3] smartcard: Warn if multiple readers are detected
Ping? Or should I just go with one of the earlier versions? On Fri, Feb 22, 2019 at 11:40:32AM +0100, Christophe Fergeau wrote: > spice-server does not deal properly with multiple smartcard readers, > only the first one will be working. Add a warning when this happens to > make it easier to diagnose such issues. > > Signed-off-by: Christophe Fergeau > --- > src/smartcard-manager.c | 19 +++ > 1 file changed, 19 insertions(+) > > diff --git a/src/smartcard-manager.c b/src/smartcard-manager.c > index ceecfdc7..35bb2757 100644 > --- a/src/smartcard-manager.c > +++ b/src/smartcard-manager.c > @@ -389,6 +389,24 @@ typedef struct { > GError *err; > } SmartcardManagerInitArgs; > > + > +static void smartcard_reader_free(gpointer data) > +{ > +g_boxed_free(SPICE_TYPE_SMARTCARD_READER, data); > +} > + > +/* spice-server only supports one smartcard reader being in use */ > +static void smartcard_check_reader_count(void) > +{ > +GList *readers; > + > +readers = > spice_smartcard_manager_get_readers(spice_smartcard_manager_get()); > +if (g_list_length(readers) > 1) { > +g_warning("Multiple smartcard readers are plugged in, only the first > one will be shared with the VM"); > +} > +g_list_free_full(readers, smartcard_reader_free); > +} > + > static gboolean smartcard_manager_init(SmartcardManagerInitArgs *args) > { > gchar *emul_args = NULL; > @@ -442,6 +460,7 @@ init: > "Failed to initialize smartcard"); > goto end; > } > +smartcard_check_reader_count(); > > retval = TRUE; > > -- > 2.20.1 > > ___ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/spice-devel signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk v2 2/2] win-usb-dev: remove ifdef for libusb on 1.0.13
On Fri, Feb 22, 2019 at 05:42:22PM +, Victor Toso wrote: > From: Victor Toso > > We already require libusb 1.0.16 or above since 8269a5be62f4ce1 > (build-sys: drop support for libusb < 1.0.16) > > Signed-off-by: Victor Toso > --- > src/win-usb-dev.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c > index 327976d..b614af0 100644 > --- a/src/win-usb-dev.c > +++ b/src/win-usb-dev.c > @@ -570,9 +570,7 @@ static gboolean g_udev_skip_search(GUdevDevice *udev) > g_return_val_if_fail(udevinfo != NULL, FALSE); > > skip = ((udevinfo->addr == 0xff) || /* root hub (HCD) */ > -#if defined(LIBUSBX_API_VERSION) && (LIBUSBX_API_VERSION >= 0x01FF) > -(udevinfo->addr == 1) || /* root hub addr for libusbx >= 1.0.13 > */ > -#endif > +(udevinfo->addr == 1) || /* root hub addr since libusbx 1.0.13 */ I'd drop "since libusbx 1.0.13" from the comment. Apart from that, series looks good to me. Christophe signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk 10/13] win-usb-dev: maintain list of libusb devices
On Tue, Mar 12, 2019 at 7:22 PM Christophe Fergeau wrote:>> On Sun, Mar 10, 2019 at 04:46:09PM +0200, Yuri Benditovich wrote: > > Change internal device list to maintain libusb devices > > instead of GUdevDevice objects. Create temporary > > GUdevDevice object only for indication to usb-dev-manager. > > > > Signed-off-by: Yuri Benditovich > > --- > > src/win-usb-dev.c | 80 ++- > > 1 file changed, 51 insertions(+), 29 deletions(-) > > > > diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c > > index 1ab704d..5d878ea 100644 > > --- a/src/win-usb-dev.c > > +++ b/src/win-usb-dev.c > > @@ -95,7 +95,7 @@ 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(GUdevDevice *udev, const gchar *msg); > > +static void g_udev_device_print(libusb_device *dev, const gchar *msg); > > > > static gboolean g_udev_skip_search(libusb_device *dev); > > > > @@ -129,8 +129,6 @@ g_udev_client_list_devices(GUdevClient *self, GList > > **devs, > > gssize rc; > > libusb_device **lusb_list, **dev; > > GUdevClientPrivate *priv; > > -GUdevDeviceInfo *udevinfo; > > -GUdevDevice *udevice; > > ssize_t n; > > > > g_return_val_if_fail(G_UDEV_IS_CLIENT(self), -1); > > @@ -155,10 +153,7 @@ g_udev_client_list_devices(GUdevClient *self, GList > > **devs, > > if (g_udev_skip_search(*dev)) { > > continue; > > } > > -udevinfo = g_new0(GUdevDeviceInfo, 1); > > -get_usb_dev_info(*dev, udevinfo); > > -udevice = g_udev_device_new(udevinfo); > > -*devs = g_list_prepend(*devs, udevice); > > +*devs = g_list_prepend(*devs, libusb_ref_device(*dev)); > > n++; > > } > > libusb_free_device_list(lusb_list, 1); > > @@ -166,11 +161,17 @@ g_udev_client_list_devices(GUdevClient *self, GList > > **devs, > > return n; > > } > > > > +static void unreference_libusb_device(gpointer data) > > +{ > > +libusb_unref_device((libusb_device *)data); > > +} > > + > > static void g_udev_client_free_device_list(GList **devs) > > { > > g_return_if_fail(devs != NULL); > > if (*devs) { > > -g_list_free_full(*devs, g_object_unref); > > +/* avoiding casting of imported procedure to GDestroyNotify */ > > +g_list_free_full(*devs, unreference_libusb_device); > > Since all unreference_libusb_device is doing is blindly casting a void * > to libusb_device *, I'd just use a cast to GDestroyNotify here rather > than introduce an intermediate method. If you prefer to keep that > intermediate helper, please remove the commit which I don't think is > needed. > > > *devs = NULL; > > } > > } > > @@ -246,9 +247,22 @@ static void > > g_udev_client_initable_iface_init(GInitableIface *iface) > > iface->init = g_udev_client_initable_init; > > } > > > > +static void g_udev_notify_device(GUdevClient *self, libusb_device *dev, > > int add) > > +{ > > +GUdevDeviceInfo *udevinfo; > > +GUdevDevice *udevice; > > +udevinfo = g_new0(GUdevDeviceInfo, 1); > > +if (get_usb_dev_info(dev, udevinfo)) { > > +udevice = g_udev_device_new(udevinfo); > > +g_signal_emit(self, signals[UEVENT_SIGNAL], 0, udevice, add); > > +} else { > > +g_free(udevinfo); > > +} > > +} > > + > > static void report_one_device(gpointer data, gpointer self) > > { > > -g_signal_emit(self, signals[UEVENT_SIGNAL], 0, data, TRUE); > > +g_udev_notify_device(self, data, TRUE); > > } > > > > void g_udev_client_report_devices(GUdevClient *self) > > @@ -388,18 +402,20 @@ static gboolean get_usb_dev_info(libusb_device *dev, > > GUdevDeviceInfo *udevinfo) > > } > > > > /* comparing bus:addr and vid:pid */ > > -static gint gudev_devices_differ(gconstpointer a, gconstpointer b) > > +static gint compare_libusb_devices(gconstpointer a, gconstpointer b) > > { > > -GUdevDeviceInfo *ai, *bi; > > +libusb_device *a_dev = (libusb_device *)a; > > +libusb_device *b_dev = (libusb_device *)b; > > +struct libusb_device_descriptor a_desc, b_desc; > > gboolean same_bus, same_addr, same_vid, same_pid; > > > > -ai = G_UDEV_DEVICE(a)->priv->udevinfo; > > -bi = G_UDEV_DEVICE(b)->priv->udevinfo; > > +libusb_get_device_descriptor(a_dev, _desc); > > +libusb_get_device_descriptor(b_dev, _desc); > > > > -same_bus = (ai->bus == bi->bus); > > -same_addr = (ai->addr == bi->addr); > > -same_vid = (ai->vid == bi->vid); > > -same_pid = (ai->pid == bi->pid); > > +same_bus = libusb_get_bus_number(a_dev) == > > libusb_get_bus_number(b_dev); > > +same_addr = libusb_get_device_address(a_dev) == > > libusb_get_device_address(b_dev); > > +same_vid = (a_desc.idVendor == b_desc.idVendor); > > +same_pid = (a_desc.idProduct == b_desc.idProduct); > > > > return (same_bus &&
Re: [Spice-devel] [PATCH spice-common v3 0/5] Generate C declarations automatically
On Mon, 2019-03-18 at 09:53 -0400, Frediano Ziglio wrote: > > On Mon, Mar 11, 2019 at 12:42:10PM -0400, Frediano Ziglio wrote: > > > > > > > > Series looks good to me, > > > > > > > > Reviewed-by: Christophe Fergeau > > > > > > > > > > Why not ack? Not good enough? Not tested? Missing something? > > > > libvirt/qemu use of Reviewed-by/Acked-by confuses me, and to me > > they are > > more or less equivalent. > > > > Christophe > > > > As long as we are coherent is fine. But to me looks like some > (Jonathon?) > use Reviewed-by to mark as "the patch was fully reviewed but some > thinks > need updates". > > Frediano I usually use Reviewed-by to indicate that I looked at it, but am not necessarily comfortable Acking it yet. But you're right that there's not really any agreement on what this means. Would be good to be consistent. Jonathon ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-server 1/2] docs: Fix typo
On Mon, Mar 18, 2019 at 09:44:59AM -0400, Frediano Ziglio wrote: > > > -Lots of code run in a single thread. > > > +Lots of code runs in a single thread. > > > > "Lots of" is plural, isn't it? Maybe "A lot of code runs .." > > > > Christophe > > > > You are right. Which one is better? FWIW, actually 'lots of' and 'a lot of' are equivalent, and the verb will be plural or singular depending on if the noun is countable (and will be plural) or is uncountable (and will be singular). So "Lots of people run" but "Lots of code runs" because code is uncountable. -- Douglas Paul ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-common v3 0/5] Generate C declarations automatically
> On Mon, Mar 11, 2019 at 12:42:10PM -0400, Frediano Ziglio wrote: > > > > > > Series looks good to me, > > > > > > Reviewed-by: Christophe Fergeau > > > > > > > Why not ack? Not good enough? Not tested? Missing something? > > libvirt/qemu use of Reviewed-by/Acked-by confuses me, and to me they are > more or less equivalent. > > Christophe > As long as we are coherent is fine. But to me looks like some (Jonathon?) use Reviewed-by to mark as "the patch was fully reviewed but some thinks need updates". Frediano ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-server 1/2] docs: Fix typo
> > On Mon, Mar 11, 2019 at 02:03:32PM +, Frediano Ziglio wrote: > > Signed-off-by: Frediano Ziglio > > --- > > docs/spice_threading_model.txt | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/docs/spice_threading_model.txt > > b/docs/spice_threading_model.txt > > index df4922168..9351141c8 100644 > > --- a/docs/spice_threading_model.txt > > +++ b/docs/spice_threading_model.txt > > @@ -1,7 +1,7 @@ > > How does SPICE handle threading > > --- > > > > -Lots of code run in a single thread. > > +Lots of code runs in a single thread. > > "Lots of" is plural, isn't it? Maybe "A lot of code runs .." > > Christophe > You are right. Which one is better? Frediano ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-protocol] protocol: Generate enums.h again to remove old protocol definitions
> > This is breaking spice-protocol API even if this should not impact > current code. Do we want to advertise this somehow? Switch to > spice-protocol 0.14.x? > Fine for me both 0.14.0 or 0.12.16 (the future, current HEAD version) > On Fri, Feb 22, 2019 at 12:16:52PM +, Frediano Ziglio wrote: > > spice.proto was updated to remove really old definitions, > > update enums.h accordingly. > > > > Signed-off-by: Frediano Ziglio > > --- > > spice/enums.h | 57 --- > > 1 file changed, 57 deletions(-) > > > > diff --git a/spice/enums.h b/spice/enums.h > > index 34e84c3..172cc4d 100644 > > --- a/spice/enums.h > > +++ b/spice/enums.h > > @@ -119,21 +119,6 @@ typedef enum SpiceMouseMode { > > SPICE_MOUSE_MODE_MASK = 0x3 > > } SpiceMouseMode; > > > > -typedef enum SpicePubkeyType { > > -SPICE_PUBKEY_TYPE_INVALID, > > -SPICE_PUBKEY_TYPE_RSA, > > -SPICE_PUBKEY_TYPE_RSA2, > > -SPICE_PUBKEY_TYPE_DSA, > > -SPICE_PUBKEY_TYPE_DSA1, > > -SPICE_PUBKEY_TYPE_DSA2, > > -SPICE_PUBKEY_TYPE_DSA3, > > -SPICE_PUBKEY_TYPE_DSA4, > > -SPICE_PUBKEY_TYPE_DH, > > -SPICE_PUBKEY_TYPE_EC, > > - > > -SPICE_PUBKEY_TYPE_ENUM_END > > -} SpicePubkeyType; > > - > > typedef enum SpiceDataCompressionType { > > SPICE_DATA_COMPRESSION_TYPE_NONE, > > SPICE_DATA_COMPRESSION_TYPE_LZ4, > > @@ -397,21 +382,6 @@ typedef enum SpiceAudioFmt { > > SPICE_AUDIO_FMT_ENUM_END > > } SpiceAudioFmt; > > > > -typedef enum SpiceTunnelServiceType { > > -SPICE_TUNNEL_SERVICE_TYPE_INVALID, > > -SPICE_TUNNEL_SERVICE_TYPE_GENERIC, > > -SPICE_TUNNEL_SERVICE_TYPE_IPP, > > - > > -SPICE_TUNNEL_SERVICE_TYPE_ENUM_END > > -} SpiceTunnelServiceType; > > - > > -typedef enum SpiceTunnelIpType { > > -SPICE_TUNNEL_IP_TYPE_INVALID, > > -SPICE_TUNNEL_IP_TYPE_IPv4, > > - > > -SPICE_TUNNEL_IP_TYPE_ENUM_END > > -} SpiceTunnelIpType; > > - > > typedef enum SpiceVscMessageType { > > SPICE_VSC_MESSAGE_TYPE_Init = 1, > > SPICE_VSC_MESSAGE_TYPE_Error, > > @@ -613,33 +583,6 @@ enum { > > SPICE_MSGC_END_RECORD > > }; > > > > -enum { > > -SPICE_MSG_TUNNEL_INIT = 101, > > -SPICE_MSG_TUNNEL_SERVICE_IP_MAP, > > -SPICE_MSG_TUNNEL_SOCKET_OPEN, > > -SPICE_MSG_TUNNEL_SOCKET_FIN, > > -SPICE_MSG_TUNNEL_SOCKET_CLOSE, > > -SPICE_MSG_TUNNEL_SOCKET_DATA, > > -SPICE_MSG_TUNNEL_SOCKET_CLOSED_ACK, > > -SPICE_MSG_TUNNEL_SOCKET_TOKEN, > > - > > -SPICE_MSG_END_TUNNEL > > -}; > > - > > -enum { > > -SPICE_MSGC_TUNNEL_SERVICE_ADD = 101, > > -SPICE_MSGC_TUNNEL_SERVICE_REMOVE, > > -SPICE_MSGC_TUNNEL_SOCKET_OPEN_ACK, > > -SPICE_MSGC_TUNNEL_SOCKET_OPEN_NACK, > > -SPICE_MSGC_TUNNEL_SOCKET_FIN, > > -SPICE_MSGC_TUNNEL_SOCKET_CLOSED, > > -SPICE_MSGC_TUNNEL_SOCKET_CLOSED_ACK, > > -SPICE_MSGC_TUNNEL_SOCKET_DATA, > > -SPICE_MSGC_TUNNEL_SOCKET_TOKEN, > > - > > -SPICE_MSGC_END_TUNNEL > > -}; > > - > > enum { > > SPICE_MSG_SMARTCARD_DATA = 101, > > Frediano ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-common v3 0/5] Generate C declarations automatically
On Mon, Mar 11, 2019 at 12:42:10PM -0400, Frediano Ziglio wrote: > > > > Series looks good to me, > > > > Reviewed-by: Christophe Fergeau > > > > Why not ack? Not good enough? Not tested? Missing something? libvirt/qemu use of Reviewed-by/Acked-by confuses me, and to me they are more or less equivalent. Christophe signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-protocol] protocol: Generate enums.h again to remove old protocol definitions
This is breaking spice-protocol API even if this should not impact current code. Do we want to advertise this somehow? Switch to spice-protocol 0.14.x? On Fri, Feb 22, 2019 at 12:16:52PM +, Frediano Ziglio wrote: > spice.proto was updated to remove really old definitions, > update enums.h accordingly. > > Signed-off-by: Frediano Ziglio > --- > spice/enums.h | 57 --- > 1 file changed, 57 deletions(-) > > diff --git a/spice/enums.h b/spice/enums.h > index 34e84c3..172cc4d 100644 > --- a/spice/enums.h > +++ b/spice/enums.h > @@ -119,21 +119,6 @@ typedef enum SpiceMouseMode { > SPICE_MOUSE_MODE_MASK = 0x3 > } SpiceMouseMode; > > -typedef enum SpicePubkeyType { > -SPICE_PUBKEY_TYPE_INVALID, > -SPICE_PUBKEY_TYPE_RSA, > -SPICE_PUBKEY_TYPE_RSA2, > -SPICE_PUBKEY_TYPE_DSA, > -SPICE_PUBKEY_TYPE_DSA1, > -SPICE_PUBKEY_TYPE_DSA2, > -SPICE_PUBKEY_TYPE_DSA3, > -SPICE_PUBKEY_TYPE_DSA4, > -SPICE_PUBKEY_TYPE_DH, > -SPICE_PUBKEY_TYPE_EC, > - > -SPICE_PUBKEY_TYPE_ENUM_END > -} SpicePubkeyType; > - > typedef enum SpiceDataCompressionType { > SPICE_DATA_COMPRESSION_TYPE_NONE, > SPICE_DATA_COMPRESSION_TYPE_LZ4, > @@ -397,21 +382,6 @@ typedef enum SpiceAudioFmt { > SPICE_AUDIO_FMT_ENUM_END > } SpiceAudioFmt; > > -typedef enum SpiceTunnelServiceType { > -SPICE_TUNNEL_SERVICE_TYPE_INVALID, > -SPICE_TUNNEL_SERVICE_TYPE_GENERIC, > -SPICE_TUNNEL_SERVICE_TYPE_IPP, > - > -SPICE_TUNNEL_SERVICE_TYPE_ENUM_END > -} SpiceTunnelServiceType; > - > -typedef enum SpiceTunnelIpType { > -SPICE_TUNNEL_IP_TYPE_INVALID, > -SPICE_TUNNEL_IP_TYPE_IPv4, > - > -SPICE_TUNNEL_IP_TYPE_ENUM_END > -} SpiceTunnelIpType; > - > typedef enum SpiceVscMessageType { > SPICE_VSC_MESSAGE_TYPE_Init = 1, > SPICE_VSC_MESSAGE_TYPE_Error, > @@ -613,33 +583,6 @@ enum { > SPICE_MSGC_END_RECORD > }; > > -enum { > -SPICE_MSG_TUNNEL_INIT = 101, > -SPICE_MSG_TUNNEL_SERVICE_IP_MAP, > -SPICE_MSG_TUNNEL_SOCKET_OPEN, > -SPICE_MSG_TUNNEL_SOCKET_FIN, > -SPICE_MSG_TUNNEL_SOCKET_CLOSE, > -SPICE_MSG_TUNNEL_SOCKET_DATA, > -SPICE_MSG_TUNNEL_SOCKET_CLOSED_ACK, > -SPICE_MSG_TUNNEL_SOCKET_TOKEN, > - > -SPICE_MSG_END_TUNNEL > -}; > - > -enum { > -SPICE_MSGC_TUNNEL_SERVICE_ADD = 101, > -SPICE_MSGC_TUNNEL_SERVICE_REMOVE, > -SPICE_MSGC_TUNNEL_SOCKET_OPEN_ACK, > -SPICE_MSGC_TUNNEL_SOCKET_OPEN_NACK, > -SPICE_MSGC_TUNNEL_SOCKET_FIN, > -SPICE_MSGC_TUNNEL_SOCKET_CLOSED, > -SPICE_MSGC_TUNNEL_SOCKET_CLOSED_ACK, > -SPICE_MSGC_TUNNEL_SOCKET_DATA, > -SPICE_MSGC_TUNNEL_SOCKET_TOKEN, > - > -SPICE_MSGC_END_TUNNEL > -}; > - > enum { > SPICE_MSG_SMARTCARD_DATA = 101, > > -- > 2.20.1 > > ___ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/spice-devel signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-server 2/2] docs: Add some notes on event scheduling and threading
On Mon, Mar 11, 2019 at 02:03:33PM +, Frediano Ziglio wrote: > Signed-off-by: Frediano Ziglio > --- > docs/spice_threading_model.txt | 8 > 1 file changed, 8 insertions(+) > > diff --git a/docs/spice_threading_model.txt b/docs/spice_threading_model.txt > index 9351141c8..25a3a030c 100644 > --- a/docs/spice_threading_model.txt > +++ b/docs/spice_threading_model.txt > @@ -39,6 +39,14 @@ connect, disconnect and migrate. Connect and migrate are > asynchronous (the job > is done while the current thread is doing something else) while disconnect is > synchronous (the main thread will wait for termination). > > +One aspect to take into consideration is the event scheduling. SPICE uses > some > +`SpiceCoreInterface` to handle events. As the events will be handled from a > +thread based on the core interface you have to use the correct core. Each > +channel has an associated core interface which can be retrieved using > +`red_channel_get_core_interface`. There's also a main core interface you can > get > +using `reds_get_core_interface`. `reds_core_timer_*` and `reds_core_watch_*` > +functions use the main core interface. Do we need a few words as to when to use the main core interface? Apart from this, looks good to me. Christophe signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-server 1/2] docs: Fix typo
On Mon, Mar 11, 2019 at 02:03:32PM +, Frediano Ziglio wrote: > Signed-off-by: Frediano Ziglio > --- > docs/spice_threading_model.txt | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/docs/spice_threading_model.txt b/docs/spice_threading_model.txt > index df4922168..9351141c8 100644 > --- a/docs/spice_threading_model.txt > +++ b/docs/spice_threading_model.txt > @@ -1,7 +1,7 @@ > How does SPICE handle threading > --- > > -Lots of code run in a single thread. > +Lots of code runs in a single thread. "Lots of" is plural, isn't it? Maybe "A lot of code runs .." Christophe signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-server 1/2] Update spice-common submodule
For the series: Acked-by: Christophe Fergeau On Fri, Mar 08, 2019 at 12:25:48PM +, Frediano Ziglio wrote: > This brings in the following changes: > > Frediano Ziglio (11): > codegen: Document ptr_array attribute > codegen: Use a better type for pointer converted to integer > codegen: Reduce indentation > codegen: Fix c_type result for TypeAlias > codegen: Check wrong attribute > codegen: Add a test for attribute combination > mem: Fix compile error if alignment-checks option is used > log: Remove useless includes > proto: Remove obsolete TunnelChannel > protocol: Add a dummy TunnelChannel > messages: Remove fields not used by the protocol > > Marc-André Lureau (1): > docs: add spice URI scheme > > Signed-off-by: Frediano Ziglio > --- > subprojects/spice-common | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/subprojects/spice-common b/subprojects/spice-common > index 02530a80d..92d5dfd4b 16 > --- a/subprojects/spice-common > +++ b/subprojects/spice-common > @@ -1 +1 @@ > -Subproject commit 02530a80dfa45c936215c47b8e3aa56720eb46b8 > +Subproject commit 92d5dfd4bfa7ae4857e96504a6f14c336ed85338 > -- > 2.20.1 > > ___ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/spice-devel signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-server] syntax-check: Add missing contributors names to AUTHORS
Acked-by: Christophe Fergeau On Mon, Mar 18, 2019 at 10:57:41AM +, Frediano Ziglio wrote: > Signed-off-by: Frediano Ziglio > --- > AUTHORS | 1 + > 1 file changed, 1 insertion(+) > > This fixed "make syntax-check" > > diff --git a/AUTHORS b/AUTHORS > index 12158e60..a9b9ea79 100644 > --- a/AUTHORS > +++ b/AUTHORS > @@ -66,5 +66,6 @@ Patches also contributed by > Wang Qiang > Yann E. MORIN > Zeeshan Ali (Khattak) > +Douglas Paul > > send patches to get your name here... > -- > 2.20.1 > > ___ > Spice-devel mailing list > Spice-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/spice-devel signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] Build error (possible mirroring issue?)
> > Hey team, > > > I get a build failure that looks like this: > > char-device.c:906:52: error: too many arguments to function call, expected > single argument 'm', have 2 arguments > m2 = spice_marshaller_get_ptr_submarshaller(m, 0); > ~~^ > /Users/ddd/Work/spice/common/common/marshaller.h:56:1: note: > 'spice_marshaller_get_ptr_submarshaller' declared here > SpiceMarshaller *spice_marshaller_get_ptr_submarshaller(SpiceMarshaller *m); > ^ > 1 error generated. > > Indeed, the prototype was changed in spice-common by > > commit 302e30ff43401d9b1e7043a5e5c4f186ca997f66 > Author: Frediano Ziglio > Date: Wed Feb 27 08:55:56 2019 + > > codegen: Remove support for --ptrsize > > This is talking master for the server component from from > https://gitlab.freedesktop.org/spice/spice.git. > > Is this a git mirroring issue, or a missing commit on the server side? > > > Thanks > Christophe I think the issue is that your spice-server is using the wrong spice-common commit, just do a "git submodule update --init --recursive". There's a patch pending to review to "fix" this issue. Frediano ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] Build error (possible mirroring issue?)
Hey team, I get a build failure that looks like this: char-device.c:906:52: error: too many arguments to function call, expected single argument 'm', have 2 arguments m2 = spice_marshaller_get_ptr_submarshaller(m, 0); ~~^ /Users/ddd/Work/spice/common/common/marshaller.h:56:1: note: 'spice_marshaller_get_ptr_submarshaller' declared here SpiceMarshaller *spice_marshaller_get_ptr_submarshaller(SpiceMarshaller *m); ^ 1 error generated. Indeed, the prototype was changed in spice-common by commit 302e30ff43401d9b1e7043a5e5c4f186ca997f66 Author: Frediano Ziglio Date: Wed Feb 27 08:55:56 2019 + codegen: Remove support for --ptrsize This is talking master for the server component from from https://gitlab.freedesktop.org/spice/spice.git. Is this a git mirroring issue, or a missing commit on the server side? Thanks Christophe ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH spice-server] syntax-check: Add missing contributors names to AUTHORS
Signed-off-by: Frediano Ziglio --- AUTHORS | 1 + 1 file changed, 1 insertion(+) This fixed "make syntax-check" diff --git a/AUTHORS b/AUTHORS index 12158e60..a9b9ea79 100644 --- a/AUTHORS +++ b/AUTHORS @@ -66,5 +66,6 @@ Patches also contributed by Wang Qiang Yann E. MORIN Zeeshan Ali (Khattak) +Douglas Paul send patches to get your name here... -- 2.20.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-server] reds: Check we don't register a channel twice in reds_register_channel
> On 3/18/19 12:12 PM, Frediano Ziglio wrote: > >> Hi, > >> > >> On 3/15/19 11:27 AM, Frediano Ziglio wrote: > >>> To avoid possibly regression check it only if extra checks are > >>> enabled. > >> > >> Currently there's already a "channel duplication" warning upon connection > >> but won't hurt to have this extra check and emit an error. > >> > > I cannot find it. This patch is for spice-server. > > > Oh, sorry, "duplicate channel" on red_client_add_channel > These functions are checking RedChannelClient, not RedChannel. Basically they check if client is connecting twice to the same channel. > > > > >>> This allowed to check previous "Move channel registration to constructed > >> > >> This allow/s? i don't get this sentence, it just avoids duplicate call. > >> I think > >> i'd not even mention the previous commit > >> > > No, it will also exit the program if this condition is met. > > > Yes, i got it, i meant that such patch would have make sense also before > the "Move channel reg.." commit, > so not necessarily need to mention it. > Yes, I suppose would have made sense even before. But that's the reason why I added it and how I used it. > > > > > >> Snir. > >> > >>> vfunc" commit. > >>> > >>> Signed-off-by: Frediano Ziglio > >>> --- > >>>server/reds.c | 5 + > >>>1 file changed, 5 insertions(+) > >>> > >>> diff --git a/server/reds.c b/server/reds.c > >>> index 429f8142..e182eba7 100644 > >>> --- a/server/reds.c > >>> +++ b/server/reds.c > >>> @@ -380,6 +380,11 @@ void stat_remove_counter(SpiceServer *reds, > >>> RedStatCounter *counter) > >>>void reds_register_channel(RedsState *reds, RedChannel *channel) > >>>{ > >>>spice_assert(reds); > >>> +if (spice_extra_checks) { > >>> +uint32_t this_type, this_id; > >>> +g_object_get(channel, "channel-type", _type, "id", > >>> _id, > >>> NULL); > >>> +spice_assert(reds_find_channel(reds, this_type, this_id) == > >>> NULL); > >>> +} > >>>reds->channels = g_list_prepend(reds->channels, channel); > >>>// create new channel in the client if possible > >>>main_channel_registered_new_channel(reds->main_channel, channel); > > Frediano > ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-server v4 1/1] video-stream: prevent crash on stream reattach
> > I experienced some crashes with qemu 3.1.0 compiled with libspice-server > 0.14.0 on Gentoo. > > The problem reproduced reliably with a guest running Ubuntu 18.04.2 LTS. > If I connect a viewer at system startup, I would get a crash just after > the fade-in of the login prompt in GDM. > > Interestingly, I usually was unable to reproduce the issue if I waited > to connect until after the greeter was fully displayed. > > The patch I used to correct the issue for me applies to the master > branch cleanly, so I suspect the problem may still exist. > > The only other references to this issue I could find were two abrt > reports in CentOS: > https://bugs.centos.org/view.php?id=15171 > https://bugs.centos.org/view.php?id=15441 > > I'm not sure if the agent->video_encoder is supposed to be guaranteed to > exist when this function is called. > > Signed-off-by: Douglas Paul Acked-by: Frediano Ziglio if the client does not support some encoding it sends back a specific report and video_encoder is closed. This probably is causing the issue you reported. > --- > server/video-stream.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/server/video-stream.c b/server/video-stream.c > index b624093e..19795098 100644 > --- a/server/video-stream.c > +++ b/server/video-stream.c > @@ -369,7 +369,9 @@ static void before_reattach_stream(DisplayChannel > *display, > #ifdef STREAM_STATS > agent->stats.num_drops_pipe++; > #endif > - > agent->video_encoder->notify_server_frame_drop(agent->video_encoder); > +if (agent->video_encoder) { > + > agent->video_encoder->notify_server_frame_drop(agent->video_encoder); > +} > } > } > } Frediano ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-server] reds: Check we don't register a channel twice in reds_register_channel
On 3/18/19 12:12 PM, Frediano Ziglio wrote: Hi, On 3/15/19 11:27 AM, Frediano Ziglio wrote: To avoid possibly regression check it only if extra checks are enabled. Currently there's already a "channel duplication" warning upon connection but won't hurt to have this extra check and emit an error. I cannot find it. This patch is for spice-server. Oh, sorry, "duplicate channel" on red_client_add_channel This allowed to check previous "Move channel registration to constructed This allow/s? i don't get this sentence, it just avoids duplicate call. I think i'd not even mention the previous commit No, it will also exit the program if this condition is met. Yes, i got it, i meant that such patch would have make sense also before the "Move channel reg.." commit, so not necessarily need to mention it. Snir. vfunc" commit. Signed-off-by: Frediano Ziglio --- server/reds.c | 5 + 1 file changed, 5 insertions(+) diff --git a/server/reds.c b/server/reds.c index 429f8142..e182eba7 100644 --- a/server/reds.c +++ b/server/reds.c @@ -380,6 +380,11 @@ void stat_remove_counter(SpiceServer *reds, RedStatCounter *counter) void reds_register_channel(RedsState *reds, RedChannel *channel) { spice_assert(reds); +if (spice_extra_checks) { +uint32_t this_type, this_id; +g_object_get(channel, "channel-type", _type, "id", _id, NULL); +spice_assert(reds_find_channel(reds, this_type, this_id) == NULL); +} reds->channels = g_list_prepend(reds->channels, channel); // create new channel in the client if possible main_channel_registered_new_channel(reds->main_channel, channel); Frediano ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-server 2/2] docs: Add some notes on event scheduling and threading
ping > > Signed-off-by: Frediano Ziglio > --- > docs/spice_threading_model.txt | 8 > 1 file changed, 8 insertions(+) > > diff --git a/docs/spice_threading_model.txt b/docs/spice_threading_model.txt > index 9351141c8..25a3a030c 100644 > --- a/docs/spice_threading_model.txt > +++ b/docs/spice_threading_model.txt > @@ -39,6 +39,14 @@ connect, disconnect and migrate. Connect and migrate are > asynchronous (the job > is done while the current thread is doing something else) while disconnect > is > synchronous (the main thread will wait for termination). > > +One aspect to take into consideration is the event scheduling. SPICE uses > some > +`SpiceCoreInterface` to handle events. As the events will be handled from a > +thread based on the core interface you have to use the correct core. Each > +channel has an associated core interface which can be retrieved using > +`red_channel_get_core_interface`. There's also a main core interface you can > get > +using `reds_get_core_interface`. `reds_core_timer_*` and `reds_core_watch_*` > +functions use the main core interface. > + > Reference counting and ownership > > -> pointer ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v2 spice-gtk] Adjust to window scaling
> > When GDK_SCALE is != 1 and egl is used, the image presented does not > fit to the window (scale of 2 is often used with hidpi monitors). > Usually this is not a problem since all components are adjusted by > gdk/gtk but with egl, pixel-based data is not being scaled. In this > case window's scale value can be used in order to determine whether > to use a pixel resource with higher resolution data. > > In order to reproduce the problem set spice with virgl/Intel-vGPU > and run spice-gtk with GDK_SCALE=2 I still didn't try. So from what I understand the sizes and coordinated from GTK when GDK_SCALE == 2 (for instance) are half of the pixel sizes, right? So for instance width == 1024 means 2048 pixels. > --- > Changes from v1: > -commit msg > -replace var naming (ws with win_scale) > > > This patch is kind of RFC, it fixes the issue, but it's a bit hacky > and specific. I didn't come across other scale issues but it is likely > that more of these exist and better and generic fix is needed. > > --- > src/spice-widget-egl.c | 15 +-- > src/spice-widget-priv.h | 1 + > 2 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/src/spice-widget-egl.c b/src/spice-widget-egl.c > index 43fccd7..600c87a 100644 > --- a/src/spice-widget-egl.c > +++ b/src/spice-widget-egl.c > @@ -326,6 +326,8 @@ static gboolean spice_widget_init_egl_win(SpiceDisplay > *display, GdkWindow *win, > if (d->egl.surface) > return TRUE; > > +d->egl.scale = gdk_window_get_scale_factor(win); > + > #ifdef GDK_WINDOWING_X11 > if (GDK_IS_X11_WINDOW(win)) { > native = (EGLNativeWindowType)GDK_WINDOW_XID(win); This factor is only read on initialization, what would happen changing resolution while the client is open or attaching a new HiDPI monitor? Can't we keep the "win" and avoid caching it? It should not take long to read the factor. > @@ -431,15 +433,17 @@ void spice_egl_resize_display(SpiceDisplay *display, > int w, int h) > { > SpiceDisplayPrivate *d = display->priv; > int prog; > +gint win_scale; > > if (!gl_make_current(display, NULL)) > return; > > +win_scale = d->egl.scale; > glGetIntegerv(GL_CURRENT_PROGRAM, ); > > glUseProgram(d->egl.prog); > -apply_ortho(d->egl.mproj, 0, w, 0, h, -1, 1); > -glViewport(0, 0, w, h); > +apply_ortho(d->egl.mproj, 0, w * win_scale , 0, h * win_scale, -1, 1); > +glViewport(0, 0, w * win_scale, h * win_scale); > Why not multiplying w and h by d->egl.scale like in other function? > if (d->ready) > spice_egl_update_display(display); > @@ -559,6 +563,13 @@ void spice_egl_update_display(SpiceDisplay *display) > > spice_display_get_scaling(display, , , , , ); > > +// Adjust to gdk scale > +s *= d->egl.scale; > +x *= d->egl.scale; > +y *= d->egl.scale; > +w *= d->egl.scale; > +h *= d->egl.scale; > + > glClearColor(0.0f, 0.0f, 0.0f, 0.0f); > glClear(GL_COLOR_BUFFER_BIT); > > diff --git a/src/spice-widget-priv.h b/src/spice-widget-priv.h > index 65eb404..8f110ac 100644 > --- a/src/spice-widget-priv.h > +++ b/src/spice-widget-priv.h > @@ -149,6 +149,7 @@ struct _SpiceDisplayPrivate { > EGLImageKHR image; > gbooleancall_draw_done; > SpiceGlScanout scanout; > +gintscale; Why in egl? It's not a property related to egl. > } egl; > #endif // HAVE_EGL > double scroll_delta_y; Frediano ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-server] reds: Check we don't register a channel twice in reds_register_channel
> > Hi, > > On 3/15/19 11:27 AM, Frediano Ziglio wrote: > > To avoid possibly regression check it only if extra checks are > > enabled. > > > Currently there's already a "channel duplication" warning upon connection > but won't hurt to have this extra check and emit an error. > I cannot find it. This patch is for spice-server. > > This allowed to check previous "Move channel registration to constructed > > > This allow/s? i don't get this sentence, it just avoids duplicate call. > I think > i'd not even mention the previous commit > No, it will also exit the program if this condition is met. > > Snir. > > > vfunc" commit. > > > > Signed-off-by: Frediano Ziglio > > --- > > server/reds.c | 5 + > > 1 file changed, 5 insertions(+) > > > > diff --git a/server/reds.c b/server/reds.c > > index 429f8142..e182eba7 100644 > > --- a/server/reds.c > > +++ b/server/reds.c > > @@ -380,6 +380,11 @@ void stat_remove_counter(SpiceServer *reds, > > RedStatCounter *counter) > > void reds_register_channel(RedsState *reds, RedChannel *channel) > > { > > spice_assert(reds); > > +if (spice_extra_checks) { > > +uint32_t this_type, this_id; > > +g_object_get(channel, "channel-type", _type, "id", _id, > > NULL); > > +spice_assert(reds_find_channel(reds, this_type, this_id) == NULL); > > +} > > reds->channels = g_list_prepend(reds->channels, channel); > > // create new channel in the client if possible > > main_channel_registered_new_channel(reds->main_channel, channel); Frediano ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-server] reds: Check we don't register a channel twice in reds_register_channel
Hi, On 3/15/19 11:27 AM, Frediano Ziglio wrote: To avoid possibly regression check it only if extra checks are enabled. Currently there's already a "channel duplication" warning upon connection but won't hurt to have this extra check and emit an error. This allowed to check previous "Move channel registration to constructed This allow/s? i don't get this sentence, it just avoids duplicate call. I think i'd not even mention the previous commit Snir. vfunc" commit. Signed-off-by: Frediano Ziglio --- server/reds.c | 5 + 1 file changed, 5 insertions(+) diff --git a/server/reds.c b/server/reds.c index 429f8142..e182eba7 100644 --- a/server/reds.c +++ b/server/reds.c @@ -380,6 +380,11 @@ void stat_remove_counter(SpiceServer *reds, RedStatCounter *counter) void reds_register_channel(RedsState *reds, RedChannel *channel) { spice_assert(reds); +if (spice_extra_checks) { +uint32_t this_type, this_id; +g_object_get(channel, "channel-type", _type, "id", _id, NULL); +spice_assert(reds_find_channel(reds, this_type, this_id) == NULL); +} reds->channels = g_list_prepend(reds->channels, channel); // create new channel in the client if possible main_channel_registered_new_channel(reds->main_channel, channel); ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel