[Spice-devel] unplug USB Device, guest Desktop will unresponsive

[Christophe Fergeau]

On Mon, Jun 12, 2017 at 05:44:45PM +0800, leaboy wrote:
> hi, everyone.
>         when I unplug a USB Storage from client machine,  the guest desktop 
> will unresponsive at thus case:
> 
> 1. the USB Storage device with 16G or 32G has redir to the guest desktop, and coping some big file.
> then unplug the USB Storage device from the client machine.
> 
> 2. when the USB Storage device is being identified in the guest os, then unplug from the client machine.
> 
> 3. when the guest is check on the auto_redir, then plugin and unplug the USB Storage Device frequently.
> 
> 
> I don’t know whether the patch code is correct, but the desktop not unresponsive at upon cases.
> 
> I debugged the code, when into coroutine_yield and the device is
> unplugged, sometimes, this usbredir-channel will not be unlocked.

More specifically (I'd add these details to the commit log), when an
error occurs, we queue a call to device_error in an idle, and switch to
the main context. device_error() calls
spice_usbredir_channel_disconnect_device() which needs the channel to be
unlocked as it calls spice_usbredir_channel_lock().
The current code deadlocks when a USB error occurs, releasing the
channel lock before the idle is called fixes this problem.

Thanks for the patch,

Christophe

> 
> 
> diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c
> index fef62ce..1f791bc 100644
> --- a/src/channel-usbredir.c
> +++ b/src/channel-usbredir.c
> @@ -927,15 +927,16 @@ static void usbredir_handle_msg(SpiceChannel *c, SpiceMsgIn *in)
>          err_data.caller = coroutine_self();
>          err_data.spice_device = g_boxed_copy(spice_usb_device_get_type(), spice_device);
>          err_data.error = err;
> +        spice_usbredir_channel_unlock(channel);
>          g_idle_add(device_error, &err_data);
>          coroutine_yield(NULL);
>  
>          g_boxed_free(spice_usb_device_get_type(), err_data.spice_device);
>  
>          g_error_free(err);
> +    } else {
> +        spice_usbredir_channel_unlock(channel);
>      }
> -
> -    spice_usbredir_channel_unlock(channel);
>  }
>  
>  #endif /* USE_USBREDIR */
> 

> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170619/9bb6ed7b/attachment-0001.sig>