Re: [Libvir] [PATCH] lxc: handle SIGCHLD from exiting container

[Jim Meyering <jim meyering net>]

Dave Leskovec <dlesko linux vnet ibm com> wrote:
> This patch allows the lxc driver to handle SIGCHLD signals from exiting
...

Hi Dave,
At least superficially, this looks fine.
Two questions:

> Index: b/src/driver.h
> ===================================================================
> --- a/src/driver.h	2008-04-10 09:54:54.000000000 -0700
> +++ b/src/driver.h	2008-04-30 15:36:47.000000000 -0700
> @@ -11,6 +11,10 @@
>
>  #include <libxml/uri.h>
>
> +#ifndef	_SIGNAL_H
> +#include <signal.h>
> +#endif

In practice it's fine to include <signal.h> unconditionally,
and even multiple times.  Have you encountered a version of <signal.h>
that may not be included twice?  If so, it probably deserves a comment
with the details.

...
> Index: b/src/lxc_driver.c
> ===================================================================
...
> -static int lxcDomainDestroy(virDomainPtr dom)
> +static int lxcVMCleanup(lxc_driver_t *driver, lxc_vm_t * vm)
>  {
>      int rc = -1;
...
> -    rc = WEXITSTATUS(childStatus);
> -    DEBUG("container exited with rc: %d", rc);
> +    if (WIFEXITED(childStatus)) {
> +        rc = WEXITSTATUS(childStatus);
> +        DEBUG("container exited with rc: %d", rc);
> +    }
> +
> +    rc = 0;

Didn't you mean to initialize rc=0 before that if block?
If not, please add a comment saying why the child failure
doesn't affect the function's return value.