Re: [libvirt] [PATCH 3/3] Add support for shutdown / reboot APIs in LXC driver
Add support for doing controlled shutdown / reboot in the LXC
driver. The default behaviour is to try talking to /dev/initctl
inside the container's virtual root (/proc/$INITPID/root). This
works with sysvinit or systemd. If that file does not exist
then send SIGTERM (for shutdown) or SIGHUP (for reboot). These
signals are not any kind of particular standard for shutdown
or reboot, just something apps can choose to handle. The new
virDomainSendProcessSignal allows for sending custom signals.
We might allow the choice of SIGTERM/HUP to be configured for
LXC containers via the XML in the future.
Maybe even via new attributes or sub-elements of the existing
<on_reboot> XML.
+
+ if (flags == 0 ||
+ (flags & VIR_DOMAIN_SHUTDOWN_INITCTL)) {
+ if ((rc =
virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_POWEROFF,
+ vroot)) < 0) {
+ goto cleanup;
+ }
+ if (rc == 0 && flags != 0 &&
Based on libvirt.c (well, if you follow my advice in 2/3 about
enforcing the mutual exclusion between the four explicit
shutdown methods), if 'flags != 0', then we are guaranteed
that 'flags == VIR_DOMAIN_SHUTDOWN_INITCTL)' at this point...
+ ((flags & ~VIR_DOMAIN_SHUTDOWN_INITCTL) == 0)) {
...which means this leg of the conditional is always true.
Or is your intent to allow the user to specify multiple flags
rather than enforcing mutual exclusion between the flags? And
if so, you need to fix libvirt.c to drop the mutual exclusion,
as well as to document that when multiple flags are specified,
it is up to the hypervisor which method is attempted first.
+static int
+lxcDomainReboot(virDomainPtr dom,
+ unsigned int flags)
+{
+ virLXCDriverPtr driver = dom->conn->privateData;
+ virLXCDomainObjPrivatePtr priv;
+ virDomainObjPtr vm;
+ char *vroot = NULL;
+ int ret = -1;
+ int rc;
+
+ virCheckFlags(VIR_DOMAIN_REBOOT_INITCTL |
+ VIR_DOMAIN_REBOOT_SIGNAL, -1);
This code is very similar to the shutdown; is it worth factoring
out a helper method that takes as additional arguments the choice
of initctl message (_POWEROFF vs. _REBOOT) and signal (SIGKILL
vs. SIGHUP) to use?
> + if (flags == 0 ||
> + (flags & VIR_DOMAIN_REBOOT_INITCTL)) {
> + if ((rc = virInitctlSetRunLevel(VIR_INITCTL_RUNLEVEL_REBOOT,
> + vroot)) < 0) {
> + goto cleanup;
> + }
> + if (rc == 0 && flags != 0 &&
+ ((flags & ~VIR_DOMAIN_SHUTDOWN_INITCTL) == 0)) {
Same review comments apply regarding whether we enforce mutual
exclusion of flags at the API level.