[Libvir] [PATCH] lxc: shutdown and destroy domain

[Dave Leskovec]

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