On Sat, 2018-05-12 at 11:19 +0800, Qiu Wenbo wrote:
> Yes. The situation can be easily reproduced by the patch below. We
> have noticed it usually happen on some thin clients (without the
> patch). Also, I can reproduce it on my desktop (latest version of
> Arch linux, ryzen 1600x) with debug build of qemu, spice-gtk, and
> usbredir with remote-viewer --spice-debug and gdb attached(also
> without the patch). Step to produce the situation:
>
> 1. launch a virtual machine, and redirect a USB mass storage device
> (or something else) to it
> 2. run udevadmin monitor
> 3. poweroff your virtual machine by running poweroff in your virtual
> machine
>
> The udevadmin won't tell you there is an ADD uevent for your USB
> device if the situation is produced. Host operating system won't do a
> driver probe automatically without an ADD uevent, and you should do a
> replug to work with the device.
>
> diff --git a/src /channel-usbredir.c b/src/channel-usbredir.c
> index 0cc5630..2c27825 100644
> --- a/src/channel-usbredir.c
> +++ b/src/channel-usbredir.c
> @@ -128,6 +128,7 @@ static void
> _channel_reset_finish(SpiceUsbredirChannel *channel)
>
> spice_usbredir_channel_lock(channel);
>
> + sleep(1);
> usbredirhost_close(priv->host);
> priv->host = NULL;
I was not able to reproduce with the patch you provide above, but I was
able to reproduce it by inserting a sleep in
_disconnect_device_thread(). As Christophe says, it's rather tricky
problem. I'm looking at it now. Thanks for reporting the issue.
Jonathon
>
>
>
> On Fri, May 11, 2018 at 12:27 AM, Jonathon Jongsma <jjongsma@redhat.c
> om> wrote:
> > On Mon, 2018-05-07 at 17:02 +0800, Qiu Wenbo wrote:
> > > This reverts commit 9fbf679453d8dbfe797a738cb536136599d7adab.
> > >
> > > In some cases, remote-viewer will exit before the async function
> > > run
> > > in
> > > another thread finish and USB devices redirected to the VM will
> > > not
> > > "pop up" to
> > > operation system. For example, a USB disk should be auto mounted
> > > when remote-viewer exit.
> >
> > Do you have a reliable way to reproduce this situation?
> >
> >
> > >
> > > Signed-off-by: Qiu Wenbo <qiuwenbo@xxxxxxxxxxxxxx>
> > > ---
> > > src/channel-usbredir.c | 48 ++++++-----------------------------
> > > ---
> > > ----
> > > 1 file changed, 7 insertions(+), 41 deletions(-)
> > >
> > > diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c
> > > index 0cc5630..34c4679 100644
> > > --- a/src/channel-usbredir.c
> > > +++ b/src/channel-usbredir.c
> > > @@ -121,54 +121,20 @@ static void
> > > spice_usbredir_channel_init(SpiceUsbredirChannel *channel)
> > > }
> > >
> > > #ifdef USE_USBREDIR
> > > -
> > > -static void _channel_reset_finish(SpiceUsbredirChannel
> > > *channel)
> > > -{
> > > - SpiceUsbredirChannelPrivate *priv = channel->priv;
> > > -
> > > - spice_usbredir_channel_lock(channel);
> > > -
> > > - usbredirhost_close(priv->host);
> > > - priv->host = NULL;
> > > -
> > > - /* Call set_context to re-create the host */
> > > - spice_usbredir_channel_set_context(channel, priv->context);
> > > -
> > > - spice_usbredir_channel_unlock(channel);
> > > -}
> > > -
> > > -static void _channel_reset_cb(GObject *gobject,
> > > - GAsyncResult *result,
> > > - gpointer user_data)
> > > -{
> > > - SpiceChannel *spice_channel = SPICE_CHANNEL(gobject);
> > > - SpiceUsbredirChannel *channel =
> > > SPICE_USBREDIR_CHANNEL(spice_channel);
> > > - gboolean migrating = GPOINTER_TO_UINT(user_data);
> > > - GError *err = NULL;
> > > -
> > > - _channel_reset_finish(channel);
> > > -
> > > - SPICE_CHANNEL_CLASS(spice_usbredir_channel_parent_class)-
> > > >channel_reset(spice_channel, migrating);
> > > -
> > > - spice_usbredir_channel_disconnect_device_finish(channel,
> > > result,
> > > &err);
> > > - g_object_unref(result);
> > > -}
> > > -
> > > static void spice_usbredir_channel_reset(SpiceChannel *c,
> > > gboolean
> > > migrating)
> > > {
> > > SpiceUsbredirChannel *channel = SPICE_USBREDIR_CHANNEL(c);
> > > SpiceUsbredirChannelPrivate *priv = channel->priv;
> > >
> > > if (priv->host) {
> > > - if (priv->state == STATE_CONNECTED) {
> > > - spice_usbredir_channel_disconnect_device_async(chan
> > > nel,
> > > NULL,
> > > - _channel_reset_cb,
> > > GUINT_TO_POINTER(migrating));
> > > - } else {
> > > - _channel_reset_finish(channel);
> > > - }
> > > - } else {
> > > - SPICE_CHANNEL_CLASS(spice_usbredir_channel_parent_class
> > > )-
> > > >channel_reset(c, migrating);
> > > + if (priv->state == STATE_CONNECTED)
> > > + spice_usbredir_channel_disconnect_device(channel);
> > > + usbredirhost_close(priv->host);
> > > + priv->host = NULL;
> > > + /* Call set_context to re-create the host */
> > > + spice_usbredir_channel_set_context(channel, priv-
> > > >context);
> > > }
> > > + SPICE_CHANNEL_CLASS(spice_usbredir_channel_parent_class)-
> > > >channel_reset(c, migrating);
> > > }
> > > #endif
> > >
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel