Your message dated Wed, 22 May 2019 11:34:44 +0000
with message-id <e1htpw8-000g6k...@fasolo.debian.org>
and subject line Bug#927310: fixed in libvirt 5.0.0-3
has caused the Debian Bug report #927310,
regarding libvirt-daemon: LXC container shut down, e.g., virsh -c lxc:// 
shutdown <name>, is ignored
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact ow...@bugs.debian.org
immediately.)


-- 
927310: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=927310
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems
--- Begin Message ---
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 &&

--- End Message ---
--- Begin Message ---
Source: libvirt
Source-Version: 5.0.0-3

We believe that the bug you reported is fixed in the latest version of
libvirt, which is due to be installed in the Debian FTP archive.

A summary of the changes between this version and the previous one is
attached.

Thank you for reporting the bug, which will now be closed.  If you
have further comments please address them to 927...@bugs.debian.org,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Guido Günther <a...@sigxcpu.org> (supplier of updated libvirt package)

(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing ftpmas...@ftp-master.debian.org)


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Format: 1.8
Date: Wed, 22 May 2019 12:31:08 +0200
Source: libvirt
Architecture: source
Version: 5.0.0-3
Distribution: unstable
Urgency: medium
Maintainer: Debian Libvirt Maintainers 
<pkg-libvirt-maintain...@lists.alioth.debian.org>
Changed-By: Guido Günther <a...@sigxcpu.org>
Closes: 897394 926999 927310 929334
Changes:
 libvirt (5.0.0-3) unstable; urgency=medium
 .
   [ Guido Günther ]
   * [6bc6e60] CVE-2019-10132: Fix vir{lock,log}d socket access.
     All patches were cherry-picked from upstream's v5.0-maint branch.
     (Closes: #929334)
   * [09016dd] d/patches: Move security fixes into security/
 .
   [ Joachim Falk ]
   * [5d96699] lxc: Fix killing of lxc containers if cgroup backend v2 is
     unavailable.
     (Closes: #926999)
   * [ea7a491] lxc: Fix container shutdown and host reboot
     (Closes: #927310, #897394)
Checksums-Sha1:
 47b830f4255c0ad5bbb52fe77392569f73970423 4353 libvirt_5.0.0-3.dsc
 ee72696860a2ceec1ce07247e0bef503ee4825c1 76996 libvirt_5.0.0-3.debian.tar.xz
 9d6e5a04213d249e66f593df63fd4c470b2e009e 19472 libvirt_5.0.0-3_amd64.buildinfo
Checksums-Sha256:
 258b58ec682c741d364e9e70004dcebb0609fb8e9dd748ff0317856af011d331 4353 
libvirt_5.0.0-3.dsc
 66ba224b7168fa44b382d9a158515cf34596ab072f3ef53d6f7083d90044e1cb 76996 
libvirt_5.0.0-3.debian.tar.xz
 7d2a4222f31bdb03342cadf1523d1a47cf04c023b10932cba77c296f625c0d08 19472 
libvirt_5.0.0-3_amd64.buildinfo
Files:
 dde11a7557b74fc06dab5aa627027918 4353 libs optional libvirt_5.0.0-3.dsc
 b426861e183f010e1499ec2bf574932e 76996 libs optional 
libvirt_5.0.0-3.debian.tar.xz
 cfd0537811f61479d7c29e7182612d8e 19472 libs optional 
libvirt_5.0.0-3_amd64.buildinfo

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEEvHzQcjh1660F3xzZB7i3sOqYEgsFAlzlLMIACgkQB7i3sOqY
EgvJlA//WiOQZfZG6SAi3c+5rO4UQ8l2nI7p4nQNE0DFmnWU6whRa+2j2qZaKfkM
Qo2fWiy6dOT/5+ci4rxDBcEHvqhQEOhk7KbBQOxI9yftQ+mzlMKt9/0xWoxM7CSB
j9/IagUnErZqZvdzFpOzIC1dAWuWPasbDwN7X2MJgILidpy5sADeRjOI5/BS5zl9
WwKkRmFCcshmwYYppu5sjSLLQYroA2vlW2odlWBKBwaKNscYmSy+GoRPReOL68sp
GlIr9nTN/htbd9tWjrEvXCIE2tfVXNIsarIxKcs514uhHzadixWN1HOIsaWpyDSq
HWtasfG/9oKdYEuntZtm7tmAbxhI2zQMFMKifj8s9Z/Yml1CljDbItEwunhS+g+9
dxlcglsNCOykDT+yWFNBP0UmkT/5UIc8MVNM0/H+jnUyQDVkeOhTimH0mB48ODjY
sufAQ8r1H8I9OS92Tjo2G/CrpCWJv3+LDex94qruiZ9ys0lHfri0TEmZP5TnP4ZN
qd0r9l+pOCLr6NemwwnUNUpGBi5mcVtWjgZ0vJz/Oq8UHJAi+Rh22yM73XxK4CdE
LcS9cr7aSgmqM+Q5sNGzGIB9Lk4T8YUYBKTevxojZJFe/4NNgaZzxKfl7BtJYKk2
8c6r1/XzG3+xn2gJY1u7fESwZSlgIJTIanK7GgFdLL87mDlSHNo=
=svda
-----END PGP SIGNATURE-----

--- End Message ---

Reply via email to