[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libvir] [PATCH 2/2] lxc: Shutdown and destroy container



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);
...


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]