[systemd-devel] [PATCH v3] systemctl: add edit verb

Ronny Chevalier chevalier.ronny at gmail.com
Sat Oct 18 09:30:02 PDT 2014


It helps editing units by either creating a drop-in file, like
/etc/systemd/system/my.service.d/amendments.conf, or by copying the
original unit from /usr/lib/systemd/ to /etc/systemd/ if the --full
option is specified. Then it invokes an editor to the related files
and daemon-reload is invoked when the editor exited successfully.

See https://bugzilla.redhat.com/show_bug.cgi?id=906824
---
 TODO                      |   2 -
 man/journalctl.xml        |   6 +-
 man/less-variables.xml    |  40 +++--
 man/localectl.xml         |   6 +-
 man/loginctl.xml          |   6 +-
 man/machinectl.xml        |   6 +-
 man/systemctl.xml         |  49 +++++-
 man/systemd-analyze.xml   |   6 +-
 man/timedatectl.xml       |   6 +-
 src/systemctl/systemctl.c | 394 +++++++++++++++++++++++++++++++++++++++++++++-
 10 files changed, 488 insertions(+), 33 deletions(-)

diff --git a/TODO b/TODO
index 3206420..cc8d8c4 100644
--- a/TODO
+++ b/TODO
@@ -66,8 +66,6 @@ Features:
 
 * systemctl: if it fails, show log output?
 
-* maybe add "systemctl edit" that copies unit files from /usr/lib/systemd/system to /etc/systemd/system and invokes vim on them
-
 * dbus: add new message hdr field for allowing interactive auth, write spec for it. update dbus spec to mandate that unknown flags *must* be ignored...
 
 * maybe introduce AssertXYZ= similar to ConditionXYZ= that causes a unit to fail (instead of skipping it) if some condition is not true...
diff --git a/man/journalctl.xml b/man/journalctl.xml
index 7fb6afc..d36889f 100644
--- a/man/journalctl.xml
+++ b/man/journalctl.xml
@@ -891,7 +891,11 @@
                 failure code is returned.</para>
         </refsect1>
 
-        <xi:include href="less-variables.xml" />
+        <refsect1>
+                <title>Environment</title>
+
+                <xi:include href="less-variables.xml" />
+        </refsect1>
 
         <refsect1>
                 <title>Examples</title>
diff --git a/man/less-variables.xml b/man/less-variables.xml
index 09cbd42..1b8aae0 100644
--- a/man/less-variables.xml
+++ b/man/less-variables.xml
@@ -2,28 +2,24 @@
 <!DOCTYPE book PUBLIC "-//OASIS//DTD DocBook XML V4.5//EN"
                  "http://www.oasis-open.org/docbook/xml/4.5/docbookx.dtd">
 
-<refsect1>
-        <title>Environment</title>
+<variablelist class='environment-variables'>
+       <varlistentry>
+               <term><varname>$SYSTEMD_PAGER</varname></term>
 
-        <variablelist class='environment-variables'>
-                <varlistentry>
-                        <term><varname>$SYSTEMD_PAGER</varname></term>
+               <listitem><para>Pager to use when
+               <option>--no-pager</option> is not given;
+               overrides <varname>$PAGER</varname>.  Setting
+               this to an empty string or the value
+               <literal>cat</literal> is equivalent to passing
+               <option>--no-pager</option>.</para></listitem>
+       </varlistentry>
 
-                        <listitem><para>Pager to use when
-                        <option>--no-pager</option> is not given;
-                        overrides <varname>$PAGER</varname>.  Setting
-                        this to an empty string or the value
-                        <literal>cat</literal> is equivalent to passing
-                        <option>--no-pager</option>.</para></listitem>
-                </varlistentry>
+       <varlistentry>
+               <term><varname>$SYSTEMD_LESS</varname></term>
 
