Nice, I was in the process of implementing this. Looks good to me. But I think it would be better to use "vi" instead of "vim" if no &editor is set. Vim is not installed on every system as default but vi is most likely. <div class="gmail_quote">Den 11/10/2014 18.37 skrev "Ronny Chevalier" <<a href="mailto:chevalier.ronny@gmail.com">chevalier.ronny@gmail.com</a>>: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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 the $SYSTEMD_EDITOR or $EDITOR or vim to the related files and daemon-reload is invoked when the editor exited successfully. See <a href="https://bugzilla.redhat.com/show_bug.cgi?id=906824" target="_blank">https://bugzilla.redhat.com/show_bug.cgi?id=906824</a> --- 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 | 44 +++++- man/systemd-analyze.xml | 6 +- man/timedatectl.xml | 6 +- src/systemctl/systemctl.c | 346 +++++++++++++++++++++++++++++++++++++++++++++- 10 files changed, 435 insertions(+), 33 deletions(-) diff --git a/TODO b/TODO index 0aaa9f2..857bdd0 100644 --- a/TODO +++ b/TODO @@ -65,8 +65,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" "<a href="http://www.oasis-open.org/docbook/xml/4.5/docbookx.dtd" target="_blank">http://www.oasis-open.org/docbook/xml/4.5/docbookx.dtd</a>"> -<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..34f689b 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,22 @@ 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>. If neither <varname>$SYSTEMD_EDITOR</varname> + nor <varname>$EDITOR</varname> are present or if it is set to an empty string, + <citerefentry><refentrytitle>vim</refentrytitle><manvolnum>1</manvolnum></citerefentry> + is used as default.</para></listitem> + </varlistentry> + </variablelist> + <xi:include href="less-variables.xml" /> + </refsect1> <refsect1> <title>See Also</title> @@ -1572,7 +1612,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@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 8d6d162..1c2e5a9 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; @@ -5535,6 +5537,346 @@ 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 run_editor(char **paths) { + _cleanup_strv_free_ char **args = NULL; + char *editor; + pid_t pid; + siginfo_t status; + int r; + + assert(paths); + + args = strv_copy(paths); + if (!args) + return log_oom(); + + /* SYSTEMD_EDITOR takes precedence over EDITOR + * If neither SYSTEMD_EDITOR nor EDITOR are present, we use vim as default + */ + editor = getenv("SYSTEMD_EDITOR"); + if (!editor) + editor = getenv("EDITOR"); + + if (!editor || isempty(editor)) + editor = strdup("vim"); + else + editor = strdup(editor); + if (!editor) + return log_oom(); + + r = strv_push_prepend(&args, editor); + if (r < 0) + return log_oom(); + + pid = fork(); + if (pid < 0) { + log_error("Failed to fork: %m"); + return -errno; + } + + if (pid == 0) { + execvp(editor, args); + log_error("Failed to execute %s: %m", editor); + _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; + _cleanup_free_ char *unit = 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; + 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(); @@ -5632,7 +5974,8 @@ 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" @@ -6643,6 +6986,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 _______________________________________________ systemd-devel mailing list <a href="mailto:systemd-devel@lists.freedesktop.org">systemd-devel@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/systemd-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/systemd-devel</a> </blockquote></div>