[libvirt] [PATCH] Add APIs for killing off processes inside a cgroup

The virCgroupKill method kills all PIDs found in a cgroup The virCgroupKillRecursively method does this recursively for child cgroups. The virCgroupKillPainfully method does a recursive kill several times in a row until everything has really died --- src/libvirt_private.syms | 3 + src/util/cgroup.c | 229 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/cgroup.h | 4 + 3 files changed, 236 insertions(+), 0 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 797a670..989b3eb 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -74,6 +74,9 @@ virCgroupGetMemoryHardLimit; virCgroupGetMemorySoftLimit; virCgroupGetMemoryUsage; virCgroupGetSwapHardLimit; +virCgroupKill; +virCgroupKillRecursive; +virCgroupKillPainfully; virCgroupMounted; virCgroupRemove; virCgroupSetBlkioWeight; diff --git a/src/util/cgroup.c b/src/util/cgroup.c index b71eef9..2c7efb4 100644 --- a/src/util/cgroup.c +++ b/src/util/cgroup.c @@ -23,6 +23,7 @@ #include <stdbool.h> #include <sys/stat.h> #include <sys/types.h> +#include <signal.h> #include <libgen.h> #include <dirent.h> @@ -32,6 +33,7 @@ #include "cgroup.h" #include "logging.h" #include "files.h" +#include "hash.h" #define CGROUP_MAX_VAL 512 @@ -258,6 +260,19 @@ static int virCgroupPathOfController(virCgroupPtr group, const char *key, char **path) { + if (controller == -1) { + int i; + for (i = 0 ; i < VIR_CGROUP_CONTROLLER_LAST ; i++) { + if (group->controllers[i].mountPoint && + group->controllers[i].placement) { + controller = i; + break; + } + } + } + if (controller == -1) + return -ENOSYS; + if (group->controllers[controller].mountPoint == NULL) return -ENOENT; @@ -1291,3 +1306,217 @@ int virCgroupGetFreezerState(virCgroupPtr group, char **state) VIR_CGROUP_CONTROLLER_CPU, "freezer.state", state); } + +static int virCgroupKillInternal(virCgroupPtr group, int signum, virHashTablePtr pids) +{ + int rc; + int killedAny = 0; + char *keypath = NULL; + bool done = false; + VIR_DEBUG("group=%p path=%s signum=%d pids=%p", group, group->path, signum, pids); + + rc = virCgroupPathOfController(group, -1, "tasks", &keypath); + if (rc != 0) { + VIR_DEBUG("No path of %s, tasks", group->path); + return rc; + } + + /* PIDs may be forking as we kill them, so loop + * until there are no new PIDs found + */ + while (!done) { + done = true; + FILE *fp; + if (!(fp = fopen(keypath, "r"))) { + rc = -errno; + VIR_DEBUG("Failed to read %s: %m\n", keypath); + goto cleanup; + } else { + while (!feof(fp)) { + unsigned long pid; + if (fscanf(fp, "%lu", &pid) != 1) { + if (feof(fp)) + break; + rc = -errno; + break; + } + if (virHashLookup(pids, (void*)pid)) + continue; + + VIR_DEBUG("pid=%lu", pid); + if (kill((pid_t)pid, signum) < 0) { + if (errno != ESRCH) { + rc = -errno; + goto cleanup; + } + /* Leave RC == 0 since we didn't kill one */ + } else { + killedAny = 1; + done = false; + } + + virHashAddEntry(pids, (void*)pid, (void*)1); + } + fclose(fp); + } + } + + rc = killedAny ? 1 : 0; + +cleanup: + VIR_FREE(keypath); + + return rc; +} + + +static unsigned long virCgroupPidCode(const void *name) +{ + return (unsigned long)name; +} +static bool virCgroupPidEqual(const void *namea, const void *nameb) +{ + return namea == nameb; +} +static void *virCgroupPidCopy(const void *name) +{ + return (void*)name; +} + +/* + * Returns + * < 0 : errno that occurred + * 0 : no PIDs killed + * 1 : at least one PID killed + */ +int virCgroupKill(virCgroupPtr group, int signum) +{ + VIR_DEBUG("group=%p path=%s signum=%d", group, group->path, signum); + int rc; + /* The 'tasks' file in cgroups can contain duplicated + * pids, so we use a hash to track which we've already + * killed. + */ + virHashTablePtr pids = virHashCreateFull(100, + NULL, + virCgroupPidCode, + virCgroupPidEqual, + virCgroupPidCopy, + NULL); + + rc = virCgroupKillInternal(group, signum, pids); + + virHashFree(pids); + + return rc; +} + + +static int virCgroupKillRecursiveInternal(virCgroupPtr group, int signum, virHashTablePtr pids, bool dormdir) +{ + int rc; + int killedAny = 0; + char *keypath = NULL; + DIR *dp; + virCgroupPtr subgroup = NULL; + struct dirent *ent; + VIR_DEBUG("group=%p path=%s signum=%d pids=%p", group, group->path, signum, pids); + + rc = virCgroupPathOfController(group, -1, "", &keypath); + if (rc != 0) { + VIR_DEBUG("No path of %s, tasks", group->path); + return rc; + } + + if ((rc = virCgroupKillInternal(group, signum, pids)) != 0) + return rc; + + VIR_DEBUG("Iterate over children of %s", keypath); + if (!(dp = opendir(keypath))) { + rc = -errno; + return rc; + } + + while ((ent = readdir(dp))) { + char *subpath; + + if (STREQ(ent->d_name, ".")) + continue; + if (STREQ(ent->d_name, "..")) + continue; + if (ent->d_type != DT_DIR) + continue; + + VIR_DEBUG("Process subdir %s", ent->d_name); + if (virAsprintf(&subpath, "%s/%s", group->path, ent->d_name) < 0) { + rc = -ENOMEM; + goto cleanup; + } + + if ((rc = virCgroupNew(subpath, &subgroup)) != 0) + goto cleanup; + + if ((rc = virCgroupKillRecursiveInternal(subgroup, signum, pids, true)) < 0) + goto cleanup; + if (rc == 1) + killedAny = 1; + + if (dormdir) + virCgroupRemove(subgroup); + + virCgroupFree(&subgroup); + } + + rc = killedAny; + +cleanup: + virCgroupFree(&subgroup); + closedir(dp); + + return rc; +} + +int virCgroupKillRecursive(virCgroupPtr group, int signum) +{ + int rc; + VIR_DEBUG("group=%p path=%s signum=%d", group, group->path, signum); + virHashTablePtr pids = virHashCreateFull(100, + NULL, + virCgroupPidCode, + virCgroupPidEqual, + virCgroupPidCopy, + NULL); + + rc = virCgroupKillRecursiveInternal(group, signum, pids, false); + + virHashFree(pids); + + return rc; +} + + +int virCgroupKillPainfully(virCgroupPtr group) +{ + int i; + int rc; + VIR_DEBUG("cgroup=%p path=%s", group, group->path); + for (i = 0 ; i < 15 ; i++) { + int signum; + if (i == 0) + signum = SIGTERM; + else if (i == 8) + signum = SIGKILL; + else + signum = 0; /* Just check for existance */ + + rc = virCgroupKillRecursive(group, signum); + VIR_DEBUG("Iteration %d rc=%d", i, rc); + /* If rc == -1 we hit error, if 0 we ran out of PIDs */ + if (rc <= 0) + break; + + usleep(200 * 1000); + } + VIR_DEBUG("Complete %d", rc); + return rc; +} diff --git a/src/util/cgroup.h b/src/util/cgroup.h index f1bdd0f..d468cb3 100644 --- a/src/util/cgroup.h +++ b/src/util/cgroup.h @@ -90,4 +90,8 @@ int virCgroupRemove(virCgroupPtr group); void virCgroupFree(virCgroupPtr *group); bool virCgroupMounted(virCgroupPtr cgroup, int controller); +int virCgroupKill(virCgroupPtr group, int signum); +int virCgroupKillRecursive(virCgroupPtr group, int signum); +int virCgroupKillPainfully(virCgroupPtr group); + #endif /* CGROUP_H */ -- 1.7.4

