[Spice-devel] [PATCH v6 10/14] usbredir: Disconnect USB device asynchronously
Dmitry Fleytman
dmitry at daynix.com
Tue Mar 8 13:44:30 UTC 2016
> On 1 Mar 2016, at 21:25 PM, Jonathon Jongsma <jjongsma at redhat.com> wrote:
>
> On Sun, 2016-02-28 at 11:54 +0200, Dmitry Fleytman wrote:
>> From: Kirill Moizik <kmoizik at redhat.com>
>>
>> Signed-off-by: Kirill Moizik <kmoizik at redhat.com>
>> Signed-off-by: Dmitry Fleytman <dfleytma at 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 <jjongsma at redhat.com <mailto:jjongsma at redhat.com>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160308/0430d033/attachment.html>
More information about the Spice-devel
mailing list