[libvirt] [PATCH 06/10] Make QEMU perform managed save of all VMs on stop of libvirtd

[Daniel P. Berrange]

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