On 02/22/2011 10:55 AM, Daniel P. Berrange wrote:
The virCgroupKill method kills all PIDs found in a cgroup
The virCgroupKillRecursively method does this recursively for child cgroups.
The virCgroupKillPainfully method does a recursive kill several times in a row until everything has really died
Do we really need three APIs, or can we just have: virCgroupKill(..., VIR_CGROUP_KILL_RECURSIVE | VIR_CGROUP_KILL_PAINFUL) I haven't reviewed the body of the patch, yet. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On Tue, Feb 22, 2011 at 11:17:05AM -0700, Eric Blake wrote:
On 02/22/2011 10:55 AM, Daniel P. Berrange wrote:
The virCgroupKill method kills all PIDs found in a cgroup
The virCgroupKillRecursively method does this recursively for child cgroups.
The virCgroupKillPainfully method does a recursive kill several times in a row until everything has really died
Do we really need three APIs, or can we just have:
virCgroupKill(..., VIR_CGROUP_KILL_RECURSIVE | VIR_CGROUP_KILL_PAINFUL)
I haven't reviewed the body of the patch, yet.
Agreed, I was surpized by this at first. Went though the patch my only remarks are rc = killedAny ? 1 : 0; is overkill since killedAny values seems only 0 or 1 anyway and the loop for virCgroupKillPainfully looks a bit arbitrary, sending 15 signals to all looks a lot, and the delay for the loop a bit short, if there is like a hundred processes in that group that could be a bit heavy, isn't it ? Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On Wed, Feb 23, 2011 at 03:26:30PM +0800, Daniel Veillard wrote:
On Tue, Feb 22, 2011 at 11:17:05AM -0700, Eric Blake wrote:
On 02/22/2011 10:55 AM, Daniel P. Berrange wrote:
The virCgroupKill method kills all PIDs found in a cgroup
The virCgroupKillRecursively method does this recursively for child cgroups.
The virCgroupKillPainfully method does a recursive kill several times in a row until everything has really died
Do we really need three APIs, or can we just have:
virCgroupKill(..., VIR_CGROUP_KILL_RECURSIVE | VIR_CGROUP_KILL_PAINFUL)
I haven't reviewed the body of the patch, yet.
Agreed, I was surpized by this at first.
Went though the patch my only remarks are rc = killedAny ? 1 : 0; is overkill since killedAny values seems only 0 or 1 anyway and the loop for virCgroupKillPainfully looks a bit arbitrary, sending 15 signals to all looks a lot, and the delay for the loop a bit short, if there is like a hundred processes in that group that could be a bit heavy, isn't it ?
The apporach & timings I'm following here are the same as that used in systemd, and the only way a process could not die after the SIGKILL is if the kernel had it in an uninterruptable sleep. So I think this is sufficient for our needs - it certainly is dramatically more reliable than the code it is replacing in LXC :-) 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 :|

On Wed, Feb 23, 2011 at 11:27:06AM +0000, Daniel P. Berrange wrote:
On Wed, Feb 23, 2011 at 03:26:30PM +0800, Daniel Veillard wrote:
On Tue, Feb 22, 2011 at 11:17:05AM -0700, Eric Blake wrote:
On 02/22/2011 10:55 AM, Daniel P. Berrange wrote:
The virCgroupKill method kills all PIDs found in a cgroup
The virCgroupKillRecursively method does this recursively for child cgroups.
The virCgroupKillPainfully method does a recursive kill several times in a row until everything has really died
Do we really need three APIs, or can we just have:
virCgroupKill(..., VIR_CGROUP_KILL_RECURSIVE | VIR_CGROUP_KILL_PAINFUL)
I haven't reviewed the body of the patch, yet.
Agreed, I was surpized by this at first.
Went though the patch my only remarks are rc = killedAny ? 1 : 0; is overkill since killedAny values seems only 0 or 1 anyway and the loop for virCgroupKillPainfully looks a bit arbitrary, sending 15 signals to all looks a lot, and the delay for the loop a bit short, if there is like a hundred processes in that group that could be a bit heavy, isn't it ?
The apporach & timings I'm following here are the same as that used in systemd, and the only way a process could not die after the SIGKILL is if the kernel had it in an uninterruptable sleep. So I think this is sufficient for our needs - it certainly is dramatically more reliable than the code it is replacing in LXC :-)
okay, okay :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On Tue, Feb 22, 2011 at 11:17:05AM -0700, Eric Blake wrote:
On 02/22/2011 10:55 AM, Daniel P. Berrange wrote:
The virCgroupKill method kills all PIDs found in a cgroup
The virCgroupKillRecursively method does this recursively for child cgroups.
The virCgroupKillPainfully method does a recursive kill several times in a row until everything has really died
Do we really need three APIs, or can we just have:
virCgroupKill(..., VIR_CGROUP_KILL_RECURSIVE | VIR_CGROUP_KILL_PAINFUL)
The last method loops, calling into the 2nd method, and the 2nd method loops, calling into the 1st, so architecturally it doesn't work to have only one method with flags. 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 :|

