On Wed, Mar 26, 2008 at 11:22:25PM -0700, Dave Leskovec wrote: > This patch contains the shutdown and destroy domain support for Linux Containers. > > A shutdown of the container is requested by sending a SIGINT to the container > root process. > A container is destroyed by sending a SIGKILL to the container root process. > When this process exits, it takes all container child processes with it. [...] > Index: b/src/lxc_driver.c Above description would make for useful comments of the associated functions ;-)

> +static int lxcDomainShutdown(virDomainPtr dom) > +{ > + int rc = -1; > + lxc_driver_t *driver = (lxc_driver_t*)dom->conn->privateData; > + lxc_vm_t *vm = lxcFindVMByID(driver, dom->id); > + > + if (!vm) { > + lxcError(dom->conn, dom, VIR_ERR_INVALID_DOMAIN, > + _("no domain with id %d"), dom->id); > + goto error_out; > + } > + > + vm->state = VIR_DOMAIN_SHUTDOWN; > + > + if (0 > (kill(vm->def->id, SIGINT))) { > + if (ESRCH != errno) { > + lxcError(dom->conn, dom, VIR_ERR_INTERNAL_ERROR, > + _("sending SIGTERM failed: %s"), strerror(errno)); > + > + vm->state = VIR_DOMAIN_RUNNING; > + > + goto error_out; > + } > + } I would instead change the stet after the kill was received and avoid changing the domain state from it's unknown value if it fails. Think of multiple successive call to virDomainshutdown() for example

> +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; > + } > + > + vm->state = VIR_DOMAIN_SHUTDOWN; > + > + if (0 > (kill(vm->def->id, SIGKILL))) { > + if (ESRCH != errno) { > + lxcError(dom->conn, dom, VIR_ERR_INTERNAL_ERROR, > + _("sending SIGKILL failed: %s"), strerror(errno)); > + > + vm->state = VIR_DOMAIN_RUNNING; > + > + goto error_out; > + } > + } Same here ... > + waitpid(vm->def->id, &childStatus, 0); > + rc = WEXITSTATUS(childStatus); > + DEBUG("container exited with rc: %d", rc); > + > + vm->state = VIR_DOMAIN_SHUTOFF; > + vm->pid = -1; > + vm->def->id = -1; > + driver->nactivevms--; > + driver->ninactivevms++; > + > + rc = 0; > + > +error_out: > + return rc; > +} > + [...] > Index: b/src/lxc_container.c > =================================================================== > --- a/src/lxc_container.c 2008-03-24 16:28:47.000000000 -0700 > +++ b/src/lxc_container.c 2008-03-24 16:53:45.000000000 -0700 > @@ -222,6 +222,14 @@ > > } > > +static void lxcExecSigintHandler(int sig ATTRIBUTE_UNUSED, > + siginfo_t *signalInfo, > + void *context ATTRIBUTE_UNUSED) > +{ > + DEBUG("container received SIGINT from %d", signalInfo->si_pid); > + kill(SIGINT, initPid); > +} > + > static int lxcExecWithTty(lxc_vm_t *vm) > { > int rc = -1; > @@ -241,7 +249,16 @@ > sigAction.sa_mask = sigMask; > sigAction.sa_flags = SA_SIGINFO; > if (0 != sigaction(SIGCHLD, &sigAction, NULL)) { > - DEBUG("sigaction failed: %s\n", strerror(errno)); > + DEBUG("sigaction failed for SIGCHLD: %s\n", strerror(errno)); > + goto exit_with_error; > + } > + > + sigAction.sa_sigaction = lxcExecSigintHandler; > + sigfillset(&sigMask); > + sigAction.sa_mask = sigMask; > + sigAction.sa_flags = SA_SIGINFO; > + if (0 != sigaction(SIGINT, &sigAction, NULL)) { > + DEBUG("sigaction failed for SIGINT: %s\n", strerror(errno)); > goto exit_with_error; > } Looks fine, Daniel