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 686777 - Add an explicit pause button
Add an explicit pause button
Status: RESOLVED FIXED
Product: gnome-boxes
Classification: Applications
Component: general
unspecified
Other Linux
: Normal normal
: --
Assigned To: GNOME Boxes maintainer(s)
GNOME Boxes maintainer(s)
Depends on: 677714
Blocks: 686781
 
 
Reported: 2012-10-24 10:10 UTC by Alexander Larsson
Modified: 2016-03-31 13:54 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Change how the selection toolbar is built (1.53 KB, patch)
2012-10-25 16:55 UTC, Alexander Larsson
committed Details | Review
Add Machine.save() with libvirt implementation (1.36 KB, patch)
2012-10-25 16:55 UTC, Alexander Larsson
committed Details | Review
Add pause button to selection toolbar (1.66 KB, patch)
2012-10-25 16:55 UTC, Alexander Larsson
committed Details | Review

Description Alexander Larsson 2012-10-24 10:10:56 UTC
We need to have some way to pause a specific VM in case you want to work on some other and not unnecessarily run other VMs. Right now the only way to do this is to exit boxes to stop everything and then start it again and only activate the VMs you want running.

I'm not sure what the best place for this is though. The properties page? The collection view? both?

I guess this depends on bug 677714 in case we want it in the property page.
Comment 1 Zeeshan Ali 2012-10-25 00:33:11 UTC
(In reply to comment #0)
> We need to have some way to pause a specific VM in case you want to work on
> some other and not unnecessarily run other VMs. Right now the only way to do
> this is to exit boxes to stop everything and then start it again and only
> activate the VMs you want running.

Based on the last sentence above, you mean 'save' by 'pause'?

> I'm not sure what the best place for this is though. The properties page? The
> collection view? both?

Where in properties would it belong? A button above 'Force Shutdown'?
Comment 2 Alexander Larsson 2012-10-25 09:04:42 UTC
I guess i mean save, rather than pause as pause is a state we're otherwise not currently exposing. Not sure though.

Also not sure about the position, especially how all these minor changes interact. This needs some design work, i'll chat to jimmac about it.
Comment 3 Alexander Larsson 2012-10-25 16:55:08 UTC
Created attachment 227288 [details] [review]
Change how the selection toolbar is built

Instead of hardcoding offsets on each item we pack them in
order by appending at the end (offset=-1).

This means we don't have to change all offsets when we add
a new button.
Comment 4 Alexander Larsson 2012-10-25 16:55:11 UTC
Created attachment 227289 [details] [review]
Add Machine.save() with libvirt implementation

This will be used to explicitly save a vm.
Comment 5 Alexander Larsson 2012-10-25 16:55:14 UTC
Created attachment 227290 [details] [review]
Add pause button to selection toolbar
Comment 6 Zeeshan Ali 2012-10-26 13:00:02 UTC
Review of attachment 227288 [details] [review]:

Looks right
Comment 7 Zeeshan Ali 2012-10-26 13:01:19 UTC
Review of attachment 227289 [details] [review]:

ACK
Comment 8 Zeeshan Ali 2012-10-26 13:04:21 UTC
Review of attachment 227290 [details] [review]:

Looks good otherwise.

::: src/selectionbar.vala
@@ +49,3 @@
+               if (machine == null)
+                   continue;
+               machine.save.begin ( (obj, result) => {

This operation is usually pretty heavy and could easily take several seconds. We really want a spinner for this.

@@ +50,3 @@
+                   continue;
+               machine.save.begin ( (obj, result) => {
+                   var m = obj as Machine;

I think this lambda can access 'machine' variable from outer context fine.
Comment 9 Zeeshan Ali 2012-10-26 13:13:22 UTC
Review of attachment 227290 [details] [review]:

::: src/selectionbar.vala
@@ +49,3 @@
+               if (machine == null)
+                   continue;
+               machine.save.begin ( (obj, result) => {

After talking to you about this on IRC, I don't think we need a spinner here.
Comment 10 Alexander Larsson 2012-10-26 13:22:21 UTC
Review of attachment 227290 [details] [review]:

::: src/selectionbar.vala
@@ +50,3 @@
+                   continue;
+               machine.save.begin ( (obj, result) => {
+                   var m = obj as Machine;

True, i was afraid of it being reused in the outer block as the loop progressed, but it seems valac uses a new block for each iteration.
Comment 11 Alexander Larsson 2012-10-26 15:00:06 UTC
Attachment 227288 [details] pushed as 75358ff - Change how the selection toolbar is built
Attachment 227289 [details] pushed as 71f5b16 - Add Machine.save() with libvirt implementation
Attachment 227290 [details] pushed as a27bc03 - Add pause button to selection toolbar