Re: [libvirt] [PATCH 06/10] Make QEMU perform managed save of all VMs on stop of libvirtd
On Tue, Nov 27, 2012 at 03:06:45PM -0500, Eric Blake wrote:
> From: "Daniel P. Berrange"
<berrange(a)redhat.com>
>
> When the virStateStop() method is invoked, perform a managed
> save of all VMs currently running
>
> Signed-off-by: Daniel P. Berrange <berrange(a)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 :|