[Spice-devel] [PATCH v3 08/13] Usbredir channel: use disconnect_async

[Dmitry Fleytman]

> On Aug 11, 2015, at 15:31 PM, Christophe Fergeau <cfergeau at redhat.com> wrote:
> 
> In the subject line, I'd only use "usbredir: disconnect USB device
> asynchronously”

Fixed.

> 
> In the commit log, there should be an explanation why it's ok to do
> that, and why the rest of the code is going to be able to deal with
> disconnection being asynchronous.
> 

Added detailed explanation in v4.


> Why are spice_usbredir_channel_dispose() and device_error() not using an
> async disconnection?

Unfortunately covering these flows makes code too complicated and creates a lot of additional corner cases.
IMHO these 2 flows are less important from UX point of view so I would leave those as is.

> 
> Christophe
> 
> On Mon, Aug 03, 2015 at 04:10:48PM +0300, Kirill Moizik wrote:
>> From: Kirill Moizik <kmoizik at redhat.com>
>> 
>> ---
>> src/channel-usbredir.c | 41 +++++++++++++++++++++++++++++++++++------
>> 1 file changed, 35 insertions(+), 6 deletions(-)
>> 
>> diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c
>> index 4cf1173..699b845 100644
>> --- a/src/channel-usbredir.c
>> +++ b/src/channel-usbredir.c
>> @@ -114,20 +114,49 @@ static void spice_usbredir_channel_init(SpiceUsbredirChannel *channel)
>> }
>> 
>> #ifdef USE_USBREDIR
>> +typedef struct _reset_cb_data
>> +{
>> +    gboolean migrating;
>> +} reset_cb_data;
>> +
>> +static void spice_usbredir_channel_reset_cb(GObject *gobject,
>> +                                            GAsyncResult *result,
>> +                                            gpointer user_data)
>> +{
>> +    SpiceChannel *spiceChannel =  SPICE_CHANNEL(gobject);
>> +    SpiceUsbredirChannel *channel = SPICE_USBREDIR_CHANNEL(spiceChannel);
>> +    SpiceUsbredirChannelPrivate *priv = channel->priv;
>> +    g_mutex_lock(priv->flows_mutex);
>> +    reset_cb_data *data = user_data;
>> +
>> +    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(spiceChannel, data->migrating);
>> +    g_mutex_unlock(priv->flows_mutex);
>> +    g_object_unref(result);
>> +    g_free(data);
>> +}
>> +
>> static void spice_usbredir_channel_reset(SpiceChannel *c, gboolean migrating)
>> {
>>     SpiceUsbredirChannel *channel = SPICE_USBREDIR_CHANNEL(c);
>>     SpiceUsbredirChannelPrivate *priv = channel->priv;
>> 
>> +    GSimpleAsyncResult *result ;
>>     if (priv->host) {
>> -        if (priv->state == STATE_CONNECTED)
>> +        if (priv->state == STATE_CONNECTED){
>> +            reset_cb_data *data = g_new(reset_cb_data,1);
>> +            data->migrating = migrating;
>> +            result = g_simple_async_result_new(G_OBJECT(c),
>> +                       spice_usbredir_channel_reset_cb, data,
>> +                       spice_usbredir_channel_reset);
>>             spice_usbredir_channel_disconnect_device_async(channel, result, NULL);
>> -        usbredirhost_close(priv->host);
>> -        priv->host = NULL;
>> -        /* Call set_context to re-create the host */
>> -        spice_usbredir_channel_set_context(channel, priv->context);
>> +        }
>> +    } 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
>> 
>> -- 
>> 2.4.3
>> 
>> _______________________________________________
>> Spice-devel mailing list
>> Spice-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/spice-devel