On Sat, Feb 02, 2013 at 08:40:22PM +0200, Oleksii Shevchuk wrote:
>
> Current one:
> - If StopWhenUnneeded=yes and RequiredBy=, WantedBy=, BoundBy= empty
> Then stop unit.
> The side effect is:
> - If user starts unit, that not wanted by any started target, then unit
> will be immediately stopped.
> The problems are:
> - The user can not be sure that the launch of a unit means that it will
> be actually started
> - We already have RefuseManualStart=
>
> Patched one:
> - When unit starts, If RequiredBy=, WantedBy=, BoundBy= empty
> Then set StopWhenUnneeded=yes
> The side effect is:
> - User can be sure - the launch of the unit means that it will be
> started
> - If unit started when WantedBy=/... unit running, the unit will be stopped
> with the parent one
> - If unit started when WantedBy=/... are empty, then unit will not be
> stopped even after such units started, unitil it will be actually
> stopped.
Hi Oleksii,
thank you for providing this explanation, it provides the rationale
for the patch. Still, I don't know if this change is warranted.
Even without this patch, the unit would be started, and *then* stopped
immediately, so it's not true that the user doesn not know if will be
started. I mean that if someone doesn't want to have the semantics
provided by StopWhenUnneeded=true, maybe they should not set it.
Let's see what others think.
Zbyszek
> Path introduces next changes:
> - unit:
> + Replace stop_when_unneeded field in Unit structure to
> * stop_when_unneeded_unit -- for default value from unit file
> * stop_when_unneeded_runtime -- for runtime value setted up in
> unit_start
> + Decision logic moved to unit_unneeded function
> + Add logic to unit_start
> * When unit starts:
> set stop_when_unneeded_runtime=stop_when_unneeded_unit
> If unit_unneeded Then set stop_when_unneeded_runtime=no
> + Add unit_stop_when_unneeded_state function, to return actual
> StopWhenUnneeded= state with next logic:
> * For starting and started units return runtime state
> * For other states return default value from StopWhenUnneeded= unit
> field
> + For unit_dump use default value from unit file
> + For serialization (daemon-reexec) use runtime value from unit state
> * "stop-when-unneeded-runtime" serialization field added
> - dbus interface:
> + Add bus_unit_append_stop_when_unneeded function, to return actual
> StopWhenUnneeded= state via unit_stop_when_unneeded_state and use
> it instead of direct u->stop_when_unneeded serialization
> ---
> src/core/dbus-unit.c | 18 +++++++++-
> src/core/load-fragment-gperf.gperf.m4 | 2 +-
> src/core/unit.c | 67
> +++++++++++++++++++++++++++--------
> src/core/unit.h | 5 ++-
> 4 files changed, 75 insertions(+), 17 deletions(-)
>
> diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c
> index d1de46a..91b6f4f 100644
> --- a/src/core/dbus-unit.c
> +++ b/src/core/dbus-unit.c
> @@ -202,6 +202,22 @@ static int bus_unit_append_can_stop(DBusMessageIter *i,
> const char *property, vo
> return 0;
> }
>
> +static int bus_unit_append_stop_when_unneeded(DBusMessageIter *i, const char
> *property, void *data) {
> + Unit *u = data;
> + dbus_bool_t b;
> +
> + assert(i);
> + assert(property);
> + assert(u);
> +
> + b = unit_stop_when_unneeded_state(u);
> +
> + if (!dbus_message_iter_append_basic(i, DBUS_TYPE_BOOLEAN, &b))
> + return -ENOMEM;
> +
> + return 0;
> +}
> +
> static int bus_unit_append_can_reload(DBusMessageIter *i, const char
> *property, void *data) {
> Unit *u = data;
> dbus_bool_t b;
> @@ -1236,7 +1252,7 @@ const BusProperty bus_unit_properties[] = {
> { "CanReload", bus_unit_append_can_reload, "b", 0
> },
> { "CanIsolate", bus_unit_append_can_isolate, "b", 0
> },
> { "Job", bus_unit_append_job, "(uo)", 0
> },
> - { "StopWhenUnneeded", bus_property_append_bool, "b",
> offsetof(Unit, stop_when_unneeded) },
> + { "StopWhenUnneeded", bus_unit_append_stop_when_unneeded, "b", 0
> },
> { "RefuseManualStart", bus_property_append_bool, "b",
> offsetof(Unit, refuse_manual_start) },
> { "RefuseManualStop", bus_property_append_bool, "b",
> offsetof(Unit, refuse_manual_stop) },
> { "AllowIsolate", bus_property_append_bool, "b",
> offsetof(Unit, allow_isolate) },
> diff --git a/src/core/load-fragment-gperf.gperf.m4
> b/src/core/load-fragment-gperf.gperf.m4
> index 1783ad0..6fad6c9 100644
> --- a/src/core/load-fragment-gperf.gperf.m4
> +++ b/src/core/load-fragment-gperf.gperf.m4
> @@ -114,7 +114,7 @@ Unit.ReloadPropagatedFrom, config_parse_unit_deps,
> UNIT_RELOAD
> Unit.PropagateReloadFrom, config_parse_unit_deps,
> UNIT_RELOAD_PROPAGATED_FROM, 0
> Unit.PartOf, config_parse_unit_deps,
> UNIT_PART_OF, 0
> Unit.RequiresMountsFor, config_parse_unit_requires_mounts_for, 0,
> offsetof(Unit, requires_mounts_for)
> -Unit.StopWhenUnneeded, config_parse_bool, 0,
> offsetof(Unit, stop_when_unneeded)
> +Unit.StopWhenUnneeded, config_parse_bool, 0,
> offsetof(Unit, stop_when_unneeded_unit)
> Unit.RefuseManualStart, config_parse_bool, 0,
> offsetof(Unit, refuse_manual_start)
> Unit.RefuseManualStop, config_parse_bool, 0,
> offsetof(Unit, refuse_manual_stop)
> Unit.AllowIsolate, config_parse_bool, 0,
> offsetof(Unit, allow_isolate)
> diff --git a/src/core/unit.c b/src/core/unit.c
> index f7d00b6..db5dac9 100644
> --- a/src/core/unit.c
> +++ b/src/core/unit.c
> @@ -436,6 +436,19 @@ const char* unit_sub_state_to_string(Unit *u) {
> return UNIT_VTABLE(u)->sub_state_to_string(u);
> }
>
> +bool unit_stop_when_unneeded_state(Unit *u)
> +{
> + assert(u);
> +
> + if (u->load_state == UNIT_MERGED)
> + return unit_stop_when_unneeded_state(unit_follow_merge(u));
> +
> + if (UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(u)))
> + return u->stop_when_unneeded_runtime;
> + else
> + return u->stop_when_unneeded_unit;
> +}
> +
> static void complete_move(Set **s, Set **other) {
> assert(s);
> assert(other);
> @@ -732,7 +745,7 @@ void unit_dump(Unit *u, FILE *f, const char *prefix) {
> "%s\tOnFailureIsolate: %s\n"
> "%s\tIgnoreOnIsolate: %s\n"
> "%s\tIgnoreOnSnapshot: %s\n",
> - prefix, yes_no(u->stop_when_unneeded),
> + prefix, yes_no(u->stop_when_unneeded_unit),
> prefix, yes_no(u->refuse_manual_start),
> prefix, yes_no(u->refuse_manual_stop),
> prefix, yes_no(u->default_dependencies),
> @@ -1064,6 +1077,14 @@ int unit_start(Unit *u) {
> return -EALREADY;
> }
>
> + u->stop_when_unneeded_runtime = u->stop_when_unneeded_unit;
> +
> + if (unit_unneeded(u)) {
> + log_debug("Started unneded unit %s with
> StopWhenUnneeded=yes."
> + "Disabling unneeded property.", u->id);
> + u->stop_when_unneeded_runtime = false;
> + }
> +
> /* Forward to the main object, if we aren't it. */
> if ((following = unit_following(u))) {
> log_debug("Redirecting start request from %s to %s.", u->id,
> following->id);
> @@ -1179,36 +1200,44 @@ bool unit_can_reload(Unit *u) {
> return UNIT_VTABLE(u)->can_reload(u);
> }
>
> -static void unit_check_unneeded(Unit *u) {
> +bool unit_unneeded(Unit *u) {
> Iterator i;
> Unit *other;
>
> assert(u);
>
> - /* If this service shall be shut down when unneeded then do
> - * so. */
> -
> - if (!u->stop_when_unneeded)
> - return;
> -
> - if (!UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(u)))
> - return;
> + if (!unit_stop_when_unneeded_state(u))
> + return false;
>
> SET_FOREACH(other, u->dependencies[UNIT_REQUIRED_BY], i)
> if (unit_pending_active(other))
> - return;
> + return false;
>
> SET_FOREACH(other, u->dependencies[UNIT_REQUIRED_BY_OVERRIDABLE], i)
> if (unit_pending_active(other))
> - return;
> + return false;
>
> SET_FOREACH(other, u->dependencies[UNIT_WANTED_BY], i)
> if (unit_pending_active(other))
> - return;
> + return false;
>
> SET_FOREACH(other, u->dependencies[UNIT_BOUND_BY], i)
> if (unit_pending_active(other))
> - return;
> + return false;
> +
> + return true;
> +}
> +
> +static void unit_check_unneeded(Unit *u) {
> +
> + /* If this service shall be shut down when unneeded then do
> + * so. */
> +
> + if (!UNIT_IS_ACTIVE_OR_ACTIVATING(unit_active_state(u)))
> + return;
> +
> + if (! unit_unneeded(u))
> + return;
>
> log_info("Service %s is not needed anymore. Stopping.", u->id);
>
> @@ -2335,6 +2364,7 @@ int unit_serialize(Unit *u, FILE *f, FDSet *fds, bool
> serialize_jobs) {
> dual_timestamp_serialize(f, "active-exit-timestamp",
> &u->active_exit_timestamp);
> dual_timestamp_serialize(f, "inactive-enter-timestamp",
> &u->inactive_enter_timestamp);
> dual_timestamp_serialize(f, "condition-timestamp",
> &u->condition_timestamp);
> + unit_serialize_item(u, f, "stop-when-unneeded-runtime",
> yes_no(u->stop_when_unneeded_runtime));
>
> if (dual_timestamp_is_set(&u->condition_timestamp))
> unit_serialize_item(u, f, "condition-result",
> yes_no(u->condition_result));
> @@ -2464,6 +2494,15 @@ int unit_deserialize(Unit *u, FILE *f, FDSet *fds) {
> u->condition_result = b;
>
> continue;
> + } else if (streq(l, "stop-when-unneeded-runtime")) {
> + int b;
> +
> + if ((b = parse_boolean(v)) < 0)
> + log_debug("Failed to parse StopWhenUnneeded
> runtime state %s", v);
> + else
> + u->stop_when_unneeded_runtime = b;
> +
> + continue;
> }
>
> if ((r = UNIT_VTABLE(u)->deserialize_item(u, l, v, fds)) < 0)
> diff --git a/src/core/unit.h b/src/core/unit.h
> index c902103..9093721 100644
> --- a/src/core/unit.h
> +++ b/src/core/unit.h
> @@ -199,7 +199,8 @@ struct Unit {
> UnitFileState unit_file_state;
>
> /* Garbage collect us we nobody wants or requires us anymore */
> - bool stop_when_unneeded;
> + bool stop_when_unneeded_unit;
> + bool stop_when_unneeded_runtime;
>
> /* Create default dependencies */
> bool default_dependencies;
> @@ -479,10 +480,12 @@ void unit_dump(Unit *u, FILE *f, const char *prefix);
> bool unit_can_reload(Unit *u);
> bool unit_can_start(Unit *u);
> bool unit_can_isolate(Unit *u);
> +bool unit_stop_when_unneeded_state(Unit *u);
>
> int unit_start(Unit *u);
> int unit_stop(Unit *u);
> int unit_reload(Unit *u);
> +bool unit_unneeded(Unit *u);
>
> int unit_kill(Unit *u, KillWho w, int signo, DBusError *error);
>
> --
> 1.8.1
> _______________________________________________
> 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
