Dave Leskovec <dlesko@xxxxxxxxxxxxxxxxxx> 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);
...
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list