> > Hi, > > On Wed, Apr 25, 2018 at 04:05:00AM -0400, Frediano Ziglio wrote: > > > > > > Ping > > > > > > > Looking back at the comments looks like this new set is still > > not doing the right thing and you agree, which is confusing to > > me. > > Well, spice-session holds the reference to SpiceUsbDeviceManager > and has API to give that reference to different > SpiceUsbredirChannel so I do think it should be fine to > initialize it which solves that rhbz#1399838 bug. > > But while doing that, regression on migration, rhbz#1558043, > which is not easy to solve while keeping that patch. > > > It's replacing a crash with a critical warning ignoring the > > problem. Well... maybe this is actually a progress. > > Indeed, I would rather not have the regression while replacing a > crash with a critical, yes. > > > Maybe if you have an idea how it should behave at the end after > > removing the critical opening a new bug and stating in the new > > commit would be more clear. > > > > Would be good also to have an idea of the different cases we > > should handle. > > Besides avoiding regression... SpiceUsbDeviceManager is only > initialized on UI objects in spice-gtk. That should be fixed at > some point as non gtk tools like spicy-stats might do usb > redirection. > > If this patches seems like good progress, I'll file a bug to fix > the critical that this patch set introduces, yes :) > If you propose a solution with a "if the patches seems like good progress" looks like you are not sure about it. You should propose good patches :-) > Cheers, > toso > > > > On Fri, Apr 06, 2018 at 09:59:43AM +0200, Victor Toso wrote: > > > > From: Victor Toso <me@xxxxxxxxxxxxxx> > > > > > > > > This reverts commit 291f3e4419e6fb4077ae43a5e09eb1c37b9dd729 > > > > > > > > Follow up patch should address rhbz#1399838 mentioned in commit above > > > > in a different way. > > > > > > > > Major reason to revert is that the SpiceUsbDeviceManager object is > > > > kept in SpiceSession as an easy way to share it between different > > > > SpiceUsbredirChannel while SpiceSession itself does not use it. This > > > > causes problems on migration as we start a new session with a new > > > > SpiceUsbDeviceManager object while the previous one still exists. > > > > Maybe SpiceUsbDeviceManager should be a global (singleton) meaning that you should not create a new object but just add a reference to it even from another SpiceSession? > > > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1558043 > > > > --- > > > > src/spice-session.c | 7 ------- > > > > 1 file changed, 7 deletions(-) > > > > > > > > diff --git a/src/spice-session.c b/src/spice-session.c > > > > index 57acc63..f856244 100644 > > > > --- a/src/spice-session.c > > > > +++ b/src/spice-session.c > > > > @@ -280,7 +280,6 @@ static void spice_session_init(SpiceSession > > > > *session) > > > > { > > > > SpiceSessionPrivate *s; > > > > gchar *channels; > > > > - GError *err = NULL; > > > > > > > > SPICE_DEBUG("New session (compiled from package " PACKAGE_STRING > > > > ")"); > > > > s = session->priv = SPICE_SESSION_GET_PRIVATE(session); > > > > @@ -293,12 +292,6 @@ static void spice_session_init(SpiceSession > > > > *session) > > > > s->images = cache_image_new((GDestroyNotify)pixman_image_unref); > > > > s->glz_window = glz_decoder_window_new(); > > > > update_proxy(session, NULL); > > > > - > > > > - s->usb_manager = spice_usb_device_manager_get(session, &err); > > > > - if (err != NULL) { > > > > - SPICE_DEBUG("Could not initialize SpiceUsbDeviceManager - %s", > > > > err->message); > > > > - g_clear_error(&err); > > > > - } > > > > } > > > > > > > > static void > > > > Frediano > _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel