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. Den 11/10/2014 18.37 skrev "Ronny Chevalier" <chevalier.ro...@gmail.com>:
> 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 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 | 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" > "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..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 > systemd-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/systemd-devel >
_______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel