After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 698387 - Can only connect to a VM once
Can only connect to a VM once
Status: RESOLVED FIXED
Product: gnome-boxes
Classification: Applications
Component: display
3.8.x
Other Linux
: High major
: 3.22
Assigned To: GNOME Boxes maintainer(s)
GNOME Boxes maintainer(s)
: 698411 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2013-04-19 17:42 UTC by Zeeshan Ali
Modified: 2016-03-31 13:22 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
app: Only cancel in-progress connections (1.13 KB, patch)
2013-04-19 20:07 UTC, Zeeshan Ali
reviewed Details | Review
machine: Reset connecting_cancellable before connecting (1.48 KB, patch)
2013-04-23 13:42 UTC, Zeeshan Ali
committed Details | Review

Description Zeeshan Ali 2013-04-19 17:42:43 UTC
There seems to be some regression in either Boxes or spice* which makes it impossible to reconnection to a box after the first time. i-e Connect to a box, go back to collection view and then try connecting again to same box. Boxes just hangs on 'Connecting to X'.
Comment 1 Zeeshan Ali 2013-04-19 20:07:15 UTC
Created attachment 241953 [details] [review]
app: Only cancel in-progress connections

When going to collection view, we shouldn't blindly cancel machine
connection. Only do so if we are in the right UI state.

This fixes a regression from 993952e that disabled Boxes from connecting
to the same box more than once.
Comment 2 Zeeshan Ali 2013-04-19 23:30:37 UTC
*** Bug 698411 has been marked as a duplicate of this bug. ***
Comment 3 Baptiste Mille-Mathias 2013-04-20 06:25:53 UTC
Zeeshan, I change the version affected to 3.8 as the bug is found also in this version.
Comment 4 Christophe Fergeau 2013-04-23 11:57:43 UTC
Review of attachment 241953 [details] [review]:

::: src/app.vala
@@ +704,3 @@
 
+                if (previous_ui_state == UIState.CREDS)
+                    machine.connecting_cancellable.cancel (); // Cancel any in-progress connections

I still don't think it's a good idea to leak internal details of our UI state machine in many places (previous_ui_state)...

Adding machine.connecting_cancellable.reset (); in select_item() seems to do the trick as well.
Comment 5 Zeeshan Ali 2013-04-23 12:52:18 UTC
Review of attachment 241953 [details] [review]:

::: src/app.vala
@@ +704,3 @@
 
+                if (previous_ui_state == UIState.CREDS)
+                    machine.connecting_cancellable.cancel (); // Cancel any in-progress connections

I don't see any leak of internal info since:

* App is a subclass of abstract UI class (actually more of an implementation since UI would have been an interface if we could have found a way to make it so).
* previous_ui_state is publicly visible anyways. It used to be 'protected' but even if that was still true, it would be still visible in App.

Besides if UI subclasses aren't supposed to make use of this property, then who is?
Comment 6 Christophe Fergeau 2013-04-23 13:03:24 UTC
Ah right, missed the fact that App derives from UI. This still does not mean that because it happens to be exported, we have to use it whenever we can.
Comment 7 Zeeshan Ali 2013-04-23 13:15:50 UTC
(In reply to comment #6)
> Ah right, missed the fact that App derives from UI. This still does not mean
> that because it happens to be exported, we have to use it whenever we can.

Sure, I don't think this use is 'just because'. However what you said seems like a better/more-reliable approach so I'll change the patch.
Comment 8 Zeeshan Ali 2013-04-23 13:42:59 UTC
Created attachment 242224 [details] [review]
machine: Reset connecting_cancellable before connecting

This fixes a regression from 993952e that disabled Boxes from connecting
to the same box more than once.
Comment 9 Christophe Fergeau 2013-04-23 14:43:27 UTC
Review of attachment 242224 [details] [review]:

This _seems_ ok...
Comment 10 Zeeshan Ali 2013-04-23 14:48:00 UTC
Attachment 242224 [details] pushed as 3c0a19b - machine: Reset connecting_cancellable before connecting