Re: [libvirt] [PATCH 2/3] Convert the virCgroupKill* APIs to report errors
On 07/18/2013 09:00 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Instead of returning errno values, change the virCgroupKill*
APIs to fully report errors.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/lxc/lxc_process.c | 15 +------
src/util/vircgroup.c | 108 ++++++++++++++++++++++++++++----------------------
2 files changed, 62 insertions(+), 61 deletions(-)
+++ 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"));
So we previously reported -errno or +1 for two different types of failure...
- rc = -1;
- goto cleanup;
- }
+ virCgroupKillPainfully(priv->cgroup);
...but now don't report any failure at all. Is this right? Shouldn't
this be:
if (virCgroupKillPainfully(priv->cgroup) < 0)
goto cleanup;
or even still distinguish the error message for processes that couldn't
be killed (but maybe hoisting it into 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
s/kill/are killed/
s/non/none/
@@ -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
copy-and-paste
+ 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;
Here, you can still return 1 without reporting any error, if you timed out.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 621 bytes)