On Wed, Mar 14, 2012 at 09:55:01AM +0100, Hans de Goede wrote:
> Currently trying to view a usbredir enabled vm from virt-manager causes
> virt-manager to crash. This crash is caused by the following happening:
>
> -virt-manager sets up the session, including connecting all the channels
> -a spice-gtk internal code path calls spice_usb_device_manager_get()
> -spice_usb_device_manager_get calls channel_new on all already connected
> usb channels
> -channel_new does:
> spice_usbredir_channel_set_context(SPICE_USBREDIR_CHANNEL(channel),
> self->priv->context);
> -But self->priv->context has not been set yet (so is NULL) -> segfault!
>
> This patch fixes this by moving the iterating over already connected
> usb channels to after the setting of self->priv->context. Note this means
> that the channels will no longer get checked when there is no USB_REDIR
> support. That is not a problem since spice_usb_device_manager_initable_init
> will return FALSE in that case anyways.
Looks good,
ACK.
>
> Signed-off-by: Hans de Goede <hdego...@redhat.com>
> ---
> gtk/usb-device-manager.c | 27 +++++++++++++++------------
> 1 file changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/gtk/usb-device-manager.c b/gtk/usb-device-manager.c
> index 37cc88b..18d5bd5 100644
> --- a/gtk/usb-device-manager.c
> +++ b/gtk/usb-device-manager.c
> @@ -147,11 +147,11 @@ static gboolean
> spice_usb_device_manager_initable_init(GInitable *initable,
> GCancellable
> *cancellable,
> GError **err)
> {
> - GList *list;
> - GList *it;
> SpiceUsbDeviceManager *self;
> SpiceUsbDeviceManagerPrivate *priv;
> #ifdef USE_USBREDIR
> + GList *list;
> + GList *it;
> int rc;
> const gchar *const subsystems[] = {"usb", NULL};
> #endif
> @@ -174,17 +174,8 @@ static gboolean
> spice_usb_device_manager_initable_init(GInitable *initable,
> return FALSE;
> }
>
> - g_signal_connect(priv->session, "channel-new",
> - G_CALLBACK(channel_new), self);
> - g_signal_connect(priv->session, "channel-destroy",
> - G_CALLBACK(channel_destroy), self);
> - list = spice_session_get_channels(priv->session);
> - for (it = g_list_first(list); it != NULL; it = g_list_next(it)) {
> - channel_new(priv->session, it->data, (gpointer*)self);
> - }
> - g_list_free(list);
> -
> #ifdef USE_USBREDIR
> + /* Initialize libusb */
> rc = libusb_init(&priv->context);
> if (rc < 0) {
> const char *desc = spice_usbutil_libusb_strerror(rc);
> @@ -194,6 +185,18 @@ static gboolean
> spice_usb_device_manager_initable_init(GInitable *initable,
> return FALSE;
> }
>
> + /* Start listening for usb channels connect/disconnect */
> + g_signal_connect(priv->session, "channel-new",
> + G_CALLBACK(channel_new), self);
> + g_signal_connect(priv->session, "channel-destroy",
> + G_CALLBACK(channel_destroy), self);
> + list = spice_session_get_channels(priv->session);
> + for (it = g_list_first(list); it != NULL; it = g_list_next(it)) {
> + channel_new(priv->session, it->data, (gpointer*)self);
> + }
> + g_list_free(list);
> +
> + /* Start listening for usb devices plug / unplug */
> priv->udev = g_udev_client_new(subsystems);
> g_signal_connect(G_OBJECT(priv->udev), "uevent",
> G_CALLBACK(spice_usb_device_manager_uevent_cb), self);
> --
> 1.7.9.3
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
