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 688980 - A few machine saving/pausing related fixes/improvements
A few machine saving/pausing related fixes/improvements
Status: RESOLVED FIXED
Product: gnome-boxes
Classification: Applications
Component: general
unspecified
Other All
: Normal normal
: --
Assigned To: GNOME Boxes maintainer(s)
GNOME Boxes maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2012-11-24 17:22 UTC by Zeeshan Ali
Modified: 2016-03-31 13:54 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
machine: Add 'can_save' property (1.22 KB, patch)
2012-11-24 17:22 UTC, Zeeshan Ali
committed Details | Review
selectionbar: Disable 'pause' button for saved machines (1.49 KB, patch)
2012-11-24 17:22 UTC, Zeeshan Ali
committed Details | Review
selectionbar: Disable 'pause' button once clicked (831 bytes, patch)
2012-11-24 17:22 UTC, Zeeshan Ali
committed Details | Review
machine: Display "Saving..." status (1.98 KB, patch)
2012-11-24 17:22 UTC, Zeeshan Ali
committed Details | Review
machine: Don't save already saved machines (934 bytes, patch)
2012-11-24 17:22 UTC, Zeeshan Ali
committed Details | Review
app: Don't quit UI before saving machines (2.45 KB, patch)
2012-11-24 17:22 UTC, Zeeshan Ali
rejected Details | Review

Description Zeeshan Ali 2012-11-24 17:22:20 UTC
Here are a few patches that I think improves user experience w.r.t pausing/saving of VMs.

There is a bit of confusion on the terms 'save' and 'pause' in the code and now in the UI as well. IMO, 'pause' operation typically implies stopping something temporarily and immediately, and be able to unpause/play from that state immediately. Since that is not the case with 'save', I am not entirely convinced of hiding the concept of saving from user.

This is something I think designers should answer for us though.
Comment 1 Zeeshan Ali 2012-11-24 17:22:22 UTC
Created attachment 229768 [details] [review]
machine: Add 'can_save' property

Add a boolean property to indicate if machine can save itself or not.
Comment 2 Zeeshan Ali 2012-11-24 17:22:25 UTC
Created attachment 229769 [details] [review]
selectionbar: Disable 'pause' button for saved machines

The 'pause' button should only be enabled if any of the selected
machines support saving and is not already in saved state.
Comment 3 Zeeshan Ali 2012-11-24 17:22:27 UTC
Created attachment 229770 [details] [review]
selectionbar: Disable 'pause' button once clicked

Once user hits the 'pause' button, there is no need for this button to
remain enabled anymore unless user changes selection.
Comment 4 Zeeshan Ali 2012-11-24 17:22:30 UTC
Created attachment 229771 [details] [review]
machine: Display "Saving..." status

When a machine(s) is being saved, it can easily take several seconds so
it would be nice to provide some visual indication about whats going on.
Comment 5 Zeeshan Ali 2012-11-24 17:22:33 UTC
Created attachment 229772 [details] [review]
machine: Don't save already saved machines

Otherwise we get an error.
Comment 6 Zeeshan Ali 2012-11-24 17:22:36 UTC
Created attachment 229773 [details] [review]
app: Don't quit UI before saving machines

Now that we show indication for machines being saved, I think its better
to keep the UI alive while we are saving. A zombie app does not help user
much IMO, especially if it remains a zombie for a long time (which can
easily happen when saving multiple machines on a not so powerful host).
Comment 7 Christophe Fergeau 2012-11-26 09:39:57 UTC
(In reply to comment #6)
> Created an attachment (id=229773) [details] [review]
> app: Don't quit UI before saving machines
> 
> Now that we show indication for machines being saved, I think its better
> to keep the UI alive while we are saving. A zombie app does not help user
> much IMO, especially if it remains a zombie for a long time (which can
> easily happen when saving multiple machines on a not so powerful host).

Not really convinced about that one, this means you have a window staying around with which you cannot really interact, you can only stare at it until it finally goes away.
Comment 8 Zeeshan Ali 2012-11-26 15:02:25 UTC
(In reply to comment #7)
> (In reply to comment #6)
> > Created an attachment (id=229773) [details] [review] [details] [review]
> > app: Don't quit UI before saving machines
> > 
> > Now that we show indication for machines being saved, I think its better
> > to keep the UI alive while we are saving. A zombie app does not help user
> > much IMO, especially if it remains a zombie for a long time (which can
> > easily happen when saving multiple machines on a not so powerful host).
> 
> Not really convinced about that one, this means you have a window staying
> around with which you cannot really interact, you can only stare at it until it
> finally goes away.

You don't need to stare at it, you can switch to other windows/apps. :) We might want to disable user interactions somehow though. Perhaps we need some different UI here or just make all widgets insensitive is enough. This would be something for designers to suggest something I guess.
Comment 9 Alexander Larsson 2012-11-26 16:09:37 UTC
Yeah, i agree with everything except "showing a window just to show a progress bar".
Comment 10 Alexander Larsson 2012-11-26 16:10:04 UTC
Review of attachment 229768 [details] [review]:

ack
Comment 11 Alexander Larsson 2012-11-26 16:10:08 UTC
Review of attachment 229769 [details] [review]:

ack
Comment 12 Alexander Larsson 2012-11-26 16:10:14 UTC
Review of attachment 229770 [details] [review]:

ack
Comment 13 Alexander Larsson 2012-11-26 16:10:19 UTC
Review of attachment 229771 [details] [review]:

ack
Comment 14 Alexander Larsson 2012-11-26 16:10:23 UTC
Review of attachment 229772 [details] [review]:

ack
Comment 15 Zeeshan Ali 2012-11-26 16:38:57 UTC
Attachment 229768 [details] pushed as eed7ee2 - machine: Add 'can_save' property
Attachment 229769 [details] pushed as 8903be8 - selectionbar: Disable 'pause' button for saved machines
Attachment 229770 [details] pushed as 70874c1 - selectionbar: Disable 'pause' button once clicked
Attachment 229771 [details] pushed as eb4c632 - machine: Display "Saving..." status
Attachment 229772 [details] pushed as de8cc2b - machine: Don't save already saved machines
Comment 16 Jakub Steiner 2012-11-29 19:20:49 UTC
I think exposing the distinction between pausing and saving would be a failure from the UI point of view. When we discussed this with Zeeshan on IRC I made a parallel to the suspend to RAM vs hibernate to disk etudes we had for the OS*.

One of the questions was whether to have some UI for when we are flushing to disk on quit when that action can potentially take a while (10seconds +). I don't even think we should be explicitly asking the user to pause a box. Saving state can be completely transparent. Why not default to only keep one running VM per Boxes window. Transparently save in the background everything that's not showing up. Allow to explicitly keep a box running in the background of course in the properties, but the default would be to suspend. There may be some more exceptions (running installation comes to mind). 

Remembering Evolution, I don't think exposing the saving on quit is a wise choice. Do we have some system policies that would kill boxes when shutting down for example?

* http://fedoraproject.org/wiki/Desktop/Whiteboards/HybridSuspend
Comment 17 Alexander Larsson 2013-01-11 10:46:32 UTC
I worked on patches to automatically have libvirt suspend the VMs on logout/shutdown, so we do that properly now.
Comment 18 Zeeshan Ali 2013-03-01 12:58:26 UTC
The idea in comment#16 has been put into its own bug now (bug#694931) so we can close this as fixed since all essential patches have been merged.