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@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/