-                <varlistentry>
-                        <term><varname>$SYSTEMD_LESS</varname></term>
-
-                        <listitem><para>Override the default
-                        options passed to
-                        <command>less</command>
-                        (<literal>FRSXMK</literal>).</para></listitem>
-                </varlistentry>
-        </variablelist>
-</refsect1>
+               <listitem><para>Override the default
+               options passed to
+               <command>less</command>
+               (<literal>FRSXMK</literal>).</para></listitem>
+       </varlistentry>
+</variablelist>
diff --git a/man/localectl.xml b/man/localectl.xml
index 38e73c7..7ae6c60 100644
--- a/man/localectl.xml
+++ b/man/localectl.xml
@@ -223,7 +223,11 @@
                 code otherwise.</para>
         </refsect1>
 
-        <xi:include href="less-variables.xml" />
+        <refsect1>
+                <title>Environment</title>
+
+                <xi:include href="less-variables.xml" />
+        </refsect1>
 
         <refsect1>
                 <title>See Also</title>
diff --git a/man/loginctl.xml b/man/loginctl.xml
index 749db92..4754790 100644
--- a/man/loginctl.xml
+++ b/man/loginctl.xml
@@ -438,7 +438,11 @@
                 code otherwise.</para>
         </refsect1>
 
-        <xi:include href="less-variables.xml" />
+        <refsect1>
+                <title>Environment</title>
+
+                <xi:include href="less-variables.xml" />
+        </refsect1>
 
         <refsect1>
                 <title>See Also</title>
diff --git a/man/machinectl.xml b/man/machinectl.xml
index 2f2e257..b95b7fe 100644
--- a/man/machinectl.xml
+++ b/man/machinectl.xml
@@ -281,7 +281,11 @@
                 code otherwise.</para>
         </refsect1>
 
-        <xi:include href="less-variables.xml" />
+        <refsect1>
+                <title>Environment</title>
+
+                <xi:include href="less-variables.xml" />
+        </refsect1>
 
         <refsect1>
                 <title>See Also</title>
diff --git a/man/systemctl.xml b/man/systemctl.xml
index 61a23de..44dc7cb 100644
--- a/man/systemctl.xml
+++ b/man/systemctl.xml
@@ -1150,6 +1150,31 @@ kobject-uevent 1 systemd-udevd-kernel.socket systemd-udevd.service
             <filename>default.target</filename> to the given unit.</para>
           </listitem>
         </varlistentry>
+
+        <varlistentry>
+          <term><command>edit <replaceable>NAME</replaceable>...</command></term>
+
+          <listitem>
+            <para>Edit one or more unit files, as specified on the command
+            line.</para>
+
+            <para>Depending on whether <option>--system</option> (the default),
+            <option>--user</option>, or <option>--global</option> is specified,
+            this create a drop-in file for each units either for the system,
+            for the calling user or for all futures logins of all users. Then
+            the editor is invoked on them (see section "Environment" below).</para>
+
+            <para>If <option>--full</option> is specified, this will copy the original
+            units instead of creating drop-in files.</para>
+
+            <para>After the units have been edited, the systemd configuration is
+            reloaded (in a way that is equivalent to <command>daemon-reload</command>),
+            but it does not restart or reload the units.</para>
+
+            <para>Note that this command cannot be used with <option>--runtime</option> or
+            to remotely edit units.</para>
+          </listitem>
+        </varlistentry>
       </variablelist>
     </refsect2>
 
@@ -1559,7 +1584,27 @@ kobject-uevent 1 systemd-udevd-kernel.socket systemd-udevd.service
     code otherwise.</para>
   </refsect1>
 
-  <xi:include href="less-variables.xml" />
+  <refsect1>
+    <title>Environment</title>
+
+    <variablelist class='environment-variables'>
+      <varlistentry>
+        <term><varname>$SYSTEMD_EDITOR</varname></term>
+
+        <listitem><para>Editor to use when editing units; overrides
+        <varname>$EDITOR</varname> and <varname>$VISUAL</varname>. If neither
+        <varname>$SYSTEMD_EDITOR</varname> nor <varname>$EDITOR</varname> nor
+        <varname>$VISUAL</varname> are present or if it is set to an empty
+        string or if their execution failed, systemctl will try to execute well
+        known editors in this order:
+        <citerefentry><refentrytitle>nano</refentrytitle><manvolnum>1</manvolnum></citerefentry>,
+        <citerefentry><refentrytitle>vim</refentrytitle><manvolnum>1</manvolnum></citerefentry>,
+        <citerefentry><refentrytitle>vi</refentrytitle><manvolnum>1</manvolnum></citerefentry>.
+        </para></listitem>
+      </varlistentry>
+    </variablelist>
+    <xi:include href="less-variables.xml" />
+  </refsect1>
 
   <refsect1>
     <title>See Also</title>
