GNOME Bugzilla – Bug 744905
USB redirection only available for active display
Last modified: 2016-09-20 08:16:03 UTC
Since the USB redirection option was added back when properties view was only available from display state (VM in foreground), the options are not available if VM is running in background. Moreover, I think USB redirection stops working (once activated) when VM is no longer in foreground. Fixes for these issues will likely help with bug#742529.
(In reply to Zeeshan Ali (Khattak) from comment #0) > Moreover, I think USB redirection > stops working (once activated) when VM is no longer in foreground. Actually I'm wrong about this last part.
In my experience the option for USB redirection is never available (it used to work, but I can't recall when). I'm using Boxes 3.18.1 on Fedora23.
in virt-viewer the USB redirection works fine
(In reply to Federico Bruni from comment #2) > In my experience the option for USB redirection is never available (it used > to work, but I can't recall when). > > I'm using Boxes 3.18.1 on Fedora23. That would be a very different issue than this bug is about. Please file a new bug.
Created attachment 328833 [details] [review] spice-display: Keep display alive if USB connected The problem is that USB redirection is currently not available when VM is in background mode. Let's keep the display alive if at least one USB device is connected.
Created attachment 328834 [details] [review] spice-display: Keep display alive if USB connected The problem is that USB redirection is currently not available when VM connected through SPICE is in background mode. Let's keep the SPICE display alive if at least one USB device is connected.
Review of attachment 328834 [details] [review]: Patch looked fine so I tried it. It works as expected until the boxe is paused (automatically or manually). At that point, display should really be disconnected. Could it be caused by bug#758840?
(In reply to Zeeshan Ali (Khattak) from comment #7) > Review of attachment 328834 [details] [review] [review]: > > Patch looked fine so I tried it. It works as expected until the boxe is > paused (automatically or manually). At that point, display should really be > disconnected. Could it be caused by bug#758840? If you open the machine in another window and then pause it from the main window, does Boxes close completely for you as well ? (disregarding this patch)
Created attachment 329111 [details] [review] machine: Disconnect paused machine's display When machine is paused, we no longer need to keep the connection with the display alive.
(In reply to Visarion Alexandru from comment #8) > (In reply to Zeeshan Ali (Khattak) from comment #7) > > Review of attachment 328834 [details] [review] [review] [review]: > > > > Patch looked fine so I tried it. It works as expected until the boxe is > > paused (automatically or manually). At that point, display should really be > > disconnected. Could it be caused by bug#758840? > > If you open the machine in another window and then pause it from the main > window, does Boxes close completely for you as well ? (disregarding this > patch) Indeed it does (without your patches).
(In reply to Zeeshan Ali (Khattak) from comment #10) > (In reply to Visarion Alexandru from comment #8) > > (In reply to Zeeshan Ali (Khattak) from comment #7) > > > Review of attachment 328834 [details] [review] [review] [review] [review]: > > > > > > Patch looked fine so I tried it. It works as expected until the boxe is > > > paused (automatically or manually). At that point, display should really be > > > disconnected. Could it be caused by bug#758840? > > > > If you open the machine in another window and then pause it from the main > > window, does Boxes close completely for you as well ? (disregarding this > > patch) > > Indeed it does (without your patches). However, I was referring to the fact that in properties view, you still get the properties even if machine is paused (and display should therefore be disconnected).
Review of attachment 329111 [details] [review]: ::: src/machine.vala @@ +124,3 @@ + if (value == MachineState.PAUSED) + disconnect_display (); Don't think this should be done here. This should not be any different from display disconnecting on machine shutting down. Check how and where that is done and that'll give you a hint on what you need to do. There is a chance that this is a spice/qemu/libvirt bug but you'll have to investigate. @@ +382,2 @@ window.display_page.remove_display (); + if (!display.should_keep_alive () || state == MachineState.PAUSED) { display.should_keep_alive() should not be returning true when machine is paused.
(In reply to Zeeshan Ali (Khattak) from comment #11) > However, I was referring to the fact that in properties view, you still get > the properties even if machine is paused (and display should therefore be > disconnected). I noticed it now as well. This happens because you have to explicitly call display.disconnect_it () for the display to disconnect, even after receiving a disconnection signal. Until now, this was done when the ui changed state to collection mode, but it is not enough anymore. The 'shutting down' case, from my tests, doesn't disconnect in the same manner as the 'pause' case. It only disconnects after viewing the properties and changing the ui state back to collection mode. Conclusion: my patch is not enough. I already did the necessary changes and I'l create a separate patch, maybe merge it with the first one.
Created attachment 330433 [details] [review] spice-display: Fix new display disconnection issues Now that we decided to keep the SPICE display connection open in ollection mode when needed, we shouldn't rely on trying to disconnect the display solely based on the UI state change, because we would ignore important disconnection cases. Let's also try to disconnect from the display when the machine is notified that a disconnection event has occured. Also, when trying to show a display, let's check if it is actually connected (and not just created to be used in showing the properties).
Review of attachment 328834 [details] [review]: actually "connected" gave me the impression that we are talking of it being connected to guest (rather than host/client). I'd use "available" term.
Review of attachment 330433 [details] [review]: * These are two (albeit small) separate changes so separate patches please. * Shortlog should summarize the change, not the effect/benefits. * shorlog is prefixed spice-display, which no changes in spice-display. * Typo in description: ollection. * when needed => in certain situations. Which reminds me, if there is no USB device, we do want to disconnect the display on going back to collection, don't we? * "because we would ignore important disconnection cases." You want to elaborate why/how it'll be ignored.
Review of attachment 330433 [details] [review]: >* when needed => in certain situations. Which reminds me, if there is no USB device, we do want to disconnect the display on going back to collection, don't we? We still try to disconnect in case the devices list has 0 length, the spice-display patch ensures this.
Created attachment 330721 [details] [review] spice-display: Keep display alive if USB available The problem is that USB redirection is currently not available when VM connected through SPICE is in background mode. Let's keep the SPICE display alive if at least one USB device is available.
Created attachment 330722 [details] [review] machine: Handle new disconnection cases Now that we decided to keep the SPICE display connection open in collection mode in certain situations, we shouldn't rely on trying to disconnect the display solely based on the UI state change, because we would ignore the case when the machine is disconnected or paused but its UI is already in collection mode.
Created attachment 330724 [details] [review] remote-machine: Connect to a non-null unconnected display The problem is that, when connecting to a display, we only connect to it if it hasn't already been created. By doing this, we ignore the case when a display is created but not connected, which is a logical scenario and can lead to a grey screen by choosing not to connect. This scenario happens already when fetching the display's properties, now that we keep the display alive if USB devices are available. To avoid this issue, let's not assume that the device is already connected just because it has been created.
Review of attachment 330433 [details] [review]: >>* when needed => in certain situations. Which reminds me, if there is no USB device, we do want to disconnect the display on going back to collection, don't we? >We still try to disconnect in case the devices list has 0 length, the spice-display patch ensures this. OK.
Attachment 330721 [details] pushed as f84c690 - spice-display: Keep display alive if USB available Attachment 330722 [details] pushed as 2fc38f4 - machine: Handle new disconnection cases Attachment 330724 [details] pushed as 6d7014c - remote-machine: Connect to a non-null unconnected display