[systemd-devel] [PATCH v2] systemctl: introduce --now for enable, disable and mask
Lennart Poettering
lennart at poettering.net
Fri May 15 03:04:14 PDT 2015
On Fri, 15.05.15 09:54, jsynacek at redhat.com (jsynacek at redhat.com) wrote:
> From: Jan Synacek <jsynacek at redhat.com>
Applied!
Thanks!
>
> ---
> changes in v2:
> * simplified creation of new arguments (replaced unnecessary dynamic allocation
> with a VLA)
> * renamed add_file_change to unit_file_add_changes
> * addressed wording issues in documentation
>
> ---
> Makefile.am | 1 +
> man/systemctl.xml | 33 ++++++++----
> src/libsystemd/sd-bus/bus-util.c | 6 ++-
> src/libsystemd/sd-bus/bus-util.h | 3 +-
> src/machine/machinectl.c | 2 +-
> src/shared/install.c | 112 +++++++++++++++++++--------------------
> src/shared/install.h | 1 +
> src/systemctl/systemctl.c | 28 ++++++++--
> 8 files changed, 114 insertions(+), 72 deletions(-)
>
> diff --git a/Makefile.am b/Makefile.am
> index e8a329f..861f3b2 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -5351,6 +5351,7 @@ machinectl_LDADD = \
> libsystemd-internal.la \
> libsystemd-logs.la \
> libsystemd-journal-internal.la \
> + libsystemd-units.la \
> libsystemd-shared.la
>
> rootbin_PROGRAMS += \
> diff --git a/man/systemctl.xml b/man/systemctl.xml
> index 4dbdfe1..dd9e5c5 100644
> --- a/man/systemctl.xml
> +++ b/man/systemctl.xml
> @@ -456,6 +456,18 @@
> </varlistentry>
>
> <varlistentry>
> + <term><option>--now</option></term>
> +
> + <listitem>
> + <para>When used with <command>enable</command>, the units
> + will also be started. When used with <command>disable</command> or
> + <command>mask</command>, the units will also be stopped. The start
> + or stop operation is only carried out when the respective enable or
> + disable operation has been successful.</para>
> + </listitem>
> + </varlistentry>
> +
> + <varlistentry>
> <term><option>--root=</option></term>
>
> <listitem>
> @@ -921,11 +933,12 @@ kobject-uevent 1 systemd-udevd-kernel.socket systemd-udevd.service
> the changes are taken into account immediately. Note that
> this does <emphasis>not</emphasis> have the effect of also
> starting any of the units being enabled. If this
> - is desired, a separate <command>start</command> command must
> - be invoked for the unit. Also note that in case of instance
> - enablement, symlinks named the same as instances are created in
> - the install location, however they all point to the same
> - template unit file.</para>
> + is desired, either <option>--now</option> should be used
> + together with this command, or an additional <command>start</command>
> + command must be invoked for the unit. Also note that in case of
> + instance enablement, symlinks named the same as instances
> + are created in the install location, however they all point to the
> + same template unit file.</para>
>
> <para>This command will print the actions executed. This
> output may be suppressed by passing <option>--quiet</option>.
> @@ -980,9 +993,10 @@ kobject-uevent 1 systemd-udevd-kernel.socket systemd-udevd.service
> <command>enable</command>. This call implicitly reloads the
> systemd daemon configuration after completing the disabling
> of the units. Note that this command does not implicitly
> - stop the units that are being disabled. If this is desired,
> - an additional <command>stop</command> command should be
> - executed afterwards.</para>
> + stop the units that are being disabled. If this is desired, either
> + <option>--now</option> should be used together with this command, or
> + an additional <command>stop</command> command should be executed
> + afterwards.</para>
>
> <para>This command will print the actions executed. This
> output may be suppressed by passing <option>--quiet</option>.
> @@ -1128,7 +1142,8 @@ kobject-uevent 1 systemd-udevd-kernel.socket systemd-udevd.service
> activation of the unit, including enablement and manual
> activation. Use this option with care. This honors the
> <option>--runtime</option> option to only mask temporarily
> - until the next reboot of the system.</para>
> + until the next reboot of the system. The <option>--now</option>
> + option can be used to ensure that the units are also stopped.</para>
> </listitem>
> </varlistentry>
>
> diff --git a/src/libsystemd/sd-bus/bus-util.c b/src/libsystemd/sd-bus/bus-util.c
> index 7536a96..5e375af 100644
> --- a/src/libsystemd/sd-bus/bus-util.c
> +++ b/src/libsystemd/sd-bus/bus-util.c
> @@ -1899,7 +1899,7 @@ int bus_wait_for_jobs_one(BusWaitForJobs *d, const char *path, bool quiet) {
> return bus_wait_for_jobs(d, quiet);
> }
>
> -int bus_deserialize_and_dump_unit_file_changes(sd_bus_message *m, bool quiet) {
> +int bus_deserialize_and_dump_unit_file_changes(sd_bus_message *m, bool quiet, UnitFileChange **changes, unsigned *n_changes) {
> const char *type, *path, *source;
> int r;
>
> @@ -1914,6 +1914,10 @@ int bus_deserialize_and_dump_unit_file_changes(sd_bus_message *m, bool quiet) {
> else
> log_info("Removed symlink %s.", path);
> }
> +
> + r = unit_file_changes_add(changes, n_changes, streq(type, "symlink") ? UNIT_FILE_SYMLINK : UNIT_FILE_UNLINK, path, source);
> + if (r < 0)
> + return r;
> }
> if (r < 0)
> return bus_log_parse_error(r);
> diff --git a/src/libsystemd/sd-bus/bus-util.h b/src/libsystemd/sd-bus/bus-util.h
> index 093b48b..999a372 100644
> --- a/src/libsystemd/sd-bus/bus-util.h
> +++ b/src/libsystemd/sd-bus/bus-util.h
> @@ -24,6 +24,7 @@
> #include "sd-event.h"
> #include "sd-bus.h"
> #include "hashmap.h"
> +#include "install.h"
> #include "time-util.h"
>
> typedef enum BusTransport {
> @@ -201,7 +202,7 @@ int bus_wait_for_jobs_one(BusWaitForJobs *d, const char *path, bool quiet);
>
> DEFINE_TRIVIAL_CLEANUP_FUNC(BusWaitForJobs*, bus_wait_for_jobs_free);
>
> -int bus_deserialize_and_dump_unit_file_changes(sd_bus_message *m, bool quiet);
> +int bus_deserialize_and_dump_unit_file_changes(sd_bus_message *m, bool quiet, UnitFileChange **changes, unsigned *n_changes);
>
> int bus_path_encode_unique(sd_bus *b, const char *prefix, const char *sender_id, const char *external_id, char **ret_path);
> int bus_path_decode_unique(const char *path, const char *prefix, char **ret_sender, char **ret_external);
> diff --git a/src/machine/machinectl.c b/src/machine/machinectl.c
> index 3764c0a..b21a339 100644
> --- a/src/machine/machinectl.c
> +++ b/src/machine/machinectl.c
> @@ -1479,7 +1479,7 @@ static int enable_machine(int argc, char *argv[], void *userdata) {
> return bus_log_parse_error(r);
> }
>
> - r = bus_deserialize_and_dump_unit_file_changes(reply, arg_quiet);
> + r = bus_deserialize_and_dump_unit_file_changes(reply, arg_quiet, NULL, NULL);
> if (r < 0)
> return r;
>
> diff --git a/src/shared/install.c b/src/shared/install.c
> index dc4cc62..6172c42 100644
> --- a/src/shared/install.c
> +++ b/src/shared/install.c
> @@ -112,51 +112,6 @@ static int get_config_path(UnitFileScope scope, bool runtime, const char *root_d
> return 0;
> }
>
> -static int add_file_change(
> - UnitFileChange **changes,
> - unsigned *n_changes,
> - UnitFileChangeType type,
> - const char *path,
> - const char *source) {
> -
> - UnitFileChange *c;
> - unsigned i;
> -
> - assert(path);
> - assert(!changes == !n_changes);
> -
> - if (!changes)
> - return 0;
> -
> - c = realloc(*changes, (*n_changes + 1) * sizeof(UnitFileChange));
> - if (!c)
> - return -ENOMEM;
> -
> - *changes = c;
> - i = *n_changes;
> -
> - c[i].type = type;
> - c[i].path = strdup(path);
> - if (!c[i].path)
> - return -ENOMEM;
> -
> - path_kill_slashes(c[i].path);
> -
> - if (source) {
> - c[i].source = strdup(source);
> - if (!c[i].source) {
> - free(c[i].path);
> - return -ENOMEM;
> - }
> -
> - path_kill_slashes(c[i].path);
> - } else
> - c[i].source = NULL;
> -
> - *n_changes = i+1;
> - return 0;
> -}
> -
> static int mark_symlink_for_removal(
> Set **remove_symlinks_to,
> const char *p) {
> @@ -309,7 +264,7 @@ static int remove_marked_symlinks_fd(
>
> path_kill_slashes(p);
> rmdir_parents(p, config_path);
> - add_file_change(changes, n_changes, UNIT_FILE_UNLINK, p, NULL);
> + unit_file_changes_add(changes, n_changes, UNIT_FILE_UNLINK, p, NULL);
>
> if (!set_get(remove_symlinks_to, p)) {
>
> @@ -596,7 +551,7 @@ int unit_file_mask(
> }
>
> if (symlink("/dev/null", path) >= 0) {
> - add_file_change(changes, n_changes, UNIT_FILE_SYMLINK, path, "/dev/null");
> + unit_file_changes_add(changes, n_changes, UNIT_FILE_SYMLINK, path, "/dev/null");
> continue;
> }
>
> @@ -607,8 +562,8 @@ int unit_file_mask(
>
> if (force) {
> if (symlink_atomic("/dev/null", path) >= 0) {
> - add_file_change(changes, n_changes, UNIT_FILE_UNLINK, path, NULL);
> - add_file_change(changes, n_changes, UNIT_FILE_SYMLINK, path, "/dev/null");
> + unit_file_changes_add(changes, n_changes, UNIT_FILE_UNLINK, path, NULL);
> + unit_file_changes_add(changes, n_changes, UNIT_FILE_SYMLINK, path, "/dev/null");
> continue;
> }
> }
> @@ -664,7 +619,7 @@ int unit_file_unmask(
> q = -errno;
> else {
> q = mark_symlink_for_removal(&remove_symlinks_to, path);
> - add_file_change(changes, n_changes, UNIT_FILE_UNLINK, path, NULL);
> + unit_file_changes_add(changes, n_changes, UNIT_FILE_UNLINK, path, NULL);
> }
> }
>
> @@ -746,7 +701,7 @@ int unit_file_link(
> return -ENOMEM;
>
> if (symlink(*i, path) >= 0) {
> - add_file_change(changes, n_changes, UNIT_FILE_SYMLINK, path, *i);
> + unit_file_changes_add(changes, n_changes, UNIT_FILE_SYMLINK, path, *i);
> continue;
> }
>
> @@ -765,8 +720,8 @@ int unit_file_link(
>
> if (force) {
> if (symlink_atomic(*i, path) >= 0) {
> - add_file_change(changes, n_changes, UNIT_FILE_UNLINK, path, NULL);
> - add_file_change(changes, n_changes, UNIT_FILE_SYMLINK, path, *i);
> + unit_file_changes_add(changes, n_changes, UNIT_FILE_UNLINK, path, NULL);
> + unit_file_changes_add(changes, n_changes, UNIT_FILE_SYMLINK, path, *i);
> continue;
> }
> }
> @@ -793,6 +748,51 @@ void unit_file_list_free(Hashmap *h) {
> hashmap_free(h);
> }
>
> +int unit_file_changes_add(
> + UnitFileChange **changes,
> + unsigned *n_changes,
> + UnitFileChangeType type,
> + const char *path,
> + const char *source) {
> +
> + UnitFileChange *c;
> + unsigned i;
> +
> + assert(path);
> + assert(!changes == !n_changes);
> +
> + if (!changes)
> + return 0;
> +
> + c = realloc(*changes, (*n_changes + 1) * sizeof(UnitFileChange));
> + if (!c)
> + return -ENOMEM;
> +
> + *changes = c;
> + i = *n_changes;
> +
> + c[i].type = type;
> + c[i].path = strdup(path);
> + if (!c[i].path)
> + return -ENOMEM;
> +
> + path_kill_slashes(c[i].path);
> +
> + if (source) {
> + c[i].source = strdup(source);
> + if (!c[i].source) {
> + free(c[i].path);
> + return -ENOMEM;
> + }
> +
> + path_kill_slashes(c[i].path);
> + } else
> + c[i].source = NULL;
> +
> + *n_changes = i+1;
> + return 0;
> +}
> +
> void unit_file_changes_free(UnitFileChange *changes, unsigned n_changes) {
> unsigned i;
>
> @@ -1198,7 +1198,7 @@ static int create_symlink(
> mkdir_parents_label(new_path, 0755);
>
> if (symlink(old_path, new_path) >= 0) {
> - add_file_change(changes, n_changes, UNIT_FILE_SYMLINK, new_path, old_path);
> + unit_file_changes_add(changes, n_changes, UNIT_FILE_SYMLINK, new_path, old_path);
> return 0;
> }
>
> @@ -1219,8 +1219,8 @@ static int create_symlink(
> if (r < 0)
> return r;
>
> - add_file_change(changes, n_changes, UNIT_FILE_UNLINK, new_path, NULL);
> - add_file_change(changes, n_changes, UNIT_FILE_SYMLINK, new_path, old_path);
> + unit_file_changes_add(changes, n_changes, UNIT_FILE_UNLINK, new_path, NULL);
> + unit_file_changes_add(changes, n_changes, UNIT_FILE_SYMLINK, new_path, old_path);
>
> return 0;
> }
> diff --git a/src/shared/install.h b/src/shared/install.h
> index 45eca42..a9d77dd 100644
> --- a/src/shared/install.h
> +++ b/src/shared/install.h
> @@ -120,6 +120,7 @@ UnitFileState unit_file_get_state(
> int unit_file_get_list(UnitFileScope scope, const char *root_dir, Hashmap *h);
>
> void unit_file_list_free(Hashmap *h);
> +int unit_file_changes_add(UnitFileChange **changes, unsigned *n_changes, UnitFileChangeType type, const char *path, const char *source);
> void unit_file_changes_free(UnitFileChange *changes, unsigned n_changes);
>
> int unit_file_query_preset(UnitFileScope scope, const char *root_dir, const char *name);
> diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
> index 1f18f9c..a5b3660 100644
> --- a/src/systemctl/systemctl.c
> +++ b/src/systemctl/systemctl.c
> @@ -136,6 +136,7 @@ static unsigned arg_lines = 10;
> static OutputMode arg_output = OUTPUT_SHORT;
> static bool arg_plain = false;
> static bool arg_firmware_setup = false;
> +static bool arg_now = false;
>
> static bool original_stdout_is_tty;
>
> @@ -1973,7 +1974,7 @@ static int set_default(sd_bus *bus, char **args) {
> return r;
> }
>
> - r = bus_deserialize_and_dump_unit_file_changes(reply, arg_quiet);
> + r = bus_deserialize_and_dump_unit_file_changes(reply, arg_quiet, NULL, NULL);
> if (r < 0)
> return r;
>
> @@ -5388,7 +5389,7 @@ static int enable_unit(sd_bus *bus, char **args) {
> return bus_log_parse_error(r);
> }
>
> - r = bus_deserialize_and_dump_unit_file_changes(reply, arg_quiet);
> + r = bus_deserialize_and_dump_unit_file_changes(reply, arg_quiet, &changes, &n_changes);
> if (r < 0)
> return r;
>
> @@ -5410,6 +5411,18 @@ static int enable_unit(sd_bus *bus, char **args) {
> "3) A unit may be started when needed via activation (socket, path, timer,\n"
> " D-Bus, udev, scripted systemctl call, ...).\n");
>
> + if (arg_now && n_changes > 0 && STR_IN_SET(args[0], "enable", "disable", "mask")) {
> + char *new_args[n_changes + 2];
> + unsigned i;
> +
> + new_args[0] = streq(args[0], "enable") ? (char *)"start" : (char *)"stop";
> + for (i = 0; i < n_changes; i++)
> + new_args[i + 1] = basename(changes[i].path);
> + new_args[i + 1] = NULL;
> +
> + r = start_unit(bus, new_args);
> + }
> +
> finish:
> unit_file_changes_free(changes, n_changes);
>
> @@ -5485,7 +5498,7 @@ static int add_dependency(sd_bus *bus, char **args) {
> return r;
> }
>
> - r = bus_deserialize_and_dump_unit_file_changes(reply, arg_quiet);
> + r = bus_deserialize_and_dump_unit_file_changes(reply, arg_quiet, NULL, NULL);
> if (r < 0)
> return r;
>
> @@ -5539,7 +5552,7 @@ static int preset_all(sd_bus *bus, char **args) {
> return r;
> }
>
> - r = bus_deserialize_and_dump_unit_file_changes(reply, arg_quiet);
> + r = bus_deserialize_and_dump_unit_file_changes(reply, arg_quiet, NULL, NULL);
> if (r < 0)
> return r;
>
> @@ -6015,6 +6028,7 @@ static void systemctl_help(void) {
> " When shutting down or sleeping, ignore inhibitors\n"
> " --kill-who=WHO Who to send signal to\n"
> " -s --signal=SIGNAL Which signal to send\n"
> + " --now Start or stop unit in addition to enabling or disabling it\n"
> " -q --quiet Suppress output\n"
> " --no-block Do not wait until operation finished\n"
> " --no-wall Don't send wall message before halt/power-off/reboot\n"
> @@ -6214,6 +6228,7 @@ static int systemctl_parse_argv(int argc, char *argv[]) {
> ARG_JOB_MODE,
> ARG_PRESET_MODE,
> ARG_FIRMWARE_SETUP,
> + ARG_NOW,
> };
>
> static const struct option options[] = {
> @@ -6257,6 +6272,7 @@ static int systemctl_parse_argv(int argc, char *argv[]) {
> { "recursive", no_argument, NULL, 'r' },
> { "preset-mode", required_argument, NULL, ARG_PRESET_MODE },
> { "firmware-setup", no_argument, NULL, ARG_FIRMWARE_SETUP },
> + { "now", no_argument, NULL, ARG_NOW },
> {}
> };
>
> @@ -6537,6 +6553,10 @@ static int systemctl_parse_argv(int argc, char *argv[]) {
>
> break;
>
> + case ARG_NOW:
> + arg_now = true;
> + break;
> +
> case '?':
> return -EINVAL;
>
> --
> 2.1.0
>
> _______________________________________________
> systemd-devel mailing list
> systemd-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Lennart
--
Lennart Poettering, Red Hat
More information about the systemd-devel
mailing list