Hi, On Wed, Sep 26, 2018 at 06:17:34AM -0400, Frediano Ziglio wrote: > > > > From: Victor Toso <me@xxxxxxxxxxxxxx> > > > > This should not change code behavior, only add some comments. > > > > This is a preparatory patch for bug below, introduced with: > > > > commit 9fbf679453d8dbfe797a738cb536136599d7adab > > Author: Kirill Moizik <kmoizik@xxxxxxxxxx> > > Date: Tue Mar 8 16:05:57 2016 +0200 > > > > usbredir: Disconnect USB device asynchronously > > > > Related: https://bugzilla.redhat.com/show_bug.cgi?id=1625550 > > Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx> > > --- > > src/channel-usbredir.c | 27 +++++++++++++++++++-------- > > 1 file changed, 19 insertions(+), 8 deletions(-) > > > > diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c > > index 1d9c380..7f6e2e3 100644 > > --- a/src/channel-usbredir.c > > +++ b/src/channel-usbredir.c > > @@ -158,16 +158,27 @@ 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(channel, NULL, > > - _channel_reset_cb, GUINT_TO_POINTER(migrating)); > > - } else { > > - _channel_reset_finish(channel); > > - } > > - } else { > > + /* Host isn't running, just reset */ > > + if (!priv->host) { > > SPICE_CHANNEL_CLASS(spice_usbredir_channel_parent_class)->channel_reset(c, > > migrating); > > + return; > > } > > + > > + /* Host is running, so we might need to disconnect the usb devices > > async. > > + * This should not block channel_reset() otherwise we might run in > > reconnection > > + * problems such as https://bugzilla.redhat.com/show_bug.cgi?id=1625550 > > + * No operation from here on should rely on SpiceChannel as its > > coroutine > > + * might be terminated. */ > > + > > + if (priv->state == STATE_CONNECTED) { > > + /* FIXME: We should add GCancellable here */ > > I still think that not chaining channel_reset here is a bug. > But this is not a regression. While I agree, I have to understand a bit more of the usb side to properly fix it. Commit 9fbf679453d8db does not provide much context, I'll have to dig mailing list for the rationale plus find a way to reset device async while resetting SpiceChannel here. Well, all might be fixed with the recent rework there with cd redir... More things to understand. > Why you want to support GCancellable? Looks like more a TODO > than a FIXME. I'll change the comment to /* FIXME: We should chain-up parent's channel-reset here */ Sounds good? > > > + spice_usbredir_channel_disconnect_device_async(channel, NULL, > > + _channel_reset_cb, GUINT_TO_POINTER(migrating)); > > + return; > > + } > > + > > + /* FIXME: This does not chain-up with parent's channel-reset, which is a > > must */ > > + _channel_reset_finish(channel); > > } > > #endif > > > > Otherwise, > > Acked-by: Frediano Ziglio <fziglio@xxxxxxxxxx> > > Frediano Thanks,
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel