On Thu, May 29, 2008 at 03:20:15PM -0700, Dave Leskovec wrote:
> This patch adds a check that validates that the container process pid still
> exists. This should catch cases where the container exits while libvirtd is down.
sounds fine,
> +/**
> + * lxcCheckContainerProcess:
> + * @def: Ptr to VM definition
> + *
> + * Checks if the container process (stored at def->id is running
> + *
> + * Returns on success or -1 in case of error
> + * 0 - no process with id vm->def->id
> + * 1 - container process exists
> + * -1 - error
> + */
> +int lxcCheckContainerProcess(lxc_vm_def_t *def)
> +{
> + int rc = -1;
> +
> + if (1 < def->id) {
> + if (-1 == kill(def->id, 0)) {
hum i didn't know of that way to check for a process, cool
> + if (ESRCH == errno) {
> + rc = 0;
> + DEBUG("pid %d no longer exists", def->id);
> + goto done;
> + }
> +
> + lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
> + _("error checking container process: %d %s"),
> + def->id, strerror(errno));
> + goto done;
> + }
The problem though is that by doing just a passive test for the PID
it feels like there is a possible race if the process counter rolled over and
another process with the same PID got create in the meantime.
i have the feeling that a test based on the state of the file descriptors
used to communicate with the container would be more reliable. Basically if the
container disapear, then the pipe should get in a half-closed state,
detecting the change at that level sounds like it would be more reliable,
don't you think so ?
But as is the patch is still a good improvement, +1
Daniel
--
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard | virtualization library http://libvirt.org/
veillard@xxxxxxxxxx | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list