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

[libvirt] [PATCH 2/3] Convert the virCgroupKill* APIs to report errors



From: "Daniel P. Berrange" <berrange redhat com>

Instead of returning errno values, change the virCgroupKill*
APIs to fully report errors.

Signed-off-by: Daniel P. Berrange <berrange redhat com>
---
 src/lxc/lxc_process.c |  15 +------
 src/util/vircgroup.c  | 108 ++++++++++++++++++++++++++++----------------------
 2 files changed, 62 insertions(+), 61 deletions(-)

diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 54eb728..b7d74a7 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -769,19 +769,7 @@ int virLXCProcessStop(virLXCDriverPtr driver,
     }
 
     if (priv->cgroup) {
-        rc = virCgroupKillPainfully(priv->cgroup);
-        if (rc < 0) {
-            virReportSystemError(-rc, "%s",
-                                 _("Failed to kill container PIDs"));
-            rc = -1;
-            goto cleanup;
-        }
-        if (rc == 1) {
-            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                           _("Some container PIDs refused to die"));
-            rc = -1;
-            goto cleanup;
-        }
+        virCgroupKillPainfully(priv->cgroup);
     } else {
         /* If cgroup doesn't exist, the VM pids must have already
          * died and so we're just cleaning up stale state
@@ -792,7 +780,6 @@ int virLXCProcessStop(virLXCDriverPtr driver,
 
     rc = 0;
 
-cleanup:
     return rc;
 }
 
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 12ace2e..fb76a13 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -2318,9 +2318,12 @@ int virCgroupGetFreezerState(virCgroupPtr group, char **state)
 
 
 #if defined HAVE_KILL && defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R
+/*
+ * Returns 1 if some PIDs kill, 0 if non are killed, or -1 on error
+ */
 static int virCgroupKillInternal(virCgroupPtr group, int signum, virHashTablePtr pids)
 {
-    int rc;
+    int ret = -1;
     bool killedAny = false;
     char *keypath = NULL;
     bool done = false;
@@ -2328,10 +2331,11 @@ static int virCgroupKillInternal(virCgroupPtr group, int signum, virHashTablePtr
     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;
+    if (virCgroupPathOfController(group, -1, "tasks", &keypath) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("No tasks file for group path '%s'"),
+                       group->path);
+        return -1;
     }
 
     /* PIDs may be forking as we kill them, so loop
@@ -2340,8 +2344,9 @@ static int virCgroupKillInternal(virCgroupPtr group, int signum, virHashTablePtr
     while (!done) {
         done = true;
         if (!(fp = fopen(keypath, "r"))) {
-            rc = -errno;
-            VIR_DEBUG("Failed to read %s: %m\n", keypath);
+            virReportSystemError(errno,
+                                 _("Failed to read %s"),
+                                 keypath);
             goto cleanup;
         } else {
             while (!feof(fp)) {
@@ -2349,8 +2354,9 @@ static int virCgroupKillInternal(virCgroupPtr group, int signum, virHashTablePtr
                 if (fscanf(fp, "%lu", &pid_value) != 1) {
                     if (feof(fp))
                         break;
-                    rc = -errno;
-                    VIR_DEBUG("Failed to read %s: %m\n", keypath);
+                    virReportSystemError(errno,
+                                         _("Failed to read %s"),
+                                         keypath);
                     goto cleanup;
                 }
                 if (virHashLookup(pids, (void*)pid_value))
@@ -2360,7 +2366,9 @@ static int virCgroupKillInternal(virCgroupPtr group, int signum, virHashTablePtr
                 /* Cgroups is a Linux concept, so this cast is safe.  */
                 if (kill((pid_t)pid_value, signum) < 0) {
                     if (errno != ESRCH) {
-                        rc = -errno;
+                        virReportSystemError(errno,
+                                             _("Failed to kill process %lu"),
+                                             pid_value);
                         goto cleanup;
                     }
                     /* Leave RC == 0 since we didn't kill one */
@@ -2375,13 +2383,13 @@ static int virCgroupKillInternal(virCgroupPtr group, int signum, virHashTablePtr
         }
     }
 
-    rc = killedAny ? 1 : 0;
+    ret = killedAny ? 1 : 0;
 
 cleanup:
     VIR_FREE(keypath);
     VIR_FORCE_FCLOSE(fp);
 
-    return rc;
+    return ret;
 }
 
 
@@ -2400,15 +2408,12 @@ static void *virCgroupPidCopy(const void *name)
 }
 
 /*
- * Returns
- *   < 0 : errno that occurred
- *     0 : no PIDs killed
- *     1 : at least one PID killed
+ * Returns 1 if some PIDs kill, 0 if non are killed, or -1 on error
  */
 int virCgroupKill(virCgroupPtr group, int signum)
 {
     VIR_DEBUG("group=%p path=%s signum=%d", group, group->path, signum);
-    int rc;
+    int ret;
     /* The 'tasks' file in cgroups can contain duplicated
      * pids, so we use a hash to track which we've already
      * killed.
@@ -2420,37 +2425,42 @@ int virCgroupKill(virCgroupPtr group, int signum)
                                              virCgroupPidCopy,
                                              NULL);
 
-    rc = virCgroupKillInternal(group, signum, pids);
+    ret = virCgroupKillInternal(group, signum, pids);
 
     virHashFree(pids);
 
-    return rc;
+    return ret;
 }
 
 
 static int virCgroupKillRecursiveInternal(virCgroupPtr group, int signum, virHashTablePtr pids, bool dormdir)
 {
+    int ret = -1;
     int rc;
-    int killedAny = 0;
+    bool killedAny = false;
     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 (virCgroupPathOfController(group, -1, "", &keypath) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("No tasks file for group path '%s'"),
+                       group->path);
+        return -1;
     }
 
-    if ((rc = virCgroupKillInternal(group, signum, pids)) != 0)
-        return rc;
+    if ((rc = virCgroupKillInternal(group, signum, pids)) < 0)
+        return -1;
+    if (rc == 1)
+        killedAny = true;
 
     VIR_DEBUG("Iterate over children of %s", keypath);
     if (!(dp = opendir(keypath))) {
-        rc = -errno;
-        return rc;
+        virReportSystemError(errno,
+                             _("Cannot open %s"), keypath);
+        return -1;
     }
 
     while ((ent = readdir(dp))) {
@@ -2463,15 +2473,13 @@ static int virCgroupKillRecursiveInternal(virCgroupPtr group, int signum, virHas
 
         VIR_DEBUG("Process subdir %s", ent->d_name);
 
-        if (virCgroupNew(ent->d_name, group, -1, &subgroup) < 0) {
-            rc = -EIO;
+        if (virCgroupNew(ent->d_name, group, -1, &subgroup) < 0)
             goto cleanup;
-        }
 
         if ((rc = virCgroupKillRecursiveInternal(subgroup, signum, pids, true)) < 0)
             goto cleanup;
         if (rc == 1)
-            killedAny = 1;
+            killedAny = true;
 
         if (dormdir)
             virCgroupRemove(subgroup);
@@ -2479,18 +2487,18 @@ static int virCgroupKillRecursiveInternal(virCgroupPtr group, int signum, virHas
         virCgroupFree(&subgroup);
     }
 
-    rc = killedAny;
+    ret = killedAny ? 1 : 0;
 
 cleanup:
     virCgroupFree(&subgroup);
     closedir(dp);
 
-    return rc;
+    return ret;
 }
 
 int virCgroupKillRecursive(virCgroupPtr group, int signum)
 {
-    int rc;
+    int ret;
     VIR_DEBUG("group=%p path=%s signum=%d", group, group->path, signum);
     virHashTablePtr pids = virHashCreateFull(100,
                                              NULL,
@@ -2499,18 +2507,18 @@ int virCgroupKillRecursive(virCgroupPtr group, int signum)
                                              virCgroupPidCopy,
                                              NULL);
 
-    rc = virCgroupKillRecursiveInternal(group, signum, pids, false);
+    ret = virCgroupKillRecursiveInternal(group, signum, pids, false);
 
     virHashFree(pids);
 
-    return rc;
+    return ret;
 }
 
 
 int virCgroupKillPainfully(virCgroupPtr group)
 {
     size_t i;
-    int rc;
+    int ret;
     VIR_DEBUG("cgroup=%p path=%s", group, group->path);
     for (i = 0; i < 15; i++) {
         int signum;
@@ -2521,33 +2529,39 @@ int virCgroupKillPainfully(virCgroupPtr group)
         else
             signum = 0; /* Just check for existence */
 
-        rc = virCgroupKillRecursive(group, signum);
-        VIR_DEBUG("Iteration %zu rc=%d", i, rc);
-        /* If rc == -1 we hit error, if 0 we ran out of PIDs */
-        if (rc <= 0)
+        ret = virCgroupKillRecursive(group, signum);
+        VIR_DEBUG("Iteration %zu rc=%d", i, ret);
+        /* If ret == -1 we hit error, if 0 we ran out of PIDs */
+        if (ret <= 0)
             break;
 
         usleep(200 * 1000);
     }
-    VIR_DEBUG("Complete %d", rc);
-    return rc;
+    VIR_DEBUG("Complete %d", ret);
+    return ret;
 }
 
 #else /* !(HAVE_KILL, HAVE_MNTENT_H, HAVE_GETMNTENT_R) */
 int virCgroupKill(virCgroupPtr group ATTRIBUTE_UNUSED,
                   int signum ATTRIBUTE_UNUSED)
 {
-    return -ENOSYS;
+    virReportSystemError(ENOSYS, "%s",
+                         _("Control groups not supported on this platform"));
+    return -1;
 }
 int virCgroupKillRecursive(virCgroupPtr group ATTRIBUTE_UNUSED,
                            int signum ATTRIBUTE_UNUSED)
 {
-    return -ENOSYS;
+    virReportSystemError(ENOSYS, "%s",
+                         _("Control groups not supported on this platform"));
+    return -1;
 }
 
 int virCgroupKillPainfully(virCgroupPtr group ATTRIBUTE_UNUSED)
 {
-    return -ENOSYS;
+    virReportSystemError(ENOSYS, "%s",
+                         _("Control groups not supported on this platform"));
+    return -1;
 }
 #endif /* HAVE_KILL, HAVE_MNTENT_H, HAVE_GETMNTENT_R */
 
-- 
1.8.1.4


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