> On 1 Mar 2016, at 21:25 PM, Jonathon Jongsma <jjong...@redhat.com> wrote: > > On Sun, 2016-02-28 at 11:54 +0200, Dmitry Fleytman wrote: >> From: Kirill Moizik <kmoi...@redhat.com> >> >> Signed-off-by: Kirill Moizik <kmoi...@redhat.com> >> Signed-off-by: Dmitry Fleytman <dfley...@redhat.com> >> --- >> src/channel-usbredir.c | 38 +++++++++++++++++++++++++++++++------- >> 1 file changed, 31 insertions(+), 7 deletions(-) >> >> diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c >> index ebf9fce..3a10823 100644 >> --- a/src/channel-usbredir.c >> +++ b/src/channel-usbredir.c >> @@ -114,20 +114,44 @@ static void >> spice_usbredir_channel_init(SpiceUsbredirChannel *channel) >> } >> >> #ifdef USE_USBREDIR >> + >> +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); >> + SpiceUsbredirChannelPrivate *priv = channel->priv; >> + gboolean migrating = GPOINTER_TO_UINT(user_data); >> + GError *err = NULL; >> + >> + 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_CHANNEL_CLASS(spice_usbredir_channel_parent_class) >> ->channel_reset(spice_channel, migrating); >> + >> + spice_usbredir_channel_unlock(channel); >> + >> + 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(channel); >> - usbredirhost_close(priv->host); >> - priv->host = NULL; >> - /* Call set_context to re-create the host */ >> - spice_usbredir_channel_set_context(channel, priv->context); >> + if (priv->state == STATE_CONNECTED) { >> + spice_usbredir_channel_disconnect_device_async(channel, NULL, >> + _channel_reset_cb, GUINT_TO_POINTER(migrating)); > > I don't think I noticed this on the previous review, but it seems there is a > change in behavior introduced here. Previously, if priv->state was not > STATE_CONNECTED, it would omit the call to > spice_usbredir_channel_disconnect_device(), but would proceed to call > usbredirhost_close(), etc. In the new version, these additional calls are done > in the async callback. And the async function is only executed if state is > STATE_CONNECTED. I'm afraid I don't know enough about this code to know > whether > that's going to be an issue. Was this change intentional?
You’re right. This change is unintentional, the original behaviour is preserved in the next version. > >> + } >> + } else { >> + SPICE_CHANNEL_CLASS(spice_usbredir_channel_parent_class) >> ->channel_reset(c, migrating); >> } >> - SPICE_CHANNEL_CLASS(spice_usbredir_channel_parent_class) >> ->channel_reset(c, migrating); >> } >> #endif >> > > Reviewed-by: Jonathon Jongsma <jjong...@redhat.com > <mailto:jjong...@redhat.com>>
_______________________________________________ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel