On 07/18/2013 09:00 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> Instead of returning errno values, change the virCgroupKill*
> APIs to fully report errors.
>
> Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> ---
> 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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list