[libvirt] [PATCH] LXC: workaround machined uncleaned data with containers running systemd.

[Daniel P. Berrange]

On Fri, Sep 27, 2013 at 05:25:55PM +0200, Cédric Bosdonnat wrote:
> The problem is described by [0] but its effect on libvirt is that
> starting a container with a full distro running systemd after having
> stopped it simply fails.
> 
> The container cleanup now calls the machined Terminate function to make
> sure that everything is in order for the next run.
> 
>  [0]: https://bugs.freedesktop.org/show_bug.cgi?id=68370
> ---
>  src/Makefile.am       |  4 +++-
>  src/lxc/lxc_process.c |  8 ++++++++
>  src/util/virsystemd.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/util/virsystemd.h |  4 ++++
>  4 files changed, 70 insertions(+), 1 deletion(-)
> 
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 4375ef7..211b42e 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -1170,7 +1170,9 @@ EXTRA_DIST += qemu/qemu.conf qemu/libvirtd_qemu.aug \
>  if WITH_LXC
>  noinst_LTLIBRARIES += libvirt_driver_lxc_impl.la
>  libvirt_driver_lxc_la_SOURCES =
> -libvirt_driver_lxc_la_LIBADD = libvirt_driver_lxc_impl.la
> +libvirt_driver_lxc_la_LIBADD = \
> +		libvirt_driver_lxc_impl.la \
> +		libvirt_util.la

This is bogus - you should be adding the new symbol to libvirt_private.syms
to make it accessible.

> diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
> index 4835bd5..f92c613 100644
> --- a/src/lxc/lxc_process.c
> +++ b/src/lxc/lxc_process.c
> @@ -50,6 +50,7 @@
>  #include "virstring.h"
>  #include "viratomic.h"
>  #include "virprocess.h"
> +#include "virsystemd.h"
>  
>  #define VIR_FROM_THIS VIR_FROM_LXC
>  
> @@ -210,6 +211,13 @@ static void virLXCProcessCleanup(virLXCDriverPtr driver,
>          virCgroupFree(&priv->cgroup);
>      }
>  
> +    /* Get machined to terminate the machine as it may not have cleaned it
> +     * properly. See https://bugs.freedesktop.org/show_bug.cgi?id=68370 for
> +     * the bug we are working around here.
> +     */
> +    virSystemdTerminateMachine(vm->def->name, "lxc", true);
> +
> +
>      /* now that we know it's stopped call the hook if present */
>      if (virHookPresent(VIR_HOOK_DRIVER_LXC)) {
>          char *xml = virDomainDefFormat(vm->def, 0);
> diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
> index e72b7f0..3320e53 100644
> --- a/src/util/virsystemd.c
> +++ b/src/util/virsystemd.c
> @@ -242,3 +242,58 @@ cleanup:
>      VIR_FREE(slicename);
>      return ret;
>  }
> +
> +int virSystemdTerminateMachine(const char *name,
> +                               const char *drivername,
> +                               bool privileged)
> +{
> +    int ret;
> +    DBusConnection *conn;
> +    char *machinename = NULL;
> +    char *username = NULL;
> +
> +    ret = virDBusIsServiceEnabled("org.freedesktop.machine1");
> +    if (ret < 0)
> +        return ret;
> +
> +    conn = virDBusGetSystemBus();
> +
> +    ret = -1;
> +    if (privileged) {
> +        if (virAsprintf(&machinename, "%s-%s", drivername, name) < 0)
> +            goto cleanup;
> +    } else {
> +        if (!(username = virGetUserName(geteuid())))
> +            goto cleanup;
> +        if (virAsprintf(&machinename, "%s-%s-%s", username, drivername, name) < 0)
> +            goto cleanup;
> +    }

This block of code is duplicated with virSystemdCreateMachine and should
be split into a shared helper method

> +
> +    /*
> +     * The systemd DBus API we're invoking has the
> +     * following signature
> +     *
> +     * TerminateMachine(in  s name);
> +     *
> +     * @name a host unique name for the machine. shows up
> +     * in 'ps' listing & similar
> +     */
> +
> +    VIR_DEBUG("Attempting to terminate machine via systemd");
> +    if (virDBusCallMethod(conn,
> +                          NULL,
> +                          "org.freedesktop.machine1",
> +                          "/org/freedesktop/machine1",
> +                          "org.freedesktop.machine1.Manager",
> +                          "TerminateMachine",
> +                          "s",
> +                          machinename) < 0)
> +        goto cleanup;
> +
> +    ret = 0;
> +
> +cleanup:
> +    VIR_FREE(username);
> +    VIR_FREE(machinename);
> +    return ret;
> +}
> diff --git a/src/util/virsystemd.h b/src/util/virsystemd.h
> index 414ae5a..6c9c6df 100644
> --- a/src/util/virsystemd.h
> +++ b/src/util/virsystemd.h
> @@ -38,4 +38,8 @@ int virSystemdCreateMachine(const char *name,
>                              bool iscontainer,
>                              const char *partition);
>  
> +int virSystemdTerminateMachine(const char *name,
> +                               const char *drivername,
> +                               bool privileged);
> +
>  #endif /* __VIR_SYSTEMD_H__ */

Can you also extend tests/virsystemdtest.c to cover this new API.


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