[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 3/3] Add new libvirt-babysitter tool

On Mon, Oct 08, 2012 at 04:57:49PM +0200, Alexander Larsson wrote:
> This helper registers with the session bus and libvirt and ensures
> that we properly save the state when the session exits. It does this
> in two ways:
> 1) Whenever the session dbus connection is disconnected (typically due
> to a logout) we save all domains in the session libvirtd.
> 2) Whenever there is a active domain in the session libvirtd we
> take a shutdown inhibition lock[1] which means that systemd will
> delay shutdown until we saved any active VMs. We then save the VM
> when we get a PrepareForShutdown event (or when the session dies as
> in 1).
> [1] http://www.freedesktop.org/wiki/Software/systemd/inhibit

In general I think this is a neat idea. We currently have a nasty
shell script that we trigger on host shutdown to save/migrate/etc
VMs. Although it is not as featureful, I like that this code is
using our APIs instead of shell code.

My primary feedback would be that this could actually be run
inside libvirtd itself.

In the libvirtd daemonRunStateInit method you can do the
initial dbus signal setup and libvirt lifecycle event
registration just after virStateInitialize completes. The
only slightly different bit is that you will need to run
the saveAllDomains() method in a separate thread so that
you don't block the main libvirt event loop which will be
required during the VM shutdown process.

You could also then directly tell libvirtd to exit.

> +static void saveAllDomains(virConnectPtr conn)
> +{
> +    virDomainPtr dom;
> +    int numDomains, i;
> +    int *domains;
> +    int state;
> +    unsigned int *flags;
> +
> +    numDomains = virConnectNumOfDomains(conn);
> +    if (numDomains < 0)
> +        return;
> +    
> +    if (VIR_ALLOC_N(domains, numDomains) < 0) {
> +        virReportOOMError();
> +        return;
> +    }
> +
> +    if (VIR_ALLOC_N(flags, numDomains) < 0) {
> +        VIR_FREE (domains);
> +        virReportOOMError();
> +        return;
> +    }
> +
> +    numDomains = virConnectListDomains(conn, domains, numDomains);

As a slight optimization you can use new virConnectListAllDomains
which returns you the full array of virDomainPtr objects straight

Oh, and obviously for now we'd only want to have this auto-shutdown
code be done for the unprivileged session libvirtd.  We'll punt on
handling the systemd libvirtd for another day, when someone wants
to attack the shell script

|: 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 :|

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]