GNOME Bugzilla – Bug 685544
What should we do with S3 mode
Last modified: 2016-03-31 14:00:45 UTC
With bug 685383 fixed we now at least don't break when S3 mode happens. However, in its current form S3 mode is not very useful for boxes. VMs going into S3 mode after some time idle don't actually use less resources, all that happens is that we unexpectedly disconnect from the vm. I see two possible approaches here: 1) Disable S3 mode in all boxes created VMs 2) Use the S3 mode as a trigger to boxes saying that the VM is now idle. We can then use this trigger to do something that can actually save resources, such as saving the VM to disk.
There seems to be quite some issues with S3/S4 support atm, so maybe it would be best to just disable it for boxes created guests for now.
(In reply to comment #1) > There seems to be quite some issues with S3/S4 support atm, so maybe it would > be best to just disable it for boxes created guests for now. Sure but that would need new API in libvirt-gconfig for this: http://libvirt.org/formatdomain.html#elementsPowerManagement
Created attachment 226141 [details] [review] vm-configurator: Disable S3/S4 states for new domains There seems to be some issues with S3 and S4 power state support in libvirt/qemu. Lets disable these for now in new domains.
(In reply to comment #3) > Created an attachment (id=226141) [details] [review] > vm-configurator: Disable S3/S4 states for new domains This patches requires this libvirt-glib patch: https://www.redhat.com/archives/libvir-list/2012-October/msg00377.html Although these two do the right thing and I see this in the XML created by libvirt-gconfig APIs: <pm> <suspend-to-ram enabled="no"/> <suspend-to-disk enabled="no"/> </pm> They seem to be missing from the XML configuration of the domain created for some reason. I don't get any error/warning from libvirt so I'm guessing that at least it accepts this as correct configuration.
Hmm, these seems right to me, but I'm not sure we can depend unconditionally on a new libvirt-glib in stable.
(In reply to comment #4) > Although these two do the right thing and I see this in the XML created by > libvirt-gconfig APIs: > > <pm> > <suspend-to-ram enabled="no"/> > <suspend-to-disk enabled="no"/> > </pm> > > They seem to be missing from the XML configuration of the domain created for > some reason. I don't get any error/warning from libvirt so I'm guessing that at > least it accepts this as correct configuration. And does it do the right thing in VMs? For example, no suspend on a f18 VM when it's created with this patch.
(In reply to comment #5) > Hmm, these seems right to me, but I'm not sure we can depend unconditionally on > a new libvirt-glib in stable. Same concerns as Alex.
(In reply to comment #6) > (In reply to comment #4) > > Although these two do the right thing and I see this in the XML created by > > libvirt-gconfig APIs: > > > > <pm> > > <suspend-to-ram enabled="no"/> > > <suspend-to-disk enabled="no"/> > > </pm> > > > > They seem to be missing from the XML configuration of the domain created for > > some reason. I don't get any error/warning from libvirt so I'm guessing that at > > least it accepts this as correct configuration. > > And does it do the right thing in VMs? For example, no suspend on a f18 VM when > it's created with this patch. Nope. I didn't expect it to either since what maters is the final xml configuration.
Created attachment 226200 [details] [review] vm-configurator: Disable S3/S4 states for new domains v2: Updated as underlying libvirt-glib API changed. Now that I am running latest libvirtd, this patch works as expected. i-e I do not get the 'suspend' option in F18 live VM.
I think this is fine if you add some tweaks to make it a configure time dependency. Unfortunately thats a bit tricky with vala as we want the ifdefs to work with the prebuilt shipped C code. The way you can do this is turn the ifdef into a runtime if (HAVE_PMSTATE) check. Then you set HAVE_PMSTATE to always 0 or 1 in configure. And additionally, in the !HAVE_PMSTATE case you add some defines such that the code still compiles (doing whatever, it will not run anyway). You can look at how i did this for the other pmstate stuff. With that in we do the right thing if you build with the newer libs, but if you have the older libs anyway (which basically has no s3 anyway so are fine) then things keep building. Then we can remove the hackery after we branch for 3.7.
Created attachment 226285 [details] [review] vm-configurator: Disable S3/S4 states for new domains There seems to be some issues with S3 and S4 power state support in libvirt/qemu. Lets disable these for now in new domains. This patch includes some hacks to not bump libvirt-gconfig dependency. If boxes is built against older libvirt-gconfg, the newly added code simply compiles to NOOP.
Review of attachment 226285 [details] [review]: ack
Attachment 226285 [details] pushed as 969b404 - vm-configurator: Disable S3/S4 states for new domains