On Sun, 2016-02-28 at 11:54 +0200, Dmitry Fleytman wrote:
> This patch fixes device list change notification handing
> logic for cases when more than one device being plugged
> or unplugged simultaneously.
>
> The simplest way to reproduce the problematic scenario
> is (un)plugging of a usb HUB with a few devices inserted.
>
> Signed-off-by: Dmitry Fleytman <dmitry@xxxxxxxxxx>
> ---
> src/win-usb-dev.c | 82 +++++++++++++++++++-----------------------------------
> -
> 1 file changed, 28 insertions(+), 54 deletions(-)
>
> diff --git a/src/win-usb-dev.c b/src/win-usb-dev.c
> index 1e4b2d6..1b4d353 100644
> --- a/src/win-usb-dev.c
> +++ b/src/win-usb-dev.c
> @@ -34,7 +34,6 @@
>
> struct _GUdevClientPrivate {
> libusb_context *ctx;
> - gssize udev_list_size;
> GList *udev_list;
> HWND hwnd;
> };
> @@ -196,9 +195,7 @@ g_udev_client_initable_init(GInitable *initable,
> GCancellable *cancellable,
> }
>
> /* get initial device list */
> - priv->udev_list_size = g_udev_client_list_devices(self, &priv->udev_list,
> - err, __FUNCTION__);
> - if (priv->udev_list_size < 0) {
> + if (g_udev_client_list_devices(self, &priv->udev_list, err, __FUNCTION__)
> < 0) {
> goto g_udev_client_init_failed;
> }
>
> @@ -339,74 +336,51 @@ static gboolean gudev_devices_are_equal(GUdevDevice *a,
> GUdevDevice *b)
Immediately above this line is the following comment:
/* Assumes each event stands for a single device change (at most) */
Since you're changing the function to handle multiple insertions or removals at
once, we should probably remove this comment.
> static void handle_dev_change(GUdevClient *self)
> {
> GUdevClientPrivate *priv = self->priv;
> - GUdevDevice *changed_dev = NULL;
> - ssize_t dev_count;
> - int is_dev_change;
> GError *err = NULL;
> GList *now_devs = NULL;
> - GList *llist, *slist; /* long-list and short-list*/
> - GList *lit, *sit; /* iterators for long-list and short-list */
> - GUdevDevice *ldev, *sdev; /* devices on long-list and short-list */
> + GList *prev, *curr; /* iterators for previous and current device lists */
>
> - dev_count = g_udev_client_list_devices(self, &now_devs, &err,
> - __FUNCTION__);
> - g_return_if_fail(dev_count >= 0);
> -
> - SPICE_DEBUG("number of current devices %"G_GSSIZE_FORMAT
> - ", I know about %"G_GSSIZE_FORMAT" devices",
> - dev_count, priv->udev_list_size);
> -
> - is_dev_change = dev_count - priv->udev_list_size;
> - if (is_dev_change == 0) {
> - g_udev_client_free_device_list(&now_devs);
> + if(g_udev_client_list_devices(self, &now_devs, &err, __FUNCTION__) < 0) {
> + g_warning("could not retrieve device list");
> return;
> }
>
> - if (is_dev_change > 0) {
> - llist = now_devs;
> - slist = priv->udev_list;
> - } else {
> - llist = priv->udev_list;
> - slist = now_devs;
> - }
> -
> - g_udev_device_print_list(llist, "handle_dev_change: long list:");
> - g_udev_device_print_list(slist, "handle_dev_change: short list:");
> + 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:");
>
> - /* Go over the longer list */
> - for (lit = g_list_first(llist); lit != NULL; lit=g_list_next(lit)) {
> - ldev = lit->data;
> - /* Look for dev in the shorther list */
> - for (sit = g_list_first(slist); sit != NULL; sit=g_list_next(sit)) {
> - sdev = sit->data;
> - if (gudev_devices_are_equal(ldev, sdev))
> + /* Unregister devices that are not present anymore */
> + for (prev = g_list_first(priv->udev_list); prev != NULL; prev =
> g_list_next(prev)) {
> + /* Look for dev in the current list */
> + for (curr = g_list_first(now_devs); curr != NULL; curr =
> g_list_next(curr)) {
> + if (gudev_devices_are_equal(prev->data, curr->data))
g_list_find_custom() is a possibility here
> break;
> }
> - if (sit == NULL) {
> - /* Found a device which appears only in the longer list */
> - changed_dev = ldev;
> - break;
> +
> + if (curr == NULL) {
> + /* Found a device that was removed */
> + g_udev_device_print(prev->data, "<<< USB device removed");
> + g_signal_emit(self, signals[UEVENT_SIGNAL], 0, "remove", prev
> ->data);
> }
> }
>
> - if (!changed_dev) {
> - g_warning("couldn't find any device change");
> - goto leave;
> - }
> + /* Register newly inserted devices */
> + for (curr = g_list_first(now_devs); curr != NULL; curr =
> g_list_next(curr)) {
> + /* Look for dev in the previous list */
> + for (prev = g_list_first(priv->udev_list); prev != NULL; prev =
> g_list_next(prev)) {
> + if (gudev_devices_are_equal(prev->data, curr->data))
> + break;
and here
> + }
>
> - if (is_dev_change > 0) {
> - g_udev_device_print(changed_dev, ">>> USB device inserted");
> - g_signal_emit(self, signals[UEVENT_SIGNAL], 0, "add", changed_dev);
> - } else {
> - g_udev_device_print(changed_dev, "<<< USB device removed");
> - g_signal_emit(self, signals[UEVENT_SIGNAL], 0, "remove",
> changed_dev);
> + if (prev == NULL) {
> + /* Found a device that was inserted */
> + g_udev_device_print(curr->data, ">>> USB device inserted");
> + g_signal_emit(self, signals[UEVENT_SIGNAL], 0, "add", curr
> ->data);
> + }
> }
>
> -leave:
> /* keep most recent info: free previous list, and keep current list */
> g_udev_client_free_device_list(&priv->udev_list);
> priv->udev_list = now_devs;
> - priv->udev_list_size = dev_count;
> }
>
> static LRESULT CALLBACK wnd_proc(HWND hwnd, UINT message, WPARAM wparam,
> LPARAM lparam)
Acked-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel