This reverts commit 14b6a1854fb4c02c5fb2f51679f8ff099f28f53c.

If virLXCDomainSetRunlevel returns -1 this indicates a serious
error / failure that must be propagated to the caller. We must
not carry on with other shutdown methods in this case.

If virLXCDomainSetRunlevel return 0, this indicates that no
initctl was found and it is thus reasonable to fallback to
sending SIGTERM.

The commit being reverted is broken because it would fallback
to SIGTERM when virLXCDomainSetRunlevel returns -1, and would
not fallback when virLXCDomainSetRunlevel returns 0. ie it
did the exact opposite of what was required.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/lxc/lxc_driver.c | 40 ++++++++++++++++++++++------------------
 1 file changed, 22 insertions(+), 18 deletions(-)

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 5453f49064..ca0090de59 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -3271,7 +3271,7 @@ lxcDomainShutdownFlags(virDomainPtr dom,
     virLXCDomainObjPrivatePtr priv;
     virDomainObjPtr vm;
     int ret = -1;
-    int rc = -1;
+    int rc;
 
     virCheckFlags(VIR_DOMAIN_SHUTDOWN_INITCTL |
                   VIR_DOMAIN_SHUTDOWN_SIGNAL, -1);
@@ -3300,17 +3300,19 @@ lxcDomainShutdownFlags(virDomainPtr dom,
         (flags & VIR_DOMAIN_SHUTDOWN_INITCTL)) {
         int command = VIR_INITCTL_RUNLEVEL_POWEROFF;
 
-        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;
-            }
+        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;
         }
+    } else {
+        rc = 0;
     }
 
-    if (rc < 0 &&
+    if (rc == 0 &&
         (flags == 0 ||
          (flags & VIR_DOMAIN_SHUTDOWN_SIGNAL))) {
         if (kill(priv->initpid, SIGTERM) < 0 &&
@@ -3347,7 +3349,7 @@ lxcDomainReboot(virDomainPtr dom,
     virLXCDomainObjPrivatePtr priv;
     virDomainObjPtr vm;
     int ret = -1;
-    int rc = -1;
+    int rc;
 
     virCheckFlags(VIR_DOMAIN_REBOOT_INITCTL |
                   VIR_DOMAIN_REBOOT_SIGNAL, -1);
@@ -3376,17 +3378,19 @@ lxcDomainReboot(virDomainPtr dom,
         (flags & VIR_DOMAIN_REBOOT_INITCTL)) {
         int command = VIR_INITCTL_RUNLEVEL_REBOOT;
 
-        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;
-            }
+        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;
         }
+    } else {
+        rc = 0;
     }
 
-    if (rc < 0 &&
+    if (rc == 0 &&
         (flags == 0 ||
          (flags & VIR_DOMAIN_REBOOT_SIGNAL))) {
         if (kill(priv->initpid, SIGHUP) < 0 &&
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 8/29/19 1:26 PM, Daniel P. Berrangé wrote:
> This reverts commit 14b6a1854fb4c02c5fb2f51679f8ff099f28f53c.
> 
> If virLXCDomainSetRunlevel returns -1 this indicates a serious
> error / failure that must be propagated to the caller. We must
> not carry on with other shutdown methods in this case.
> 
> If virLXCDomainSetRunlevel return 0, this indicates that no
> initctl was found and it is thus reasonable to fallback to
> sending SIGTERM.
> 
> The commit being reverted is broken because it would fallback
> to SIGTERM when virLXCDomainSetRunlevel returns -1, and would
> not fallback when virLXCDomainSetRunlevel returns 0. ie it
> did the exact opposite of what was required.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   src/lxc/lxc_driver.c | 40 ++++++++++++++++++++++------------------
>   1 file changed, 22 insertions(+), 18 deletions(-)

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list