systemctl: warn when operating on service files that changed on disk but haven't...

diff --git a/fixme b/fixme
index c6978fb186aa8bb3fe6f8666275b21648081d90d..048ea214059b12ca4017f567d42174bdae88520c 100644 (file)
--- a/fixme
+++ b/fixme
@@ -37,22 +37,12 @@
 
 * timeout waiting for mount devices?
 
 
 * timeout waiting for mount devices?
 

-* default logic for serial getty, ck logging, ssh readahead
-

 * place /etc/inittab with explaining blurb.
 
 * place /etc/inittab with explaining blurb.
 

-* OnFailure=foo.unit
-
-* default.target must be %ghosted...
-

 * In command lines, support both "$FOO" and $FOO
 
 * systemd-install disable should recursively kill all symlinks
 
 * In command lines, support both "$FOO" and $FOO
 
 * systemd-install disable should recursively kill all symlinks
 

-* in %post create all symlinks manually and use inittab data
-
-* check mtimes of dirs and unit files in systemctl
-

 * /etc must always take precedence even if we follow symlinks!
 
 * /lib/init/rw
 * /etc must always take precedence even if we follow symlinks!
 
 * /lib/init/rw


@@ -69,6 +59,12 @@
 
 External:
 
 
 External:
 

+* default.target must be %ghosted...
+
+* in %post create all symlinks manually and use inittab data
+
+* default logic for serial getty, ck logging, ssh readahead
+

 * patch /etc/init.d/functions with:
 
   if [ $PPID -ne 1 && mountpoint /cgroup/systemd ] ; then echo "You suck!" ; fi
 * patch /etc/init.d/functions with:
 
   if [ $PPID -ne 1 && mountpoint /cgroup/systemd ] ; then echo "You suck!" ; fi

diff --git a/src/dbus-unit.c b/src/dbus-unit.c
index da7d1bd5b791255e677492f173cf507a33fbf7ef..66b7ae829757edc24800d9e04d7ddbd9f077f8d6 100644 (file)
--- a/src/dbus-unit.c
+++ b/src/dbus-unit.c
@@ -254,6 +254,23 @@ int bus_unit_append_cgroups(Manager *m, DBusMessageIter *i, const char *property
         return 0;
 }
 
         return 0;
 }
 

