On 07/19/2013 08:42 AM, Daniel P. Berrange wrote:
On Thu, Jul 18, 2013 at 09:36:59PM -0600, Eric Blake wrote:
> 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>
>> ---
>> + 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)?
I'll add in
- virCgroupKillPainfully(priv->cgroup);
+ rc = virCgroupKillPainfully(priv->cgroup);
+ if (rc < 0)
+ return -1;
+ if (rc > 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Some processes refused to die"));
+ return -1;
+ }
Works for me. ACK as revised.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library
http://libvirt.org