From: Alan Jenkins <alan.christopher.jenkins@gmail.com>
Date: Thu, 24 Aug 2017 09:33:24 +0000 (+0100)
Subject: logind: add missing resume signal when we fail to initiate sleep/shutdown
X-Git-Tag: v235~192^2~1
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=6d7f7fd49f8a094fc36e750de1e80afea80c8228;p=platform%2Fupstream%2Fsystemd.git

logind: add missing resume signal when we fail to initiate sleep/shutdown

This fixed https://bugzilla.redhat.com/show_bug.cgi?id=1476313
as much as I was able to reproduce it in a VM, at least.

E.g. this signal might wake the screen back up, providing a more visible
indicator of suspend failure.  In my VM testing, it was also required in
order to unblock keyboard input in gnome-shell after the failed suspend.

At the same time, fix the error handling for scheduled shutdowns.  This now
mirrors the behaviour of when you use `shutdown -k` - it sends all the
scary messages about shutting down, "but you'll have to do it [shut down
the system] yourself".  It also avoids the risk of locking out the admin
(nologin file), in case they logged out for some reason (and they use
`sudo` instead of root).

Not that I have any idea why you'd want to use `shutdown -k`, but the code
is easier to analyze if it rolls back on error (in the absence of any code
comment as to why that's not wanted).
---

diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index 6793274..77dd278 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -1480,6 +1480,28 @@ int manager_set_lid_switch_ignore(Manager *m, usec_t until) {
         return r;
 }
 
+static int send_prepare_for(Manager *m, InhibitWhat w, bool _active) {
+
+        static const char * const signal_name[_INHIBIT_WHAT_MAX] = {
+                [INHIBIT_SHUTDOWN] = "PrepareForShutdown",
+                [INHIBIT_SLEEP] = "PrepareForSleep"
+        };
+
+        int active = _active;
+
+        assert(m);
+        assert(w >= 0);
+        assert(w < _INHIBIT_WHAT_MAX);
+        assert(signal_name[w]);
+
+        return sd_bus_emit_signal(m->bus,
+                                  "/org/freedesktop/login1",
+                                  "org.freedesktop.login1.Manager",
+                                  signal_name[w],
+                                  "b",
+                                  active);
+}
+
 static int execute_shutdown_or_sleep(
                 Manager *m,
                 InhibitWhat w,
@@ -1509,15 +1531,17 @@ static int execute_shutdown_or_sleep(
                         &reply,
                         "ss", unit_name, "replace-irreversibly");
         if (r < 0)
-                return r;
+                goto error;
 
         r = sd_bus_message_read(reply, "o", &p);
         if (r < 0)
-                return r;
+                goto error;
 
         c = strdup(p);
-        if (!c)
-                return -ENOMEM;
+        if (!c) {
+                r = -ENOMEM;
+                goto error;
+        }
 
         m->action_unit = unit_name;
         free(m->action_job);
@@ -1528,6 +1552,12 @@ static int execute_shutdown_or_sleep(
         manager_set_lid_switch_ignore(m, now(CLOCK_MONOTONIC) + m->holdoff_timeout_usec);
 
         return 0;
+
+error:
+        /* Tell people that they now may take a lock again */
+        send_prepare_for(m, m->action_what, false);
+
+        return r;
 }
 
 int manager_dispatch_delayed(Manager *manager, bool timeout) {
@@ -1558,7 +1588,8 @@ int manager_dispatch_delayed(Manager *manager, bool timeout) {
         /* Actually do the operation */
         r = execute_shutdown_or_sleep(manager, manager->action_what, manager->action_unit, &error);
         if (r < 0) {
-                log_warning("Failed to send delayed message: %s", bus_error_message(&error, r));
+                log_warning("Error during inhibitor-delayed operation (already returned success to client): %s",
+                            bus_error_message(&error, r));
 
                 manager->action_unit = NULL;
                 manager->action_what = 0;
@@ -1619,28 +1650,6 @@ static int delay_shutdown_or_sleep(
         return 0;
 }
 
-static int send_prepare_for(Manager *m, InhibitWhat w, bool _active) {
-
-        static const char * const signal_name[_INHIBIT_WHAT_MAX] = {
-                [INHIBIT_SHUTDOWN] = "PrepareForShutdown",
-                [INHIBIT_SLEEP] = "PrepareForSleep"
-        };
-
-        int active = _active;
-
-        assert(m);
-        assert(w >= 0);
-        assert(w < _INHIBIT_WHAT_MAX);
-        assert(signal_name[w]);
-
-        return sd_bus_emit_signal(m->bus,
-                                  "/org/freedesktop/login1",
-                                  "org.freedesktop.login1.Manager",
-                                  signal_name[w],
-                                  "b",
-                                  active);
-}
-
 int bus_manager_shutdown_or_sleep_now_or_later(
                 Manager *m,
                 const char *unit_name,
@@ -1940,8 +1949,9 @@ static int manager_scheduled_shutdown_handler(
 
         /* Don't allow multiple jobs being executed at the same time */
         if (m->action_what) {
+                r = -EALREADY;
                 log_error("Scheduled shutdown to %s failed: shutdown or sleep operation already in progress", target);
-                return -EALREADY;
+                goto error;
         }
 
         if (m->shutdown_dry_run) {
@@ -1958,10 +1968,16 @@ static int manager_scheduled_shutdown_handler(
         }
 
         r = bus_manager_shutdown_or_sleep_now_or_later(m, target, INHIBIT_SHUTDOWN, &error);
-        if (r < 0)
-                return log_error_errno(r, "Scheduled shutdown to %s failed: %m", target);
+        if (r < 0) {
+                log_error_errno(r, "Scheduled shutdown to %s failed: %m", target);
+                goto error;
+        }
 
         return 0;
+
+error:
+        reset_scheduled_shutdown(m);
+        return r;
 }
 
 static int method_schedule_shutdown(sd_bus_message *message, void *userdata, sd_bus_error *error) {