On Tue, Nov 27, 2012 at 03:06:45PM -0500, Eric Blake wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
When the virStateStop() method is invoked, perform a managed save of all VMs currently running
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_driver.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+)
How does this interact with the libvirt-guests init script? I guess that if that is installed, it gets to run first before the system libvirtd is shutdown, so the qemu:///system won't see any running guests, and this patch would just affect qemu:///session?
In theory it can do both qemu:///system and qemu:///session, however, the later patches only wire this up for qemu:///session, precisely because libvirt-guests already exists.
Is managed save always the right thing, or should we allow the user to configure for a graceful shutdown and/or a migration to other host? Do we need timeouts, as managed save may take a while per guest? Similar to how /etc/sysconfig/libvirt-guests can request O_DIRECT to avoid file system pollution, do we need a way for the user to configure the use of that flag? Is doing the same thing for all guests wise, or do we need to allow for the possibility of a per-guest choice of what action to take (maybe via a new <on_host_shutdown> element?) where we only fall back to the configured default if the guest XML didn't request something? Do we correctly and automatically restart all the guests that were saved by this hook the next time libvirtd restarts?
We don't attempt to automatically restart all guests - we will only restart those marked as "autostart", which I think is the right thing todo in general. I think managed save is the only reasonable choice here, because we want something that is going to be reliable, and zero-conf for the user. Migration requires the user to pick a dest host, and is not guaranteed to converge. Graceful shutdown relies on a co-operating guest. So IMHO, neither of those are really satisfactory options for running on desktop logout / host shutdown. Not sure about O_DIRECT - I'm inclined to say we should just *always* use O_DIRECT - unless someone can point out a downside with it ? Long term, I make no secret of the fact that I want to see libvirt-guests die in horribly painful way. This kind of functionality should be brought "in house" to libvirtd and obviously this new code is a start in that direction.
+ /* First we pause all VMs to make them stop dirtying + pages, etc. We remember if any VMs were paused so + we can restore that on resume. */ + for (i = 0 ; i < numDomains ; i++) { + flags[i] = VIR_DOMAIN_SAVE_RUNNING; + if (virDomainGetState(domains[i], &state, NULL, 0) == 0) { + if (state == VIR_DOMAIN_PAUSED) { + flags[i] = VIR_DOMAIN_SAVE_PAUSED; + } + } + virDomainSuspend(domains[i]);
Should you be checking for errors here?
Yes & no. Pausing the guests is just a performance tweak, so if it fails it isn't critical. If pausing fails, chances are the managed save willl fail too, so we'll likely get an erorr regardless.
+ } + + ret = 0; + /* Then we save the VMs to disk */ + for (i = 0 ; i < numDomains ; i++) + if (virDomainManagedSave(domains[i], flags[i]) < 0) + ret = -1;
What happens if a guest managed to exit itself after the initial virConnectListAllDomains() and this point? The virDomainManagedSave will fail, but that is not an error since a stopped guest has nothing to save after all. Do you need to be checking specific error codes here before treating this as a failure?
I guess we could handle that - though the caller will ignore all the errors anyway :-) Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|