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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list