Daniel Veillard wrote:
> 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 ;-)
:-) I'll add those.
>
>> +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
Yep, I'll make this change here and below as well.
Thanks!
>
>> +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
>
--
Best Regards,
Dave Leskovec
IBM Linux Technology Center
Open Virtualization
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list