On 02/23/2011 04:23 AM, Daniel P. Berrange wrote:
On Tue, Feb 22, 2011 at 11:17:05AM -0700, Eric Blake wrote:
On 02/22/2011 10:55 AM, Daniel P. Berrange wrote:
The virCgroupKill method kills all PIDs found in a cgroup
The virCgroupKillRecursively method does this recursively for child cgroups.
The virCgroupKillPainfully method does a recursive kill several times in a row until everything has really died
Do we really need three APIs, or can we just have:
virCgroupKill(..., VIR_CGROUP_KILL_RECURSIVE | VIR_CGROUP_KILL_PAINFUL)
The last method loops, calling into the 2nd method, and the 2nd method loops, calling into the 1st, so architecturally it doesn't work to have only one method with flags.
So under the hood it's easier to implement with three functions. But still, from the user's point of view (including remote rpcs), why not have just one function with flags? static int virCgroupKillOne(...) { ... } static int virCgroupKillRecursive(...) { loop on virCgroupKillOne() } static int virCgroupKillPainful(...) { loop on virCgroupKillRecursive() } extern int virCgroupKill(..., flags) { switch (flags) { case 0: virCgroupKillOne(); break; case KILL_RECURSIVE: virCgroupKillRecursive(); break; case KILL_PAINFUL: virCgroupKillPainful(); break; } } -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On Wed, Feb 23, 2011 at 07:58:36AM -0700, Eric Blake wrote:
On 02/23/2011 04:23 AM, Daniel P. Berrange wrote:
On Tue, Feb 22, 2011 at 11:17:05AM -0700, Eric Blake wrote:
On 02/22/2011 10:55 AM, Daniel P. Berrange wrote:
The virCgroupKill method kills all PIDs found in a cgroup
The virCgroupKillRecursively method does this recursively for child cgroups.
The virCgroupKillPainfully method does a recursive kill several times in a row until everything has really died
Do we really need three APIs, or can we just have:
virCgroupKill(..., VIR_CGROUP_KILL_RECURSIVE | VIR_CGROUP_KILL_PAINFUL)
The last method loops, calling into the 2nd method, and the 2nd method loops, calling into the 1st, so architecturally it doesn't work to have only one method with flags.
So under the hood it's easier to implement with three functions. But still, from the user's point of view (including remote rpcs), why not have just one function with flags?
This isn't a public API. Its just an internal thing, so there's no reason not to restrict visibility of them. 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 :|
participants (3)
-
Daniel P. Berrange
-
Daniel Veillard
-
Eric Blake