From: Lennart Poettering Date: Fri, 16 Jul 2010 22:57:51 +0000 (+0200) Subject: systemctl: warn when operating on service files that changed on disk but haven't... X-Git-Tag: v4~37 X-Git-Url: http://www.chiark.greenend.org.uk/ucgi/~ianmdlvl/git?p=elogind.git;a=commitdiff_plain;h=45fb0699c45d2e042e04a53e3ea00501e3f65f59 systemctl: warn when operating on service files that changed on disk but haven't been reloaded --- diff --git a/fixme b/fixme index c6978fb18..048ea2140 100644 --- a/fixme +++ b/fixme @@ -37,22 +37,12 @@ * timeout waiting for mount devices? -* default logic for serial getty, ck logging, ssh readahead - * 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 %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 @@ -69,6 +59,12 @@ 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 diff --git a/src/dbus-unit.c b/src/dbus-unit.c index da7d1bd5b..66b7ae829 100644 --- 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; } +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; diff --git a/src/dbus-unit.h b/src/dbus-unit.h index 1d11af3a0..d4af5a8b2 100644 --- a/src/dbus-unit.h +++ b/src/dbus-unit.h @@ -88,6 +88,7 @@ " \n" \ " \n" \ " \n" \ + " \n" \ " \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", "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); @@ -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_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); diff --git a/src/load-fragment.c b/src/load-fragment.c index 8e4ec74b0..a2974cbea 100644 --- a/src/load-fragment.c +++ b/src/load-fragment.c @@ -29,6 +29,7 @@ #include #include #include +#include #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" }, + { "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" }, @@ -1653,6 +1655,7 @@ static int load_from_path(Unit *u, const char *path) { FILE *f = NULL; char *filename = NULL, *id = NULL; Unit *merged; + struct stat st; if (!u) { /* Dirty dirty hack. */ @@ -1740,6 +1743,17 @@ static int load_from_path(Unit *u, const char *path) { 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; @@ -1748,6 +1762,8 @@ static int load_from_path(Unit *u, const char *path) { u->meta.fragment_path = filename; filename = NULL; + u->meta.fragment_mtime = timespec_load(&st.st_mtim); + u->meta.load_state = UNIT_LOADED; r = 0; diff --git a/src/systemctl.c b/src/systemctl.c index 1ded93695..380678694 100644 --- a/src/systemctl.c +++ b/src/systemctl.c @@ -742,6 +742,77 @@ finish: 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; @@ -860,6 +931,7 @@ static int start_unit_one( DBusMessage *m = NULL, *reply = NULL; DBusError error; + const char *path; int r; assert(bus); @@ -903,18 +975,21 @@ static int start_unit_one( 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) { - const char *path; 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; @@ -1287,6 +1362,8 @@ typedef struct UnitStatusInfo { const char *fragment_path; const char *default_control_group; + bool need_daemon_reload; + /* 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); } + + 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) { @@ -1495,6 +1576,8 @@ static int status_property(const char *name, DBusMessageIter *iter, UnitStatusIn if (streq(name, "Accept")) i->accept = b; + else if (streq(name, "NeedDaemonReload")) + i->need_daemon_reload = b; break; } diff --git a/src/unit.c b/src/unit.c index f8be8b26a..e46182ada 100644 --- a/src/unit.c +++ b/src/unit.c @@ -27,6 +27,7 @@ #include #include #include +#include #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\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), @@ -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, 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); @@ -2029,6 +2032,24 @@ void unit_status_printf(Unit *u, const char *format, ...) { 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", diff --git a/src/unit.h b/src/unit.h index 5d68583f1..d3a00797f 100644 --- 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 */ + usec_t fragment_mtime; /* 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, ...); +bool unit_need_daemon_reload(Unit *u); + const char *unit_type_to_string(UnitType i); UnitType unit_type_from_string(const char *s);