[systemd-devel] [PATCH] systemctl: add edit verb
Daniel Buch
boogiewasthere at gmail.com
Sat Oct 11 11:17:12 PDT 2014
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.ronny at 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 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 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/systemd-devel/attachments/20141011/cf2741bf/attachment-0001.html>
More information about the systemd-devel
mailing list