Package: libvirt-daemon Version: 5.0.0-2 Severity: grave Tags: patch Justification: renders package unusable
Dear maintainer, LXC container shut down is ignore. Amongst others, this will induce a hang on host shut down as the libvirt daemon waits 3 minutes per active container for shut down. Relevant patches from upstram are >From 64eca3d5e30030147383bc63eba77e723563d4e2 Mon Sep 17 00:00:00 2001 From: Michal Privoznik <mpriv...@redhat.com> Date: Fri, 25 Jan 2019 12:37:53 +0100 Subject: [PATCH 1/2] virinitctl: Expose fifo paths and allow caller to chose one So far the virInitctlSetRunLevel() is fully automatic. It finds the correct fifo to use to talk to the init and it will set the desired runlevel. Well, callers (so far there is just one) will need to inspect the fifo a bit just before the runlevel is set. Therefore, expose the internal list of fifos and also allow caller to explicitly use one. Signed-off-by: Michal Privoznik <mpriv...@redhat.com> Reviewed-by: Erik Skultety <eskul...@redhat.com> >From 94fce255461ad6bf0366dd4428921d7d41ba1a8f Mon Sep 17 00:00:00 2001 From: Michal Privoznik <mpriv...@redhat.com> Date: Fri, 25 Jan 2019 12:42:54 +0100 Subject: [PATCH 2/2] lxc: Don't reboot host on virDomainReboot If the container is really a simple one (init is just bash and the whole root is passed through) then virDomainReboot and virDomainShutdown will talk to the actual init within the host. Therefore, 'virsh shutdown $dom' will result in shutting down the host. True, at that point the container is shut down too but looks a bit harsh to me. The solution is to check if the init inside the container is or is not the same as the init running on the host. Signed-off-by: Michal Privoznik <mpriv...@redhat.com> Reviewed-by: Erik Skultety <eskul...@redhat.com> >From 14b6a1854fb4c02c5fb2f51679f8ff099f28f53c Mon Sep 17 00:00:00 2001 From: Maxim Kozin <koloma...@gmail.com> Date: Wed, 6 Mar 2019 21:39:11 +0300 Subject: [PATCH] lxc: Try harder to stop/reboot containers If shutting down a container via setting the runlevel fails, the control jumps right onto endjob label and doesn't even try sending the signal. If flags allow it, we should try both methods. Signed-off-by: Maxim Kozin <koloma...@gmail.com> Signed-off-by: Michal Privoznik <mpriv...@redhat.com> -- System Information: Debian Release: buster/sid APT prefers testing APT policy: (500, 'testing') Architecture: amd64 (x86_64) Kernel: Linux 4.19.0-4-amd64 (SMP w/4 CPU cores) Locale: LANG=de_DE.UTF-8, LC_CTYPE=de_DE.UTF-8 (charmap=UTF-8), LANGUAGE=de_DE.UTF-8 (charmap=UTF-8) Shell: /bin/sh linked to /usr/bin/dash Init: systemd (via /run/systemd/system) Versions of packages libvirt-daemon depends on: ii libacl1 2.2.53-4 ii libapparmor1 2.13.2-10 ii libaudit1 1:2.8.4-2 ii libavahi-client3 0.7-4+b1 ii libavahi-common3 0.7-4+b1 ii libblkid1 2.33.1-0.1 ii libc6 2.28-8 ii libcap-ng0 0.7.9-2 ii libcurl3-gnutls 7.64.0-2 ii libdbus-1-3 1.12.12-1 ii libdevmapper1.02.1 2:1.02.155-2 ii libfuse2 2.9.9-1 ii libgcc1 1:8.3.0-6 ii libgnutls30 3.6.6-2 ii libnetcf1 1:0.2.8-1+b2 ii libnl-3-200 3.4.0-1 ii libnl-route-3-200 3.4.0-1 ii libnuma1 2.0.12-1 ii libparted2 3.2-24 ii libpcap0.8 1.8.1-6 ii libpciaccess0 0.14-1 ii libsasl2-2 2.1.27+dfsg-1 ii libselinux1 2.8-1+b1 ii libssh2-1 1.8.0-2.1 ii libudev1 241-3 hi libvirt0 5.0.0-2 ii libxenmisc4.11 4.11.1+26-g87f51bf366-3 ii libxenstore3.0 4.11.1+26-g87f51bf366-3 ii libxentoollog1 4.11.1+26-g87f51bf366-3 ii libxml2 2.9.4+dfsg1-7+b3 ii libyajl2 2.1.0-3 Versions of packages libvirt-daemon recommends: ii libxml2-utils 2.9.4+dfsg1-7+b3 ii netcat-openbsd 1.195-2 ii qemu-kvm 1:3.1+dfsg-7 Versions of packages libvirt-daemon suggests: pn libvirt-daemon-driver-storage-gluster <none> pn libvirt-daemon-driver-storage-rbd <none> pn libvirt-daemon-driver-storage-zfs <none> hi libvirt-daemon-system 5.0.0-2 ii numad 0.5+20150602-5 -- no debconf information
>From 64eca3d5e30030147383bc63eba77e723563d4e2 Mon Sep 17 00:00:00 2001 From: Michal Privoznik <mpriv...@redhat.com> Date: Fri, 25 Jan 2019 12:37:53 +0100 Subject: [PATCH 1/2] virinitctl: Expose fifo paths and allow caller to chose one So far the virInitctlSetRunLevel() is fully automatic. It finds the correct fifo to use to talk to the init and it will set the desired runlevel. Well, callers (so far there is just one) will need to inspect the fifo a bit just before the runlevel is set. Therefore, expose the internal list of fifos and also allow caller to explicitly use one. Signed-off-by: Michal Privoznik <mpriv...@redhat.com> Reviewed-by: Erik Skultety <eskul...@redhat.com> --- src/libvirt_private.syms | 1 + src/lxc/lxc_driver.c | 2 +- src/util/virinitctl.c | 67 ++++++++++++++++++++++++++-------------- src/util/virinitctl.h | 6 +++- 4 files changed, 50 insertions(+), 26 deletions(-) Index: libvirt/src/libvirt_private.syms =================================================================== --- libvirt.orig/src/libvirt_private.syms +++ libvirt/src/libvirt_private.syms @@ -2047,6 +2047,7 @@ virIdentitySetX509DName; # util/virinitctl.h +virInitctlFifos; virInitctlSetRunLevel; Index: libvirt/src/lxc/lxc_driver.c =================================================================== --- libvirt.orig/src/lxc/lxc_driver.c +++ libvirt/src/lxc/lxc_driver.c @@ -3277,7 +3277,7 @@ lxcDomainInitctlCallback(pid_t pid ATTRI void *opaque) { int *command = opaque; - return virInitctlSetRunLevel(*command); + return virInitctlSetRunLevel(NULL, *command); } Index: libvirt/src/util/virinitctl.c =================================================================== --- libvirt.orig/src/util/virinitctl.c +++ libvirt/src/util/virinitctl.c @@ -101,7 +101,21 @@ struct virInitctlRequest { verify(sizeof(struct virInitctlRequest) == 384); # endif -/* + +/* List of fifos that inits are known to listen on */ +const char *virInitctlFifos[] = { + "/run/initctl", + "/dev/initctl", + "/etc/.initctl", + NULL +}; + + +/** + * virInitctlSetRunLevel: + * @fifo: the path to fifo that init listens on (can be NULL for autodetection) + * @level: the desired runlevel + * * Send a message to init to change the runlevel. This function is * asynchronous-signal-safe (thus safe to use after fork of a * multithreaded parent) - which is good, because it should only be @@ -110,18 +124,14 @@ struct virInitctlRequest { * Returns 1 on success, 0 if initctl does not exist, -1 on error */ int -virInitctlSetRunLevel(virInitctlRunLevel level) +virInitctlSetRunLevel(const char *fifo, + virInitctlRunLevel level) { struct virInitctlRequest req; int fd = -1; int ret = -1; - const char *initctl_fifo = NULL; + const int open_flags = O_WRONLY|O_NONBLOCK|O_CLOEXEC|O_NOCTTY; size_t i = 0; - const char *initctl_fifos[] = { - "/run/initctl", - "/dev/initctl", - "/etc/.initctl", - }; memset(&req, 0, sizeof(req)); @@ -131,31 +141,39 @@ virInitctlSetRunLevel(virInitctlRunLevel /* Yes it is an 'int' field, but wants a numeric character. Go figure */ req.runlevel = '0' + level; - for (i = 0; i < ARRAY_CARDINALITY(initctl_fifos); i++) { - initctl_fifo = initctl_fifos[i]; - - if ((fd = open(initctl_fifo, - O_WRONLY|O_NONBLOCK|O_CLOEXEC|O_NOCTTY)) >= 0) - break; - - if (errno != ENOENT) { + if (fifo) { + if ((fd = open(fifo, open_flags)) < 0) { virReportSystemError(errno, _("Cannot open init control %s"), - initctl_fifo); + fifo); goto cleanup; } - } + } else { + for (i = 0; virInitctlFifos[i]; i++) { + fifo = virInitctlFifos[i]; + + if ((fd = open(fifo, open_flags)) >= 0) + break; + + if (errno != ENOENT) { + virReportSystemError(errno, + _("Cannot open init control %s"), + fifo); + goto cleanup; + } + } - /* Ensure we found a valid initctl fifo */ - if (fd < 0) { - ret = 0; - goto cleanup; + /* Ensure we found a valid initctl fifo */ + if (fd < 0) { + ret = 0; + goto cleanup; + } } if (safewrite(fd, &req, sizeof(req)) != sizeof(req)) { virReportSystemError(errno, _("Failed to send request to init control %s"), - initctl_fifo); + fifo); goto cleanup; } @@ -166,7 +184,8 @@ virInitctlSetRunLevel(virInitctlRunLevel return ret; } #else -int virInitctlSetRunLevel(virInitctlRunLevel level ATTRIBUTE_UNUSED) +int virInitctlSetRunLevel(const char *fifo ATTRIBUTE_UNUSED, + virInitctlRunLevel level ATTRIBUTE_UNUSED) { virReportUnsupportedError(); return -1; Index: libvirt/src/util/virinitctl.h =================================================================== --- libvirt.orig/src/util/virinitctl.h +++ libvirt/src/util/virinitctl.h @@ -33,6 +33,10 @@ typedef enum { VIR_INITCTL_RUNLEVEL_LAST } virInitctlRunLevel; -int virInitctlSetRunLevel(virInitctlRunLevel level); + +extern const char *virInitctlFifos[]; + +int virInitctlSetRunLevel(const char *fifo, + virInitctlRunLevel level); #endif /* LIBVIRT_VIRINITCTL_H */ >From 94fce255461ad6bf0366dd4428921d7d41ba1a8f Mon Sep 17 00:00:00 2001 From: Michal Privoznik <mpriv...@redhat.com> Date: Fri, 25 Jan 2019 12:42:54 +0100 Subject: [PATCH 2/2] lxc: Don't reboot host on virDomainReboot If the container is really a simple one (init is just bash and the whole root is passed through) then virDomainReboot and virDomainShutdown will talk to the actual init within the host. Therefore, 'virsh shutdown $dom' will result in shutting down the host. True, at that point the container is shut down too but looks a bit harsh to me. The solution is to check if the init inside the container is or is not the same as the init running on the host. Signed-off-by: Michal Privoznik <mpriv...@redhat.com> Reviewed-by: Erik Skultety <eskul...@redhat.com> --- src/lxc/lxc_domain.c | 90 ++++++++++++++++++++++++++++++++++++++++++++ src/lxc/lxc_domain.h | 4 ++ src/lxc/lxc_driver.c | 17 +-------- 3 files changed, 96 insertions(+), 15 deletions(-) Index: libvirt/src/lxc/lxc_domain.c =================================================================== --- libvirt.orig/src/lxc/lxc_domain.c +++ libvirt/src/lxc/lxc_domain.c @@ -32,6 +32,7 @@ #include "virfile.h" #include "virtime.h" #include "virsystemd.h" +#include "virinitctl.h" #define VIR_FROM_THIS VIR_FROM_LXC #define LXC_NAMESPACE_HREF "http://libvirt.org/schemas/domain/lxc/1.0" @@ -416,3 +417,92 @@ virLXCDomainGetMachineName(virDomainDefP return ret; } + + +typedef struct _lxcDomainInitctlCallbackData lxcDomainInitctlCallbackData; +struct _lxcDomainInitctlCallbackData { + int runlevel; + bool *st_valid; + struct stat *st; +}; + + +static int +lxcDomainInitctlCallback(pid_t pid ATTRIBUTE_UNUSED, + void *opaque) +{ + lxcDomainInitctlCallbackData *data = opaque; + size_t i; + + for (i = 0; virInitctlFifos[i]; i++) { + const char *fifo = virInitctlFifos[i]; + struct stat cont_sb; + + if (stat(fifo, &cont_sb) < 0) { + if (errno == ENOENT) + continue; + + virReportSystemError(errno, _("Unable to stat %s"), fifo); + return -1; + } + + /* Check if the init fifo is not the very one that's on + * the host. We don't want to change the host's runlevel. + */ + if (data->st_valid[i] && + data->st[i].st_dev == cont_sb.st_dev && + data->st[i].st_ino == cont_sb.st_ino) + continue; + + return virInitctlSetRunLevel(fifo, data->runlevel); + } + + /* If no usable fifo was found then declare success. Caller + * will try killing the domain with signal. */ + return 0; +} + + +int +virLXCDomainSetRunlevel(virDomainObjPtr vm, + int runlevel) +{ + virLXCDomainObjPrivatePtr priv = vm->privateData; + lxcDomainInitctlCallbackData data; + size_t nfifos = 0; + size_t i; + int ret = -1; + + memset(&data, 0, sizeof(data)); + + data.runlevel = runlevel; + + for (nfifos = 0; virInitctlFifos[nfifos]; nfifos++) + ; + + if (VIR_ALLOC_N(data.st, nfifos) < 0 || + VIR_ALLOC_N(data.st_valid, nfifos) < 0) + goto cleanup; + + for (i = 0; virInitctlFifos[i]; i++) { + const char *fifo = virInitctlFifos[i]; + + if (stat(fifo, &(data.st[i])) < 0) { + if (errno == ENOENT) + continue; + + virReportSystemError(errno, _("Unable to stat %s"), fifo); + goto cleanup; + } + + data.st_valid[i] = true; + } + + ret = virProcessRunInMountNamespace(priv->initpid, + lxcDomainInitctlCallback, + &data); + cleanup: + VIR_FREE(data.st); + VIR_FREE(data.st_valid); + return ret; +} Index: libvirt/src/lxc/lxc_domain.h =================================================================== --- libvirt.orig/src/lxc/lxc_domain.h +++ libvirt/src/lxc/lxc_domain.h @@ -109,4 +109,8 @@ virLXCDomainObjEndJob(virLXCDriverPtr dr char * virLXCDomainGetMachineName(virDomainDefPtr def, pid_t pid); +int +virLXCDomainSetRunlevel(virDomainObjPtr vm, + int runlevel); + #endif /* LIBVIRT_LXC_DOMAIN_H */ Index: libvirt/src/lxc/lxc_driver.c =================================================================== --- libvirt.orig/src/lxc/lxc_driver.c +++ libvirt/src/lxc/lxc_driver.c @@ -3273,15 +3273,6 @@ lxcConnectListAllDomains(virConnectPtr c static int -lxcDomainInitctlCallback(pid_t pid ATTRIBUTE_UNUSED, - void *opaque) -{ - int *command = opaque; - return virInitctlSetRunLevel(NULL, *command); -} - - -static int lxcDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { @@ -3318,9 +3309,7 @@ lxcDomainShutdownFlags(virDomainPtr dom, (flags & VIR_DOMAIN_SHUTDOWN_INITCTL)) { int command = VIR_INITCTL_RUNLEVEL_POWEROFF; - if ((rc = virProcessRunInMountNamespace(priv->initpid, - lxcDomainInitctlCallback, - &command)) < 0) + if ((rc = virLXCDomainSetRunlevel(vm, command)) < 0) goto endjob; if (rc == 0 && flags != 0 && ((flags & ~VIR_DOMAIN_SHUTDOWN_INITCTL) == 0)) { @@ -3398,9 +3387,7 @@ lxcDomainReboot(virDomainPtr dom, (flags & VIR_DOMAIN_REBOOT_INITCTL)) { int command = VIR_INITCTL_RUNLEVEL_REBOOT; - if ((rc = virProcessRunInMountNamespace(priv->initpid, - lxcDomainInitctlCallback, - &command)) < 0) + if ((rc = virLXCDomainSetRunlevel(vm, command)) < 0) goto endjob; if (rc == 0 && flags != 0 && ((flags & ~VIR_DOMAIN_SHUTDOWN_INITCTL) == 0)) { >From 14b6a1854fb4c02c5fb2f51679f8ff099f28f53c Mon Sep 17 00:00:00 2001 From: Maxim Kozin <koloma...@gmail.com> Date: Wed, 6 Mar 2019 21:39:11 +0300 Subject: [PATCH] lxc: Try harder to stop/reboot containers If shutting down a container via setting the runlevel fails, the control jumps right onto endjob label and doesn't even try sending the signal. If flags allow it, we should try both methods. Signed-off-by: Maxim Kozin <koloma...@gmail.com> Signed-off-by: Michal Privoznik <mpriv...@redhat.com> --- src/lxc/lxc_driver.c | 40 ++++++++++++++++++---------------------- 1 file changed, 18 insertions(+), 22 deletions(-) Index: libvirt/src/lxc/lxc_driver.c =================================================================== --- libvirt.orig/src/lxc/lxc_driver.c +++ libvirt/src/lxc/lxc_driver.c @@ -3280,7 +3280,7 @@ lxcDomainShutdownFlags(virDomainPtr dom, virLXCDomainObjPrivatePtr priv; virDomainObjPtr vm; int ret = -1; - int rc; + int rc = -1; virCheckFlags(VIR_DOMAIN_SHUTDOWN_INITCTL | VIR_DOMAIN_SHUTDOWN_SIGNAL, -1); @@ -3309,19 +3309,17 @@ lxcDomainShutdownFlags(virDomainPtr dom, (flags & VIR_DOMAIN_SHUTDOWN_INITCTL)) { int command = VIR_INITCTL_RUNLEVEL_POWEROFF; - if ((rc = virLXCDomainSetRunlevel(vm, command)) < 0) - goto endjob; - if (rc == 0 && flags != 0 && - ((flags & ~VIR_DOMAIN_SHUTDOWN_INITCTL) == 0)) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("Container does not provide an initctl pipe")); - goto endjob; + if ((rc = virLXCDomainSetRunlevel(vm, command)) < 0) { + if (flags != 0 && + (flags & VIR_DOMAIN_SHUTDOWN_INITCTL)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Container does not provide an initctl pipe")); + goto endjob; + } } - } else { - rc = 0; } - if (rc == 0 && + if (rc < 0 && (flags == 0 || (flags & VIR_DOMAIN_SHUTDOWN_SIGNAL))) { if (kill(priv->initpid, SIGTERM) < 0 && @@ -3358,7 +3356,7 @@ lxcDomainReboot(virDomainPtr dom, virLXCDomainObjPrivatePtr priv; virDomainObjPtr vm; int ret = -1; - int rc; + int rc = -1; virCheckFlags(VIR_DOMAIN_REBOOT_INITCTL | VIR_DOMAIN_REBOOT_SIGNAL, -1); @@ -3387,19 +3385,17 @@ lxcDomainReboot(virDomainPtr dom, (flags & VIR_DOMAIN_REBOOT_INITCTL)) { int command = VIR_INITCTL_RUNLEVEL_REBOOT; - if ((rc = virLXCDomainSetRunlevel(vm, command)) < 0) - goto endjob; - if (rc == 0 && flags != 0 && - ((flags & ~VIR_DOMAIN_SHUTDOWN_INITCTL) == 0)) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("Container does not provide an initctl pipe")); - goto endjob; + if ((rc = virLXCDomainSetRunlevel(vm, command)) < 0) { + if (flags != 0 && + (flags & VIR_DOMAIN_REBOOT_INITCTL)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Container does not provide an initctl pipe")); + goto endjob; + } } - } else { - rc = 0; } - if (rc == 0 && + if (rc < 0 && (flags == 0 || (flags & VIR_DOMAIN_REBOOT_SIGNAL))) { if (kill(priv->initpid, SIGHUP) < 0 &&
>From 64eca3d5e30030147383bc63eba77e723563d4e2 Mon Sep 17 00:00:00 2001 From: Michal Privoznik <mpriv...@redhat.com> Date: Fri, 25 Jan 2019 12:37:53 +0100 Subject: [PATCH 1/2] virinitctl: Expose fifo paths and allow caller to chose one So far the virInitctlSetRunLevel() is fully automatic. It finds the correct fifo to use to talk to the init and it will set the desired runlevel. Well, callers (so far there is just one) will need to inspect the fifo a bit just before the runlevel is set. Therefore, expose the internal list of fifos and also allow caller to explicitly use one. Signed-off-by: Michal Privoznik <mpriv...@redhat.com> Reviewed-by: Erik Skultety <eskul...@redhat.com> --- src/libvirt_private.syms | 1 + src/lxc/lxc_driver.c | 2 +- src/util/virinitctl.c | 67 ++++++++++++++++++++++++++-------------- src/util/virinitctl.h | 6 +++- 4 files changed, 50 insertions(+), 26 deletions(-) Index: libvirt/src/libvirt_private.syms =================================================================== --- libvirt.orig/src/libvirt_private.syms +++ libvirt/src/libvirt_private.syms @@ -2047,6 +2047,7 @@ virIdentitySetX509DName; # util/virinitctl.h +virInitctlFifos; virInitctlSetRunLevel; Index: libvirt/src/lxc/lxc_driver.c =================================================================== --- libvirt.orig/src/lxc/lxc_driver.c +++ libvirt/src/lxc/lxc_driver.c @@ -3277,7 +3277,7 @@ lxcDomainInitctlCallback(pid_t pid ATTRI void *opaque) { int *command = opaque; - return virInitctlSetRunLevel(*command); + return virInitctlSetRunLevel(NULL, *command); } Index: libvirt/src/util/virinitctl.c =================================================================== --- libvirt.orig/src/util/virinitctl.c +++ libvirt/src/util/virinitctl.c @@ -101,7 +101,21 @@ struct virInitctlRequest { verify(sizeof(struct virInitctlRequest) == 384); # endif -/* + +/* List of fifos that inits are known to listen on */ +const char *virInitctlFifos[] = { + "/run/initctl", + "/dev/initctl", + "/etc/.initctl", + NULL +}; + + +/** + * virInitctlSetRunLevel: + * @fifo: the path to fifo that init listens on (can be NULL for autodetection) + * @level: the desired runlevel + * * Send a message to init to change the runlevel. This function is * asynchronous-signal-safe (thus safe to use after fork of a * multithreaded parent) - which is good, because it should only be @@ -110,18 +124,14 @@ struct virInitctlRequest { * Returns 1 on success, 0 if initctl does not exist, -1 on error */ int -virInitctlSetRunLevel(virInitctlRunLevel level) +virInitctlSetRunLevel(const char *fifo, + virInitctlRunLevel level) { struct virInitctlRequest req; int fd = -1; int ret = -1; - const char *initctl_fifo = NULL; + const int open_flags = O_WRONLY|O_NONBLOCK|O_CLOEXEC|O_NOCTTY; size_t i = 0; - const char *initctl_fifos[] = { - "/run/initctl", - "/dev/initctl", - "/etc/.initctl", - }; memset(&req, 0, sizeof(req)); @@ -131,31 +141,39 @@ virInitctlSetRunLevel(virInitctlRunLevel /* Yes it is an 'int' field, but wants a numeric character. Go figure */ req.runlevel = '0' + level; - for (i = 0; i < ARRAY_CARDINALITY(initctl_fifos); i++) { - initctl_fifo = initctl_fifos[i]; - - if ((fd = open(initctl_fifo, - O_WRONLY|O_NONBLOCK|O_CLOEXEC|O_NOCTTY)) >= 0) - break; - - if (errno != ENOENT) { + if (fifo) { + if ((fd = open(fifo, open_flags)) < 0) { virReportSystemError(errno, _("Cannot open init control %s"), - initctl_fifo); + fifo); goto cleanup; } - } + } else { + for (i = 0; virInitctlFifos[i]; i++) { + fifo = virInitctlFifos[i]; + + if ((fd = open(fifo, open_flags)) >= 0) + break; + + if (errno != ENOENT) { + virReportSystemError(errno, + _("Cannot open init control %s"), + fifo); + goto cleanup; + } + } - /* Ensure we found a valid initctl fifo */ - if (fd < 0) { - ret = 0; - goto cleanup; + /* Ensure we found a valid initctl fifo */ + if (fd < 0) { + ret = 0; + goto cleanup; + } } if (safewrite(fd, &req, sizeof(req)) != sizeof(req)) { virReportSystemError(errno, _("Failed to send request to init control %s"), - initctl_fifo); + fifo); goto cleanup; } @@ -166,7 +184,8 @@ virInitctlSetRunLevel(virInitctlRunLevel return ret; } #else -int virInitctlSetRunLevel(virInitctlRunLevel level ATTRIBUTE_UNUSED) +int virInitctlSetRunLevel(const char *fifo ATTRIBUTE_UNUSED, + virInitctlRunLevel level ATTRIBUTE_UNUSED) { virReportUnsupportedError(); return -1; Index: libvirt/src/util/virinitctl.h =================================================================== --- libvirt.orig/src/util/virinitctl.h +++ libvirt/src/util/virinitctl.h @@ -33,6 +33,10 @@ typedef enum { VIR_INITCTL_RUNLEVEL_LAST } virInitctlRunLevel; -int virInitctlSetRunLevel(virInitctlRunLevel level); + +extern const char *virInitctlFifos[]; + +int virInitctlSetRunLevel(const char *fifo, + virInitctlRunLevel level); #endif /* LIBVIRT_VIRINITCTL_H */ >From 94fce255461ad6bf0366dd4428921d7d41ba1a8f Mon Sep 17 00:00:00 2001 From: Michal Privoznik <mpriv...@redhat.com> Date: Fri, 25 Jan 2019 12:42:54 +0100 Subject: [PATCH 2/2] lxc: Don't reboot host on virDomainReboot If the container is really a simple one (init is just bash and the whole root is passed through) then virDomainReboot and virDomainShutdown will talk to the actual init within the host. Therefore, 'virsh shutdown $dom' will result in shutting down the host. True, at that point the container is shut down too but looks a bit harsh to me. The solution is to check if the init inside the container is or is not the same as the init running on the host. Signed-off-by: Michal Privoznik <mpriv...@redhat.com> Reviewed-by: Erik Skultety <eskul...@redhat.com> --- src/lxc/lxc_domain.c | 90 ++++++++++++++++++++++++++++++++++++++++++++ src/lxc/lxc_domain.h | 4 ++ src/lxc/lxc_driver.c | 17 +-------- 3 files changed, 96 insertions(+), 15 deletions(-) Index: libvirt/src/lxc/lxc_domain.c =================================================================== --- libvirt.orig/src/lxc/lxc_domain.c +++ libvirt/src/lxc/lxc_domain.c @@ -32,6 +32,7 @@ #include "virfile.h" #include "virtime.h" #include "virsystemd.h" +#include "virinitctl.h" #define VIR_FROM_THIS VIR_FROM_LXC #define LXC_NAMESPACE_HREF "http://libvirt.org/schemas/domain/lxc/1.0" @@ -416,3 +417,92 @@ virLXCDomainGetMachineName(virDomainDefP return ret; } + + +typedef struct _lxcDomainInitctlCallbackData lxcDomainInitctlCallbackData; +struct _lxcDomainInitctlCallbackData { + int runlevel; + bool *st_valid; + struct stat *st; +}; + + +static int +lxcDomainInitctlCallback(pid_t pid ATTRIBUTE_UNUSED, + void *opaque) +{ + lxcDomainInitctlCallbackData *data = opaque; + size_t i; + + for (i = 0; virInitctlFifos[i]; i++) { + const char *fifo = virInitctlFifos[i]; + struct stat cont_sb; + + if (stat(fifo, &cont_sb) < 0) { + if (errno == ENOENT) + continue; + + virReportSystemError(errno, _("Unable to stat %s"), fifo); + return -1; + } + + /* Check if the init fifo is not the very one that's on + * the host. We don't want to change the host's runlevel. + */ + if (data->st_valid[i] && + data->st[i].st_dev == cont_sb.st_dev && + data->st[i].st_ino == cont_sb.st_ino) + continue; + + return virInitctlSetRunLevel(fifo, data->runlevel); + } + + /* If no usable fifo was found then declare success. Caller + * will try killing the domain with signal. */ + return 0; +} + + +int +virLXCDomainSetRunlevel(virDomainObjPtr vm, + int runlevel) +{ + virLXCDomainObjPrivatePtr priv = vm->privateData; + lxcDomainInitctlCallbackData data; + size_t nfifos = 0; + size_t i; + int ret = -1; + + memset(&data, 0, sizeof(data)); + + data.runlevel = runlevel; + + for (nfifos = 0; virInitctlFifos[nfifos]; nfifos++) + ; + + if (VIR_ALLOC_N(data.st, nfifos) < 0 || + VIR_ALLOC_N(data.st_valid, nfifos) < 0) + goto cleanup; + + for (i = 0; virInitctlFifos[i]; i++) { + const char *fifo = virInitctlFifos[i]; + + if (stat(fifo, &(data.st[i])) < 0) { + if (errno == ENOENT) + continue; + + virReportSystemError(errno, _("Unable to stat %s"), fifo); + goto cleanup; + } + + data.st_valid[i] = true; + } + + ret = virProcessRunInMountNamespace(priv->initpid, + lxcDomainInitctlCallback, + &data); + cleanup: + VIR_FREE(data.st); + VIR_FREE(data.st_valid); + return ret; +} Index: libvirt/src/lxc/lxc_domain.h =================================================================== --- libvirt.orig/src/lxc/lxc_domain.h +++ libvirt/src/lxc/lxc_domain.h @@ -109,4 +109,8 @@ virLXCDomainObjEndJob(virLXCDriverPtr dr char * virLXCDomainGetMachineName(virDomainDefPtr def, pid_t pid); +int +virLXCDomainSetRunlevel(virDomainObjPtr vm, + int runlevel); + #endif /* LIBVIRT_LXC_DOMAIN_H */ Index: libvirt/src/lxc/lxc_driver.c =================================================================== --- libvirt.orig/src/lxc/lxc_driver.c +++ libvirt/src/lxc/lxc_driver.c @@ -3273,15 +3273,6 @@ lxcConnectListAllDomains(virConnectPtr c static int -lxcDomainInitctlCallback(pid_t pid ATTRIBUTE_UNUSED, - void *opaque) -{ - int *command = opaque; - return virInitctlSetRunLevel(NULL, *command); -} - - -static int lxcDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { @@ -3318,9 +3309,7 @@ lxcDomainShutdownFlags(virDomainPtr dom, (flags & VIR_DOMAIN_SHUTDOWN_INITCTL)) { int command = VIR_INITCTL_RUNLEVEL_POWEROFF; - if ((rc = virProcessRunInMountNamespace(priv->initpid, - lxcDomainInitctlCallback, - &command)) < 0) + if ((rc = virLXCDomainSetRunlevel(vm, command)) < 0) goto endjob; if (rc == 0 && flags != 0 && ((flags & ~VIR_DOMAIN_SHUTDOWN_INITCTL) == 0)) { @@ -3398,9 +3387,7 @@ lxcDomainReboot(virDomainPtr dom, (flags & VIR_DOMAIN_REBOOT_INITCTL)) { int command = VIR_INITCTL_RUNLEVEL_REBOOT; - if ((rc = virProcessRunInMountNamespace(priv->initpid, - lxcDomainInitctlCallback, - &command)) < 0) + if ((rc = virLXCDomainSetRunlevel(vm, command)) < 0) goto endjob; if (rc == 0 && flags != 0 && ((flags & ~VIR_DOMAIN_SHUTDOWN_INITCTL) == 0)) { >From 14b6a1854fb4c02c5fb2f51679f8ff099f28f53c Mon Sep 17 00:00:00 2001 From: Maxim Kozin <koloma...@gmail.com> Date: Wed, 6 Mar 2019 21:39:11 +0300 Subject: [PATCH] lxc: Try harder to stop/reboot containers If shutting down a container via setting the runlevel fails, the control jumps right onto endjob label and doesn't even try sending the signal. If flags allow it, we should try both methods. Signed-off-by: Maxim Kozin <koloma...@gmail.com> Signed-off-by: Michal Privoznik <mpriv...@redhat.com> --- src/lxc/lxc_driver.c | 40 ++++++++++++++++++---------------------- 1 file changed, 18 insertions(+), 22 deletions(-) Index: libvirt/src/lxc/lxc_driver.c =================================================================== --- libvirt.orig/src/lxc/lxc_driver.c +++ libvirt/src/lxc/lxc_driver.c @@ -3280,7 +3280,7 @@ lxcDomainShutdownFlags(virDomainPtr dom, virLXCDomainObjPrivatePtr priv; virDomainObjPtr vm; int ret = -1; - int rc; + int rc = -1; virCheckFlags(VIR_DOMAIN_SHUTDOWN_INITCTL | VIR_DOMAIN_SHUTDOWN_SIGNAL, -1); @@ -3309,19 +3309,17 @@ lxcDomainShutdownFlags(virDomainPtr dom, (flags & VIR_DOMAIN_SHUTDOWN_INITCTL)) { int command = VIR_INITCTL_RUNLEVEL_POWEROFF; - if ((rc = virLXCDomainSetRunlevel(vm, command)) < 0) - goto endjob; - if (rc == 0 && flags != 0 && - ((flags & ~VIR_DOMAIN_SHUTDOWN_INITCTL) == 0)) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("Container does not provide an initctl pipe")); - goto endjob; + if ((rc = virLXCDomainSetRunlevel(vm, command)) < 0) { + if (flags != 0 && + (flags & VIR_DOMAIN_SHUTDOWN_INITCTL)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Container does not provide an initctl pipe")); + goto endjob; + } } - } else { - rc = 0; } - if (rc == 0 && + if (rc < 0 && (flags == 0 || (flags & VIR_DOMAIN_SHUTDOWN_SIGNAL))) { if (kill(priv->initpid, SIGTERM) < 0 && @@ -3358,7 +3356,7 @@ lxcDomainReboot(virDomainPtr dom, virLXCDomainObjPrivatePtr priv; virDomainObjPtr vm; int ret = -1; - int rc; + int rc = -1; virCheckFlags(VIR_DOMAIN_REBOOT_INITCTL | VIR_DOMAIN_REBOOT_SIGNAL, -1); @@ -3387,19 +3385,17 @@ lxcDomainReboot(virDomainPtr dom, (flags & VIR_DOMAIN_REBOOT_INITCTL)) { int command = VIR_INITCTL_RUNLEVEL_REBOOT; - if ((rc = virLXCDomainSetRunlevel(vm, command)) < 0) - goto endjob; - if (rc == 0 && flags != 0 && - ((flags & ~VIR_DOMAIN_SHUTDOWN_INITCTL) == 0)) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("Container does not provide an initctl pipe")); - goto endjob; + if ((rc = virLXCDomainSetRunlevel(vm, command)) < 0) { + if (flags != 0 && + (flags & VIR_DOMAIN_REBOOT_INITCTL)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Container does not provide an initctl pipe")); + goto endjob; + } } - } else { - rc = 0; } - if (rc == 0 && + if (rc < 0 && (flags == 0 || (flags & VIR_DOMAIN_REBOOT_SIGNAL))) { if (kill(priv->initpid, SIGHUP) < 0 &&