[Libvir] [PATCH 2/2] lxc: Shutdown and destroy container
This is a repost of the shutdown and destroy container support. Changes in this version: * Moved state changes to after the signal is successfully sent rather than restoring if it failed. * Signal handling in lxc_container goes away since the tty forwarding process is no longer the container root process. * Since the tty forwarding process is now outside the container, we have to kill and wait for it in the destroy. Thanks! -- Best Regards, Dave Leskovec IBM Linux Technology Center Open Virtualization
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); ...
Hi Jim, Responses inline below and I've attached an updated patch. Thanks! Jim Meyering wrote:
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.
Ya, if we get one of those errors for the container, chances are about 100% that we'll get it for the tty process as well.
+ }
Does the ESRCH case deserve some sort of diagnostic? (I don't know)
This would happen if the container has already exited. This may be the case if the container was running some job that ran til completion and exited or crashed. We don't really know what the intended behavior was so I don't think we can call it an error.
+ } + + 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.
If we failed to send the signal and errno != ESRCH, then we won't get here. If errno = ESRCH, then we still may need to wait on the process to get rid of the zombie and capture the exit status. Added some error handling for the waitpid() calls.
+ rc = WEXITSTATUS(childStatus); + DEBUG("container exited with rc: %d", rc); + + /* also need to kill tty forward process */ + /* wrap this with error handling etc.
yes ;-)
:-) added this and removed the comments
in the right place? */
Looks ok to me.
+ /* also wait for the process */ + kill(vm->pid, SIGKILL); ...
Thanks! -- Best Regards, Dave Leskovec IBM Linux Technology Center Open Virtualization
participants (2)
-
Dave Leskovec -
Jim Meyering