Dave Leskovec <dlesko@linux.vnet.ibm.com> wrote:
This is a repost of the shutdown and destroy container support. Changes in this
A few comments: ...
+static int lxcDomainDestroy(virDomainPtr dom) +{ + int rc = -1; + lxc_driver_t *driver = (lxc_driver_t*)dom->conn->privateData; + lxc_vm_t *vm = lxcFindVMByID(driver, dom->id); + int childStatus; + + if (!vm) { + lxcError(dom->conn, dom, VIR_ERR_INVALID_DOMAIN, + _("no domain with id %d"), dom->id); + goto error_out; + } + + if (0 > (kill(vm->def->id, SIGKILL))) { + if (ESRCH != errno) { + lxcError(dom->conn, dom, VIR_ERR_INTERNAL_ERROR, + _("sending SIGKILL failed: %s"), strerror(errno)); + + goto error_out;
I wondered... If this kill fails due to non-ESRCH, is it worth trying to kill vm->pid? Seeing that the only other kill errno values are EINVAL and EPERM, I guess not.
+ }
Does the ESRCH case deserve some sort of diagnostic? (I don't know)
+ } + + vm->state = VIR_DOMAIN_SHUTDOWN; + + waitpid(vm->def->id, &childStatus, 0);
How about waiting only if the signal was successfully sent? And detect waitpid failure.
+ rc = WEXITSTATUS(childStatus); + DEBUG("container exited with rc: %d", rc); + + /* also need to kill tty forward process */ + /* wrap this with error handling etc.
yes ;-)
in the right place? */
Looks ok to me.
+ /* also wait for the process */ + kill(vm->pid, SIGKILL); ...