[Spice-devel] [PATCH spice-gtk v2] session: initialize USB device manager on session creation

[Christophe Fergeau]

Hey,

On Thu, Jan 03, 2019 at 03:41:35PM +0400, marcandre.lureau at redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau at redhat.com>
> 
> Simple command line tools simply connect all channels, however
> usbredir channels require that the manager was previously
> initialized. Currently, running spicy-stats on a VM with usbredir
> channels prints:
> 
> (lt-spicy-stats:25224): GSpice-CRITICAL **: 14:30:54.724: spice_usbredir_channel_up: assertion 'priv->host != NULL' failed
> 
> (lt-spicy-stats:25224): GSpice-CRITICAL **: 14:30:54.724: usbredir_handle_msg: assertion 'priv->host != NULL' failed
> 
> There is not strong reason not to initialize the USB device manager
> when the session is created.
> 
> Notes:
> - when usbredir isn't compiled in, those criticals aren't reached
> - a previous attempt was to initialize the usb device manager during
>   session instance init, however the manager shouldn't interact with a
>   temporary migration session

Looks good to me (it's very similar to 291f3e4 except that the code only
runs in spice_session_new, and not in spice_session_new_from_session as
was happening when this code was part of spice_session_init).

I've tested migration + usbredir and spicy-stats, and I did not hit any
of the 2 bugs we had in the past (and I've checked that without " Revert
"channel-usbredir: Fix crash on channel-up" I can still reproduce the
migration bug, and without your patch, I get the criticals from the
commit log).

Reviewed-by: Christophe Fergeau <cfergeau at redhat.com>

Christophe


> 
> Fixes:
> https://gitlab.freedesktop.org/spice/spice-gtk/issues/67
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>
> ---
>  src/spice-session.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/src/spice-session.c b/src/spice-session.c
> index a2ed401..76c90b3 100644
> --- a/src/spice-session.c
> +++ b/src/spice-session.c
> @@ -1492,7 +1492,17 @@ static void spice_session_class_init(SpiceSessionClass *klass)
>   **/
>  SpiceSession *spice_session_new(void)
>  {
> -    return SPICE_SESSION(g_object_new(SPICE_TYPE_SESSION, NULL));
> +    SpiceSession *self = SPICE_SESSION(g_object_new(SPICE_TYPE_SESSION, NULL));
> +    SpiceSessionPrivate *priv = self->priv;
> +    GError *err = NULL;
> +
> +    priv->usb_manager = spice_usb_device_manager_get(self, &err);
> +    if (err != NULL) {
> +        SPICE_DEBUG("Could not initialize SpiceUsbDeviceManager - %s", err->message);
> +        g_clear_error(&err);
> +    }
> +
> +    return self;
>  }
>  
>  G_GNUC_INTERNAL
> -- 
> 2.20.1.2.gb21ebb671b
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20190103/5a492b6e/attachment.sig>