Merge pull request #13866 from keszybz/nspawn-restarts

Make 'machinectl reboot' functional

src/basic/string-util.c

@@ -743,7 +743,7 @@ static void advance_offsets(
 }
 
 char *strip_tab_ansi(char **ibuf, size_t *_isz, size_t highlight[2]) {
-        const char *i, *begin = NULL;
+        const char *begin = NULL;
         enum {
                 STATE_OTHER,
                 STATE_ESCAPE,
@@ -751,7 +751,7 @@ char *strip_tab_ansi(char **ibuf, size_t *_isz, size_t highlight[2]) {
                 STATE_CSO,
         } state = STATE_OTHER;
         char *obuf = NULL;
-        size_t osz = 0, isz, shift[2] = {};
+        size_t osz = 0, isz, shift[2] = {}, n_carriage_returns = 0;
         FILE *f;
 
         assert(ibuf);
@@ -762,6 +762,8 @@ char *strip_tab_ansi(char **ibuf, size_t *_isz, size_t highlight[2]) {
          * 1. Replaces TABs by 8 spaces
          * 2. Strips ANSI color sequences (a subset of CSI), i.e. ESC '[' … 'm' sequences
          * 3. Strips ANSI operating system sequences (CSO), i.e. ESC ']' … BEL sequences
+         * 4. Strip trailing \r characters (since they would "move the cursor", but have no
+         *    other effect).
          *
          * Everything else will be left as it is. In particular other ANSI sequences are left as they are, as
          * are any other special characters. Truncated ANSI sequences are left-as is too. This call is
@@ -777,14 +779,24 @@ char *strip_tab_ansi(char **ibuf, size_t *_isz, size_t highlight[2]) {
         if (!f)
                 return NULL;
 
-        for (i = *ibuf; i < *ibuf + isz + 1; i++) {
+        for (const char *i = *ibuf; i < *ibuf + isz + 1; i++) {
 
                 switch (state) {
 
                 case STATE_OTHER:
                         if (i >= *ibuf + isz) /* EOT */
                                 break;
-                        else if (*i == '\x1B')
+
+                        if (*i == '\r') {
+                                n_carriage_returns++;
+                                break;
+                        } else if (*i == '\n')
+                                /* Ignore carriage returns before new line */
+                                n_carriage_returns = 0;
+                        for (; n_carriage_returns > 0; n_carriage_returns--)
+                                fputc('\r', f);
+
+                        if (*i == '\x1B')
                                 state = STATE_ESCAPE;
                         else if (*i == '\t') {
                                 fputs("        ", f);
@@ -795,6 +807,8 @@ char *strip_tab_ansi(char **ibuf, size_t *_isz, size_t highlight[2]) {
                         break;
 
                 case STATE_ESCAPE:
+                        assert(n_carriage_returns == 0);
+
                         if (i >= *ibuf + isz) { /* EOT */
                                 fputc('\x1B', f);
                                 advance_offsets(i - *ibuf, highlight, shift, 1);
@@ -815,6 +829,7 @@ char *strip_tab_ansi(char **ibuf, size_t *_isz, size_t highlight[2]) {
                         break;
 
                 case STATE_CSI:
+                        assert(n_carriage_returns == 0);
 
                         if (i >= *ibuf + isz || /* EOT … */
                             !strchr("01234567890;m", *i)) { /* … or invalid chars in sequence */
@@ -829,6 +844,7 @@ char *strip_tab_ansi(char **ibuf, size_t *_isz, size_t highlight[2]) {
                         break;
 
                 case STATE_CSO:
+                        assert(n_carriage_returns == 0);
 
                         if (i >= *ibuf + isz || /* EOT … */
                             (*i != '\a' && (uint8_t) *i < 32U) || (uint8_t) *i > 126U) { /* … or invalid chars in sequence */
@@ -848,7 +864,6 @@ char *strip_tab_ansi(char **ibuf, size_t *_isz, size_t highlight[2]) {
                 fclose(f);
                 return mfree(obuf);
         }
-
         fclose(f);
 
         free_and_replace(*ibuf, obuf);

src/core/service.c

@@ -1658,21 +1658,28 @@ static int cgroup_good(Service *s) {
         return r == 0;
 }
 
-static bool service_shall_restart(Service *s) {
+static bool service_shall_restart(Service *s, const char **reason) {
         assert(s);
 
         /* Don't restart after manual stops */
-        if (s->forbid_restart)
+        if (s->forbid_restart) {
+                *reason = "manual stop";
                 return false;
+        }
 
         /* Never restart if this is configured as special exception */
-        if (exit_status_set_test(&s->restart_prevent_status, s->main_exec_status.code, s->main_exec_status.status))
+        if (exit_status_set_test(&s->restart_prevent_status, s->main_exec_status.code, s->main_exec_status.status)) {
+                *reason = "prevented by exit status";
                 return false;
+        }
 
         /* Restart if the exit code/status are configured as restart triggers */
-        if (exit_status_set_test(&s->restart_force_status,  s->main_exec_status.code, s->main_exec_status.status))
+        if (exit_status_set_test(&s->restart_force_status,  s->main_exec_status.code, s->main_exec_status.status)) {
+                *reason = "forced by exit status";
                 return true;
+        }
 
+        *reason = "restart setting";
         switch (s->restart) {
 
         case SERVICE_RESTART_NO:
@@ -1739,8 +1746,19 @@ static void service_enter_dead(Service *s, ServiceResult f, bool allow_restart)
                 end_state = SERVICE_FAILED;
         }
 
-        if (allow_restart && service_shall_restart(s))
-                s->will_auto_restart = true;
+        if (!allow_restart)
+                log_unit_debug(UNIT(s), "Service restart not allowed.");
+        else {
+                const char *reason;
+                bool shall_restart;
+
+                shall_restart = service_shall_restart(s, &reason);
+                log_unit_debug(UNIT(s), "Service will %srestart (%s)",
+                                        shall_restart ? "" : "not ",
+                                        reason);
+                if (shall_restart)
+                        s->will_auto_restart = true;
+        }
 
         /* Make sure service_release_resources() doesn't destroy our FD store, while we are changing through
          * SERVICE_FAILED/SERVICE_DEAD before entering into SERVICE_AUTO_RESTART. */

src/machine/machine-dbus.c

@@ -62,6 +62,34 @@ static int property_get_netif(
         return sd_bus_message_append_array(reply, 'i', m->netif, m->n_netif * sizeof(int));
 }
 
+int bus_machine_method_unregister(sd_bus_message *message, void *userdata, sd_bus_error *error) {
+        Machine *m = userdata;
+        int r;
+
+        assert(message);
+        assert(m);
+
+        r = bus_verify_polkit_async(
+                        message,
+                        CAP_KILL,
+                        "org.freedesktop.machine1.manage-machines",
+                        NULL,
+                        false,
+                        UID_INVALID,
+                        &m->manager->polkit_registry,
+                        error);
+        if (r < 0)
+                return r;
+        if (r == 0)
+                return 1; /* Will call us back */
+
+        r = machine_finalize(m);
+        if (r < 0)
+                return r;
+
+        return sd_bus_reply_method_return(message, NULL);
+}
+
 int bus_machine_method_terminate(sd_bus_message *message, void *userdata, sd_bus_error *error) {
         Machine *m = userdata;
         int r;

src/machine/machine-dbus.h

@@ -11,6 +11,7 @@ char *machine_bus_path(Machine *s);
 int machine_object_find(sd_bus *bus, const char *path, const char *interface, void *userdata, void **found, sd_bus_error *error);
 int machine_node_enumerator(sd_bus *bus, const char *path, void *userdata, char ***nodes, sd_bus_error *error);
 
+int bus_machine_method_unregister(sd_bus_message *message, void *userdata, sd_bus_error *error);
 int bus_machine_method_terminate(sd_bus_message *message, void *userdata, sd_bus_error *error);
 int bus_machine_method_kill(sd_bus_message *message, void *userdata, sd_bus_error *error);
 int bus_machine_method_get_addresses(sd_bus_message *message, void *userdata, sd_bus_error *error);

src/machine/machine.c

@@ -355,6 +355,7 @@ static int machine_start_scope(Machine *m, sd_bus_message *properties, sd_bus_er
                         return log_error_errno(r, "Failed to start machine scope: %s", bus_error_message(error, r));
 
                 m->unit = TAKE_PTR(scope);
+                m->referenced = true;
                 free_and_replace(m->scope_job, job);
         }
 
@@ -422,9 +423,12 @@ static int machine_stop_scope(Machine *m) {
         } else
                 free_and_replace(m->scope_job, job);
 
-        q = manager_unref_unit(m->manager, m->unit, &error);
-        if (q < 0)
-                log_warning_errno(q, "Failed to drop reference to machine scope, ignoring: %s", bus_error_message(&error, r));
+        if (m->referenced) {
+                q = manager_unref_unit(m->manager, m->unit, &error);
+                if (q < 0)
+                        log_warning_errno(q, "Failed to drop reference to machine scope, ignoring: %s", bus_error_message(&error, r));
+                m->referenced = false;
+        }
 
         return r;
 }
@@ -449,13 +453,16 @@ int machine_stop(Machine *m) {
 int machine_finalize(Machine *m) {
         assert(m);
 
-        if (m->started)
+        if (m->started) {
                 log_struct(LOG_INFO,
                            "MESSAGE_ID=" SD_MESSAGE_MACHINE_STOP_STR,
                            "NAME=%s", m->name,
                            "LEADER="PID_FMT, m->leader,
                            LOG_MESSAGE("Machine %s terminated.", m->name));
 
+                m->stopping = true; /* The machine is supposed to be going away. Don't try to kill it. */
+        }
+
         machine_unlink(m);
         machine_add_to_gc_queue(m);
 

src/machine/machine.h

@@ -54,6 +54,7 @@ struct Machine {
         bool in_gc_queue:1;
         bool started:1;
         bool stopping:1;
+        bool referenced:1;
 
         sd_bus_message *create_message;
 

src/machine/machined-dbus.c

@@ -448,6 +448,10 @@ static int redirect_method_to_machine(sd_bus_message *message, Manager *m, sd_bu
         return method(message, machine, error);
 }
 
+static int method_unregister_machine(sd_bus_message *message, void *userdata, sd_bus_error *error) {
+        return redirect_method_to_machine(message, userdata, error, bus_machine_method_unregister);
+}
+
 static int method_terminate_machine(sd_bus_message *message, void *userdata, sd_bus_error *error) {
         return redirect_method_to_machine(message, userdata, error, bus_machine_method_terminate);
 }
@@ -1137,6 +1141,7 @@ const sd_bus_vtable manager_vtable[] = {
         SD_BUS_METHOD("CreateMachineWithNetwork", "sayssusaia(sv)", "o", method_create_machine_with_network, 0),
         SD_BUS_METHOD("RegisterMachine", "sayssus", "o", method_register_machine, 0),
         SD_BUS_METHOD("RegisterMachineWithNetwork", "sayssusai", "o", method_register_machine_with_network, 0),
+        SD_BUS_METHOD("UnregisterMachine", "s", NULL, method_unregister_machine, SD_BUS_VTABLE_UNPRIVILEGED),
         SD_BUS_METHOD("TerminateMachine", "s", NULL, method_terminate_machine, SD_BUS_VTABLE_UNPRIVILEGED),
         SD_BUS_METHOD("KillMachine", "ssi", NULL, method_kill_machine, SD_BUS_VTABLE_UNPRIVILEGED),
         SD_BUS_METHOD("GetMachineAddresses", "s", "a(iay)", method_get_machine_addresses, SD_BUS_VTABLE_UNPRIVILEGED),

src/machine/org.freedesktop.machine1.conf

@@ -76,6 +76,10 @@
                        send_interface="org.freedesktop.machine1.Manager"
                        send_member="OpenMachineShell"/>
 
+                <allow send_destination="org.freedesktop.machine1"
+                       send_interface="org.freedesktop.machine1.Manager"
+                       send_member="UnregisterMachine"/>
+
                 <allow send_destination="org.freedesktop.machine1"
                        send_interface="org.freedesktop.machine1.Manager"
                        send_member="TerminateMachine"/>

src/nspawn/nspawn-register.c

@@ -209,7 +209,7 @@ int register_machine(
         return 0;
 }
 
-int terminate_machine(
+int unregister_machine(
                 sd_bus *bus,
                 const char *machine_name) {
 
@@ -223,13 +223,13 @@ int terminate_machine(
                         "org.freedesktop.machine1",
                         "/org/freedesktop/machine1",
                         "org.freedesktop.machine1.Manager",
-                        "TerminateMachine",
+                        "UnregisterMachine",
                         &error,
                         NULL,
                         "s",
                         machine_name);
         if (r < 0)
-                log_debug("Failed to terminate machine: %s", bus_error_message(&error, r));
+                log_debug("Failed to unregister machine: %s", bus_error_message(&error, r));
 
         return 0;
 }

src/nspawn/nspawn-register.h

@@ -8,7 +8,7 @@
 #include "nspawn-mount.h"
 
 int register_machine(sd_bus *bus, const char *machine_name, pid_t pid, const char *directory, sd_id128_t uuid, int local_ifindex, const char *slice, CustomMount *mounts, unsigned n_mounts, int kill_signal, char **properties, sd_bus_message *properties_message, bool keep_unit, const char *service);
-int terminate_machine(sd_bus *bus, const char *machine_name);
+int unregister_machine(sd_bus *bus, const char *machine_name);
 
 int allocate_scope(sd_bus *bus, const char *machine_name, pid_t pid, const char *slice, CustomMount *mounts, unsigned n_mounts, int kill_signal, char **properties, sd_bus_message *properties_message);
 int terminate_scope(sd_bus *bus, const char *machine_name);

src/nspawn/nspawn.c

@@ -4618,12 +4618,8 @@ static int run_container(
         }
 
         /* Kill if it is not dead yet anyway */
-        if (bus) {
-                if (arg_register)
-                        terminate_machine(bus, arg_machine);
-                else if (!arg_keep_unit)
-                        terminate_scope(bus, arg_machine);
-        }
+        if (!arg_register && !arg_keep_unit && bus)
+                terminate_scope(bus, arg_machine);
 
         /* Normally redundant, but better safe than sorry */
         (void) kill(*pid, SIGKILL);
@@ -4631,6 +4627,10 @@ static int run_container(
         r = wait_for_container(*pid, &container_status);
         *pid = 0;
 
+        /* Tell machined that we are gone. */
+        if (bus)
+                (void) unregister_machine(bus, arg_machine);
+
         if (r < 0)
                 /* We failed to wait for the container, or the container exited abnormally. */
                 return r;

src/test/test-strip-tab-ansi.c

@@ -34,6 +34,21 @@ int main(int argc, char *argv[]) {
         assert_se(streq(p, "\x1B[waldo"));
         free(p);
 
+        assert_se(p = strdup("\r\rwaldo"));
+        assert_se(strip_tab_ansi(&p, NULL, NULL));
+        assert_se(streq(p, "\r\rwaldo"));
+        free(p);
+
+        assert_se(p = strdup("waldo\r\r"));
+        assert_se(strip_tab_ansi(&p, NULL, NULL));
+        assert_se(streq(p, "waldo"));
+        free(p);
+
+        assert_se(p = strdup("waldo\r\r\n\r\n"));
+        assert_se(strip_tab_ansi(&p, NULL, NULL));
+        assert_se(streq(p, "waldo\n\n"));
+        free(p);
+
         assert_se(terminal_urlify_path("/etc/fstab", "i am a fabulous link", &urlified) >= 0);
         assert_se(p = strjoin("something ", urlified, " something-else"));
         assert_se(q = strdup(p));

src/test/test-utf8.c

@@ -13,6 +13,9 @@ static void test_utf8_is_printable(void) {
         assert_se(utf8_is_printable("\342\204\242", 3));
         assert_se(!utf8_is_printable("\341\204", 2));
         assert_se(utf8_is_printable("ąę", 4));
+        assert_se(!utf8_is_printable("\r", 1));
+        assert_se(utf8_is_printable("\n", 1));
+        assert_se(utf8_is_printable("\t", 1));
 }
 
 static void test_utf8_is_valid(void) {