+int bus_unit_append_need_daemon_reload(Manager *m, DBusMessageIter *i, const char *property, void *data) {
+        Unit *u = data;
+        dbus_bool_t b;
+
+        assert(m);
+        assert(i);
+        assert(property);
+        assert(u);
+
+        b = unit_need_daemon_reload(u);
+
+        if (!dbus_message_iter_append_basic(i, DBUS_TYPE_BOOLEAN, &b))
+                return -ENOMEM;
+
+        return 0;
+}
+

 static DBusHandlerResult bus_unit_message_dispatch(Unit *u, DBusConnection *connection, DBusMessage *message) {
         DBusMessage *reply = NULL;
         Manager *m = u->meta.manager;
 static DBusHandlerResult bus_unit_message_dispatch(Unit *u, DBusConnection *connection, DBusMessage *message) {
         DBusMessage *reply = NULL;
         Manager *m = u->meta.manager;

diff --git a/src/dbus-unit.h b/src/dbus-unit.h
index 1d11af3a04e2c2e7ea5a2f0720acd01cd3f5a763..d4af5a8b2384d50ddf51ae01d0064e8a727c069a 100644 (file)
--- a/src/dbus-unit.h
+++ b/src/dbus-unit.h
@@ -88,6 +88,7 @@
         "  <property name=\"DefaultDependencies\" type=\"b\" access=\"read\"/>\n" \
         "  <property name=\"DefaultControlGroup\" type=\"s\" access=\"read\"/>\n" \
         "  <property name=\"ControlGroups\" type=\"as\" access=\"read\"/>\n" \
         "  <property name=\"DefaultDependencies\" type=\"b\" access=\"read\"/>\n" \
         "  <property name=\"DefaultControlGroup\" type=\"s\" access=\"read\"/>\n" \
         "  <property name=\"ControlGroups\" type=\"as\" access=\"read\"/>\n" \

+        "  <property name=\"NeedDaemonReload\" type=\"b\" access=\"read\"/>\n" \

         " </interface>\n"
 
 #define BUS_UNIT_PROPERTIES \
         " </interface>\n"
 
 #define BUS_UNIT_PROPERTIES \


@@ -121,7 +122,8 @@
         { "org.freedesktop.systemd1.Unit", "OnlyByDependency",     bus_property_append_bool,       "b",    &u->meta.only_by_dependency       }, \
         { "org.freedesktop.systemd1.Unit", "DefaultDependencies",  bus_property_append_bool,       "b",    &u->meta.default_dependencies     }, \
         { "org.freedesktop.systemd1.Unit", "DefaultControlGroup",  bus_unit_append_default_cgroup, "s",    u                                 }, \
         { "org.freedesktop.systemd1.Unit", "OnlyByDependency",     bus_property_append_bool,       "b",    &u->meta.only_by_dependency       }, \
         { "org.freedesktop.systemd1.Unit", "DefaultDependencies",  bus_property_append_bool,       "b",    &u->meta.default_dependencies     }, \
         { "org.freedesktop.systemd1.Unit", "DefaultControlGroup",  bus_unit_append_default_cgroup, "s",    u                                 }, \

-        { "org.freedesktop.systemd1.Unit", "ControlGroups",        bus_unit_append_cgroups,        "as",   u                                 }
+        { "org.freedesktop.systemd1.Unit", "ControlGroups",        bus_unit_append_cgroups,        "as",   u                                 }, \
+        { "org.freedesktop.systemd1.Unit", "NeedDaemonReload",     bus_unit_append_need_daemon_reload, "b", u                                }

 
 int bus_unit_append_names(Manager *m, DBusMessageIter *i, const char *property, void *data);
 int bus_unit_append_dependencies(Manager *m, DBusMessageIter *i, const char *property, void *data);
 
 int bus_unit_append_names(Manager *m, DBusMessageIter *i, const char *property, void *data);
 int bus_unit_append_dependencies(Manager *m, DBusMessageIter *i, const char *property, void *data);


@@ -134,6 +136,7 @@ int bus_unit_append_can_reload(Manager *m, DBusMessageIter *i, const char *prope
 int bus_unit_append_job(Manager *m, DBusMessageIter *i, const char *property, void *data);
 int bus_unit_append_default_cgroup(Manager *m, DBusMessageIter *i, const char *property, void *data);
 int bus_unit_append_cgroups(Manager *m, DBusMessageIter *i, const char *property, void *data);
 int bus_unit_append_job(Manager *m, DBusMessageIter *i, const char *property, void *data);
 int bus_unit_append_default_cgroup(Manager *m, DBusMessageIter *i, const char *property, void *data);
 int bus_unit_append_cgroups(Manager *m, DBusMessageIter *i, const char *property, void *data);

+int bus_unit_append_need_daemon_reload(Manager *m, DBusMessageIter *i, const char *property, void *data);

 
 void bus_unit_send_change_signal(Unit *u);
 void bus_unit_send_removed_signal(Unit *u);
 
 void bus_unit_send_change_signal(Unit *u);
 void bus_unit_send_removed_signal(Unit *u);

diff --git a/src/load-fragment.c b/src/load-fragment.c
index 8e4ec74b0735b98e55ee2bcd1ee665298fa2a673..a2974cbeaded5bb595c610c16fdbd259276f1823 100644 (file)
--- a/src/load-fragment.c
+++ b/src/load-fragment.c
@@ -29,6 +29,7 @@
 #include <sys/prctl.h>
 #include <sys/mount.h>
 #include <linux/fs.h>
 #include <sys/prctl.h>
 #include <sys/mount.h>
 #include <linux/fs.h>

+#include <sys/stat.h>

 
 #include "unit.h"
 #include "strv.h"
 
 #include "unit.h"
 #include "strv.h"


@@ -1558,6 +1559,7 @@ static int load_from_path(Unit *u, const char *path) {
                 { "Conflicts",              config_parse_deps,            UINT_TO_PTR(UNIT_CONFLICTS),                     "Unit"    },
                 { "Before",                 config_parse_deps,            UINT_TO_PTR(UNIT_BEFORE),                        "Unit"    },
                 { "After",                  config_parse_deps,            UINT_TO_PTR(UNIT_AFTER),                         "Unit"    },
                 { "Conflicts",              config_parse_deps,            UINT_TO_PTR(UNIT_CONFLICTS),                     "Unit"    },
                 { "Before",                 config_parse_deps,            UINT_TO_PTR(UNIT_BEFORE),                        "Unit"    },
                 { "After",                  config_parse_deps,            UINT_TO_PTR(UNIT_AFTER),                         "Unit"    },

+                { "OnFailure",              config_parse_deps,            UINT_TO_PTR(UNIT_ON_FAILURE),                    "Unit"    },

                 { "RecursiveStop",          config_parse_bool,            &u->meta.recursive_stop,                         "Unit"    },
                 { "StopWhenUnneeded",       config_parse_bool,            &u->meta.stop_when_unneeded,                     "Unit"    },
                 { "OnlyByDependency",       config_parse_bool,            &u->meta.only_by_dependency,                     "Unit"    },
                 { "RecursiveStop",          config_parse_bool,            &u->meta.recursive_stop,                         "Unit"    },
                 { "StopWhenUnneeded",       config_parse_bool,            &u->meta.stop_when_unneeded,                     "Unit"    },
                 { "OnlyByDependency",       config_parse_bool,            &u->meta.only_by_dependency,                     "Unit"    },


@@ -1653,6 +1655,7 @@ static int load_from_path(Unit *u, const char *path) {
         FILE *f = NULL;
         char *filename = NULL, *id = NULL;
         Unit *merged;
         FILE *f = NULL;
         char *filename = NULL, *id = NULL;
         Unit *merged;

+        struct stat st;

 
         if (!u) {
                 /* Dirty dirty hack. */
 
         if (!u) {
                 /* Dirty dirty hack. */


@@ -1740,6 +1743,17 @@ static int load_from_path(Unit *u, const char *path) {
                 goto finish;
         }
 
                 goto finish;
         }
 

+        zero(st);
+        if (fstat(fileno(f), &st) < 0) {
+                r = -errno;
+                goto finish;
+        }
+
+        if (!S_ISREG(st.st_mode)) {
+                r = -ENOENT;
+                goto finish;
+        }
+

         /* Now, parse the file contents */
         if ((r = config_parse(filename, f, sections, items, false, u)) < 0)
                 goto finish;
         /* Now, parse the file contents */
         if ((r = config_parse(filename, f, sections, items, false, u)) < 0)
                 goto finish;


@@ -1748,6 +1762,8 @@ static int load_from_path(Unit *u, const char *path) {
         u->meta.fragment_path = filename;
         filename = NULL;
 
         u->meta.fragment_path = filename;
         filename = NULL;
 

+        u->meta.fragment_mtime = timespec_load(&st.st_mtim);
+

         u->meta.load_state = UNIT_LOADED;
         r = 0;
 
         u->meta.load_state = UNIT_LOADED;
         r = 0;
 

diff --git a/src/systemctl.c b/src/systemctl.c
index 1ded936954279755b7f643fc66e6dc49582c1a78..3806786949daee05e861889332cbdac0d76cf52f 100644 (file)
--- a/src/systemctl.c
+++ b/src/systemctl.c
@@ -742,6 +742,77 @@ finish:
         return r;
 }
 
         return r;
 }
 

+static bool unit_need_daemon_reload(DBusConnection *bus, const char *unit) {
+        DBusMessage *m, *reply;
+        dbus_bool_t b = FALSE;
+        DBusMessageIter iter, sub;
+        const char
+                *interface = "org.freedesktop.systemd1.Unit",
+                *property = "NeedDaemonReload",
+                *path;
+
+        /* We ignore all errors here, since this is used to show a warning only */
+
+        if (!(m = dbus_message_new_method_call(
+                              "org.freedesktop.systemd1",
+                              "/org/freedesktop/systemd1",
+                              "org.freedesktop.systemd1.Manager",
+                              "GetUnit")))
+                goto finish;
+
+        if (!dbus_message_append_args(m,
+                                      DBUS_TYPE_STRING, &unit,
+                                      DBUS_TYPE_INVALID))
+                goto finish;
+
+        if (!(reply = dbus_connection_send_with_reply_and_block(bus, m, -1, NULL)))
+                goto finish;
+
+        if (!dbus_message_get_args(reply, NULL,
+                                   DBUS_TYPE_OBJECT_PATH, &path,
+                                   DBUS_TYPE_INVALID))
+                goto finish;
+
+        dbus_message_unref(m);
+        if (!(m = dbus_message_new_method_call(
+                              "org.freedesktop.systemd1",
+                              path,
+                              "org.freedesktop.DBus.Properties",
+                              "Get")))
+                goto finish;
+
+        if (!dbus_message_append_args(m,
+                                      DBUS_TYPE_STRING, &interface,
+                                      DBUS_TYPE_STRING, &property,
+                                      DBUS_TYPE_INVALID)) {
+                goto finish;
+        }
+
+        dbus_message_unref(reply);
+        if (!(reply = dbus_connection_send_with_reply_and_block(bus, m, -1, NULL)))
+                goto finish;
+
+        if (!dbus_message_iter_init(reply, &iter) ||
+            dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
+                goto finish;
+
+        dbus_message_iter_recurse(&iter, &sub);
+
+        if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_BOOLEAN)
+                goto finish;
+
+        dbus_message_iter_get_basic(&sub, &b);
+
+finish:
+        if (m)
+                dbus_message_unref(m);
+
+        if (reply)
+                dbus_message_unref(reply);
+
+        return b;
+}
+

 typedef struct WaitData {
         Set *set;
         bool failed;
 typedef struct WaitData {
         Set *set;
         bool failed;


@@ -860,6 +931,7 @@ static int start_unit_one(
 
         DBusMessage *m = NULL, *reply = NULL;
         DBusError error;
 
         DBusMessage *m = NULL, *reply = NULL;
         DBusError error;

+        const char *path;

         int r;
 
         assert(bus);
         int r;
 
         assert(bus);


@@ -903,18 +975,21 @@ static int start_unit_one(
                 goto finish;
         }
 
                 goto finish;
         }
 

+        if (!dbus_message_get_args(reply, &error,
+                                   DBUS_TYPE_OBJECT_PATH, &path,
+                                   DBUS_TYPE_INVALID)) {
+                log_error("Failed to parse reply: %s", error.message);
+                r = -EIO;
+                goto finish;
+        }
+
+        if (unit_need_daemon_reload(bus, name))
+                log_warning("Unit file of created job changed on disk, 'systemctl %s daemon-reload' recommended.",
+                            arg_session ? "--session" : "--system");
+

         if (!arg_no_block) {
         if (!arg_no_block) {

-                const char *path;

                 char *p;
 
                 char *p;
 

-                if (!dbus_message_get_args(reply, &error,
-                                           DBUS_TYPE_OBJECT_PATH, &path,
-                                           DBUS_TYPE_INVALID)) {
-                        log_error("Failed to parse reply: %s", error.message);
-                        r = -EIO;
-                        goto finish;
-                }
-

                 if (!(p = strdup(path))) {
                         log_error("Failed to duplicate path.");
                         r = -ENOMEM;
                 if (!(p = strdup(path))) {
                         log_error("Failed to duplicate path.");
                         r = -ENOMEM;


@@ -1287,6 +1362,8 @@ typedef struct UnitStatusInfo {
         const char *fragment_path;
         const char *default_control_group;
 
         const char *fragment_path;
         const char *default_control_group;
 

+        bool need_daemon_reload;
+

         /* Service */
         pid_t main_pid;
         pid_t control_pid;
         /* Service */
         pid_t main_pid;
         pid_t control_pid;


@@ -1449,6 +1526,10 @@ static void print_status_info(UnitStatusInfo *i) {
 
                 show_cgroup_by_path(i->default_control_group, "\t\t  ", c);
         }
 
                 show_cgroup_by_path(i->default_control_group, "\t\t  ", c);
         }

+
+        if (i->need_daemon_reload)
+                printf("\n" ANSI_HIGHLIGHT_ON "Warning:" ANSI_HIGHLIGHT_OFF " Unit file changed on disk, 'systemctl %s daemon-reload' recommended.\n",
+                       arg_session ? "--session" : "--system");

 }
 
 static int status_property(const char *name, DBusMessageIter *iter, UnitStatusInfo *i) {
 }
 
 static int status_property(const char *name, DBusMessageIter *iter, UnitStatusInfo *i) {


@@ -1495,6 +1576,8 @@ static int status_property(const char *name, DBusMessageIter *iter, UnitStatusIn
 
                 if (streq(name, "Accept"))
                         i->accept = b;
 
                 if (streq(name, "Accept"))
                         i->accept = b;

+                else if (streq(name, "NeedDaemonReload"))
+                        i->need_daemon_reload = b;

 
                 break;
         }
 
                 break;
         }

diff --git a/src/unit.c b/src/unit.c
index f8be8b26a148bd9418348ad23ce48d7160ed5b9c..e46182adad420a8ffb1144fc2cc80da3c8f0411c 100644 (file)
--- a/src/unit.c
+++ b/src/unit.c
@@ -27,6 +27,7 @@
 #include <sys/poll.h>
 #include <stdlib.h>
 #include <unistd.h>
 #include <sys/poll.h>
 #include <stdlib.h>
 #include <unistd.h>

+#include <sys/stat.h>

 
 #include "set.h"
 #include "unit.h"
 
 #include "set.h"
 #include "unit.h"


@@ -606,7 +607,8 @@ void unit_dump(Unit *u, FILE *f, const char *prefix) {
                 "%s\tActive Enter Timestamp: %s\n"
                 "%s\tActive Exit Timestamp: %s\n"
                 "%s\tInactive Enter Timestamp: %s\n"
                 "%s\tActive Enter Timestamp: %s\n"
                 "%s\tActive Exit Timestamp: %s\n"
                 "%s\tInactive Enter Timestamp: %s\n"

-                "%s\tGC Check Good: %s\n",
+                "%s\tGC Check Good: %s\n"
+                "%s\tNeed Daemon Reload: %s\n",

                 prefix, u->meta.id,
                 prefix, unit_description(u),
                 prefix, strna(u->meta.instance),
                 prefix, u->meta.id,
                 prefix, unit_description(u),
                 prefix, strna(u->meta.instance),


@@ -616,7 +618,8 @@ void unit_dump(Unit *u, FILE *f, const char *prefix) {
                 prefix, strna(format_timestamp(timestamp2, sizeof(timestamp2), u->meta.active_enter_timestamp.realtime)),
                 prefix, strna(format_timestamp(timestamp3, sizeof(timestamp3), u->meta.active_exit_timestamp.realtime)),
                 prefix, strna(format_timestamp(timestamp4, sizeof(timestamp4), u->meta.inactive_enter_timestamp.realtime)),
                 prefix, strna(format_timestamp(timestamp2, sizeof(timestamp2), u->meta.active_enter_timestamp.realtime)),
                 prefix, strna(format_timestamp(timestamp3, sizeof(timestamp3), u->meta.active_exit_timestamp.realtime)),
                 prefix, strna(format_timestamp(timestamp4, sizeof(timestamp4), u->meta.inactive_enter_timestamp.realtime)),

-                prefix, yes_no(unit_check_gc(u)));
+                prefix, yes_no(unit_check_gc(u)),
+                prefix, yes_no(unit_need_daemon_reload(u)));

 
         SET_FOREACH(t, u->meta.names, i)
                 fprintf(f, "%s\tName: %s\n", prefix, t);
 
         SET_FOREACH(t, u->meta.names, i)
                 fprintf(f, "%s\tName: %s\n", prefix, t);


@@ -2029,6 +2032,24 @@ void unit_status_printf(Unit *u, const char *format, ...) {
         va_end(ap);
 }
 
         va_end(ap);
 }
 

+bool unit_need_daemon_reload(Unit *u) {
+        struct stat st;
+
+        assert(u);
+
+        if (!u->meta.fragment_path)
+                return false;
+
+        zero(st);
+        if (stat(u->meta.fragment_path, &st) < 0)
+                /* What, cannot access this anymore? */
+                return true;
+
+        return
+                u->meta.fragment_mtime &&
+                timespec_load(&st.st_mtim) != u->meta.fragment_mtime;
+}
+

 static const char* const unit_type_table[_UNIT_TYPE_MAX] = {
         [UNIT_SERVICE] = "service",
         [UNIT_TIMER] = "timer",
 static const char* const unit_type_table[_UNIT_TYPE_MAX] = {
         [UNIT_SERVICE] = "service",
         [UNIT_TIMER] = "timer",

diff --git a/src/unit.h b/src/unit.h
index 5d68583f10c267c50e61e1596a90f0ddbde4ca1c..d3a00797f996ebd853472f38cf9d0b190121141d 100644 (file)
--- a/src/unit.h
+++ b/src/unit.h
@@ -141,6 +141,7 @@ struct Meta {
 
         char *description;
         char *fragment_path; /* if loaded from a config file this is the primary path to it */
 
         char *description;
         char *fragment_path; /* if loaded from a config file this is the primary path to it */

+        usec_t fragment_mtime;

 
         /* If there is something to do with this unit, then this is
          * the job for it */
 
         /* If there is something to do with this unit, then this is
          * the job for it */


@@ -458,6 +459,8 @@ int unit_coldplug(Unit *u);
 
 void unit_status_printf(Unit *u, const char *format, ...);
 
 
 void unit_status_printf(Unit *u, const char *format, ...);
 

+bool unit_need_daemon_reload(Unit *u);
+

 const char *unit_type_to_string(UnitType i);
 UnitType unit_type_from_string(const char *s);
 
 const char *unit_type_to_string(UnitType i);
 UnitType unit_type_from_string(const char *s);