@@ -1572,7 +1617,7 @@ kobject-uevent 1 systemd-udevd-kernel.socket systemd-udevd.service
       <citerefentry><refentrytitle>systemd.resource-management</refentrytitle><manvolnum>5</manvolnum></citerefentry>,
       <citerefentry><refentrytitle>systemd.special</refentrytitle><manvolnum>7</manvolnum></citerefentry>,
       <citerefentry project='man-pages'><refentrytitle>wall</refentrytitle><manvolnum>1</manvolnum></citerefentry>,
-      <citerefentry><refentrytitle>systemd.preset</refentrytitle><manvolnum>5</manvolnum></citerefentry>
+      <citerefentry><refentrytitle>systemd.preset</refentrytitle><manvolnum>5</manvolnum></citerefentry>,
       <citerefentry><refentrytitle>glob</refentrytitle><manvolnum>7</manvolnum></citerefentry>
     </para>
   </refsect1>
diff --git a/man/systemd-analyze.xml b/man/systemd-analyze.xml
index 073e807..0dd21a5 100644
--- a/man/systemd-analyze.xml
+++ b/man/systemd-analyze.xml
@@ -383,7 +383,11 @@ Service b at 0.service not loaded, b.socket cannot be started.
                 </example>
         </refsect1>
 
-        <xi:include href="less-variables.xml" />
+        <refsect1>
+                <title>Environment</title>
+
+                <xi:include href="less-variables.xml" />
+        </refsect1>
 
         <refsect1>
                 <title>See Also</title>
diff --git a/man/timedatectl.xml b/man/timedatectl.xml
index f3edb8d..849cc06 100644
--- a/man/timedatectl.xml
+++ b/man/timedatectl.xml
@@ -197,7 +197,11 @@
                 code otherwise.</para>
         </refsect1>
 
-        <xi:include href="less-variables.xml" />
+        <refsect1>
+                <title>Environment</title>
+
+                <xi:include href="less-variables.xml" />
+        </refsect1>
 
         <refsect1>
                 <title>Examples</title>
diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 28eaa6a..619f7e0 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -72,6 +72,8 @@
 #include "bus-message.h"
 #include "bus-error.h"
 #include "bus-errors.h"
+#include "copy.h"
+#include "mkdir.h"
 
 static char **arg_types = NULL;
 static char **arg_states = NULL;
@@ -5642,6 +5644,393 @@ static int is_system_running(sd_bus *bus, char **args) {
         return streq(state, "running") ? EXIT_SUCCESS : EXIT_FAILURE;
 }
 
+static int unit_file_find_path(LookupPaths *lp, const char *unit_name, char **unit_path) {
+        char **p;
+
+        assert(lp);
+        assert(unit_name);
+        assert(unit_path);
+
+        STRV_FOREACH(p, lp->unit_path) {
+                char *path;
+
+                path = strjoin(*p, "/", unit_name, NULL);
+                if (!path)
+                        return log_oom();
+
+                if (access(path, F_OK) == 0) {
+                        *unit_path = path;
+                        return 1;
+                }
+
+                free(path);
+        }
+
+        return 0;
+}
+
+static int unit_file_drop_in(const char *unit_name, const char *config_home, char **new_path) {
+        char *tmp_path;
+        int r;
+
+        assert(unit_name);
+        assert(new_path);
+
+        switch (arg_scope) {
+                case UNIT_FILE_SYSTEM:
+                        tmp_path = strjoin(SYSTEM_CONFIG_UNIT_PATH, "/", unit_name, ".d/amendments.conf", NULL);
+                        break;
+                case UNIT_FILE_GLOBAL:
+                        tmp_path = strjoin(USER_CONFIG_UNIT_PATH, "/", unit_name, ".d/amendments.conf", NULL);
+                        break;
+                case UNIT_FILE_USER:
+                        assert(config_home);
+                        tmp_path = strjoin(config_home, "/", unit_name, ".d/amendments.conf", NULL);
+                        break;
+                default:
+                        assert_not_reached("Invalid scope");
+        }
+        if (!tmp_path)
+                return log_oom();
+
+        r = mkdir_parents(tmp_path, 0755);
+        if (r < 0) {
+                log_error("Failed to create directories for %s: %s", tmp_path, strerror(-r));
+                free(tmp_path);
+                return r;
+        }
+
+        *new_path = tmp_path;
+
+        return 0;
+}
+
+static int unit_file_copy_if_needed(const char *unit_name, const char *fragment_path, char **new_path) {
+        char *tmp_path;
+        int r;
+
+        assert(fragment_path);
+        assert(unit_name);
+        assert(new_path);
+
+        /* If it's a unit for the --user scope there is no need to copy it, it's already in the right directory.
+         * Same if this is --system/--global scope and the file is in {SYSTEM,USER}_CONFIG_UNIT_PATH
+         */
+        if (arg_scope == UNIT_FILE_USER
+            || startswith(fragment_path, SYSTEM_CONFIG_UNIT_PATH)
+            || startswith(fragment_path, USER_CONFIG_UNIT_PATH)) {
+                *new_path = strdup(fragment_path);
+                if (!*new_path)
+                        return log_oom();
+                return 0;
+        }
+
+        switch (arg_scope) {
+                case UNIT_FILE_SYSTEM:
+                        tmp_path = strjoin(SYSTEM_CONFIG_UNIT_PATH, "/", unit_name, NULL);
+                        break;
+                case UNIT_FILE_GLOBAL:
+                        tmp_path = strjoin(USER_CONFIG_UNIT_PATH, "/", unit_name, NULL);
+                        break;
+                default:
+                        assert_not_reached("Invalid scope");
+        }
+        if (!tmp_path)
+                return log_oom();
+
+        if (access(tmp_path, F_OK) == 0) {
+                char response;
+
+                r = ask_char(&response, "yn", "%s already exists, are you sure to overwrite it with %s? [(y)es, (n)o] ", tmp_path, fragment_path);
+                if (r < 0) {
+                        free(tmp_path);
+                        return r;
+                }
+                if (response != 'y') {
+                        log_warning("%s ignored", unit_name);
+                        free(tmp_path);
+                        return -1;
+                }
+        }
+
+        r = mkdir_parents(tmp_path, 0755);
+        if (r < 0) {
+                log_error("Failed to create directories for %s: %s", tmp_path, strerror(-r));
+                free(tmp_path);
+                return r;
+        }
+
+        r = copy_file(fragment_path, tmp_path, 0, 0644);
+        if (r < 0) {
+                log_error("Failed to copy %s to %s: %s", fragment_path, tmp_path, strerror(-r));
+                free(tmp_path);
+                return r;
+        }
+
+        *new_path = tmp_path;
+
+        return 0;
+}
+
+static int get_editors(char ***editors) {
+        char **tmp_editors = strv_new("nano", "vim", "vi", NULL);
+        char *editor;
+
+        /* SYSTEMD_EDITOR takes precedence over EDITOR which takes precedence over VISUAL
+         * If neither SYSTEMD_EDITOR nor EDITOR nor VISUAL are present,
+         * we try to execute well known editors
+         */
+        editor = getenv("SYSTEMD_EDITOR");
+        if (!editor)
+                editor = getenv("EDITOR");
+        if (!editor)
+                editor = getenv("VISUAL");
+
+        if (editor) {
+                int r;
+
+                editor = strdup(editor);
+                if (!editor)
+                        return log_oom();
+
+                r = strv_consume_prepend(&tmp_editors, editor);
+                if (r < 0)
+                        return log_oom();
+        }
+
+        *editors = tmp_editors;
+
+        return 0;
+}
+
+static int run_editor(char **paths) {
+        pid_t pid;
+        siginfo_t status;
+        int r;
+
+        assert(paths);
+
+        pid = fork();
+        if (pid < 0) {
+                log_error("Failed to fork: %m");
+                return -errno;
+        }
+
+        if (pid == 0) {
+                _cleanup_strv_free_ char **editors = NULL;
+                char *editor;
+                char **p;
+
+                r = get_editors(&editors);
+                if (r < 0) {
+                        _exit(EXIT_FAILURE);
+                }
+
+                STRV_FOREACH(p, editors) {
+                        _cleanup_strv_free_ char **args = NULL;
+
+                        editor = strdup(*p);
+                        if (!editor) {
+                                log_oom();
+                                _exit(EXIT_FAILURE);
+                        }
+
+                        args = strv_copy(paths);
+                        if (!args) {
+                                log_oom();
+                                _exit(EXIT_FAILURE);
+                        }
+
+                        r = strv_consume_prepend(&args, editor);
+                        if (r < 0) {
+                                log_oom();
+                                _exit(EXIT_FAILURE);
+                        }
+
+                        execvp(editor, args);
+                        /* We do not fail if the editor doesn't exist
+                         * because we want to try each one of them before
+                         * failing.
+                         */
+                        if (errno != ENOENT) {
+                                log_error("Failed to execute %s: %m", editor);
+                                _exit(EXIT_FAILURE);
+                        }
+                }
+
+                log_error("Cannot edit unit(s): No editor available. Please set either SYSTEMD_EDITOR or EDITOR or VISUAL environment variable");
+                _exit(EXIT_FAILURE);
+        }
+
+        r = wait_for_terminate(pid, &status);
+        if (r < 0) {
+                log_error("Failed to wait for child: %s", strerror(-r));
+                return r;
+        }
+
+        return WIFEXITED(status) ? WEXITSTATUS(status) : -1;
+}
+
+static int find_units_path(sd_bus *bus, char **names, char ***paths) {
+        _cleanup_free_ char *config_home = NULL;
+        char **name;
+        int r;
+
+        assert(names);
+        assert(paths);
+
+        if (arg_scope == UNIT_FILE_USER) {
+                r = user_config_home(&config_home);
+                if (r < 0)
+                        return log_oom();
+
+                if (r == 0) {
+                        log_error("Cannot edit units for the user instance: home directory unknown");
+                        return -1;
+                }
+        }
+
+        if (!bus || avoid_bus()) {
+                _cleanup_lookup_paths_free_ LookupPaths lp = {};
+
+                /* If there is no bus, we try to find the units by testing each available directory
+                 * according to the scope.
+                 */
+                r = lookup_paths_init(&lp,
+                                arg_scope == UNIT_FILE_SYSTEM ? SYSTEMD_SYSTEM : SYSTEMD_USER,
+                                arg_scope == UNIT_FILE_USER,
+                                arg_root,
+                                NULL, NULL, NULL);
+                if (r < 0) {
+                        log_error("Cannot get lookup paths: %s", strerror(-r));
+                        return r;
+                }
+
+                STRV_FOREACH(name, names) {
+                        _cleanup_free_ char *path = NULL;
+                        char *new_path;
+
+                        r = unit_file_find_path(&lp, *name, &path);
+                        if (r < 0)
+                                return r;
+                        if (r == 0) {
+                                log_warning("%s ignored: not found", *name);
+                                continue;
+                        }
+
+                        if (arg_full)
+                                r = unit_file_copy_if_needed(*name, path, &new_path);
+                        else
+                                r = unit_file_drop_in(*name, config_home, &new_path);
+
+                        if (r < 0)
+                                continue;
+
+                        r = strv_push(paths, new_path);
+                        if (r < 0)
+                                return log_oom();
+                }
+        } else {
+                STRV_FOREACH(name, names) {
+                        _cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
+                        _cleanup_free_ char *fragment_path = NULL;
+                        _cleanup_free_ char *unit = NULL;
+                        char *new_path;
+
+                        unit = unit_dbus_path_from_name(*name);
+                        if (!unit)
+                                return log_oom();
+
+                        if (need_daemon_reload(bus, *name) > 0) {
+                                log_warning("%s ignored: unit file changed on disk. Run 'systemctl%s daemon-reload'.",
+                                        *name, arg_scope == UNIT_FILE_SYSTEM ? "" : " --user");
+                                continue;
+                        }
+
+                        r = sd_bus_get_property_string(
+                                        bus,
+                                        "org.freedesktop.systemd1",
+                                        unit,
+                                        "org.freedesktop.systemd1.Unit",
+                                        "FragmentPath",
+                                        &error,
+                                        &fragment_path);
+                        if (r < 0) {
+                                log_warning("Failed to get FragmentPath: %s", bus_error_message(&error, r));
+                                continue;
+                        }
+
+                        if (isempty(fragment_path)) {
+                                log_warning("%s ignored: not found", *name);
+                                continue;
+                        }
+
+                        if (arg_full)
+                                r = unit_file_copy_if_needed(*name, fragment_path, &new_path);
+                        else
+                                r = unit_file_drop_in(*name, config_home, &new_path);
+                        if (r < 0)
+                                continue;
+
+                        r = strv_push(paths, new_path);
+                        if (r < 0)
+                                return log_oom();
+                }
+        }
+
+        return 0;
+}
+
+static int edit(sd_bus *bus, char **args) {
+        _cleanup_strv_free_ char **names = NULL;
+        _cleanup_strv_free_ char **paths = NULL;
+        int r;
+
+        assert(args);
+
+        if (!on_tty())
+                return 0;
+
+        if (arg_transport != BUS_TRANSPORT_LOCAL) {
+                log_error("Cannot remotely edit units");
+                return -EINVAL;
+        }
+
+        if (arg_runtime) {
+                log_error("Cannot edit runtime units");
+                return -EINVAL;
+        }
+
+        r = expand_names(bus, args + 1, NULL, &names);
+        if (r < 0) {
+                log_error("Failed to expand names: %s", strerror(-r));
+                return r;
+        }
+
+        if (!names) {
+                log_error("No unit name found by expanding names");
+                return -ENOENT;
+        }
+
+        r = find_units_path(bus, names, &paths);
+        if (r < 0)
+                return r;
+
+        if (strv_isempty(paths)) {
+                log_error("Cannot find any units to edit");
+                return -ENOENT;
+        }
+
+        r = run_editor(paths);
+        if (r < 0)
+                return r;
+
+        if (!arg_no_reload)
+                r = daemon_reload(bus, args);
+
+        return r;
+}
+
 static void systemctl_help(void) {
 
         pager_open_if_enabled();
@@ -5739,7 +6128,9 @@ static void systemctl_help(void) {
                "  add-requires TARGET NAME...     Add 'Requires' dependency for the target\n"
                "                                  on specified one or more units\n"
                "  get-default                     Get the name of the default target\n"
-               "  set-default NAME                Set the default target\n\n"
+               "  set-default NAME                Set the default target\n"
+               "  edit NAME...                    Edit one or more unit files\n"
+               "\n"
                "Machine Commands:\n"
                "  list-machines [PATTERN...]      List local containers and host\n\n"
                "Job Commands:\n"
@@ -6750,6 +7141,7 @@ static int systemctl_main(sd_bus *bus, int argc, char *argv[], int bus_error) {
                 { "is-system-running",     EQUAL, 1, is_system_running },
                 { "add-wants",             MORE,  3, add_dependency,        NOBUS },
                 { "add-requires",          MORE,  3, add_dependency,        NOBUS },
+                { "edit",                  MORE,  2, edit,             NOBUS },
                 {}
         }, *verb = verbs;
 
-- 
2.1.2



More information about the systemd-devel mailing list