[systemd-devel] [PATCH] service: don't create extra cgroup for control process when reloading SysV service

[Lennart Poettering]

On Mon, 10.03.14 15:25, Lukas Nykryn (lnykryn at redhat.com) wrote:

> Unfortunately common practice in initscripts is to have reload as an
> alias for restart (https://fedoraproject.org/wiki/Packaging:SysVInitScript).
> In that case the newly started process will be killed immediately after
> the reload process ends and its cgroup is destroyed.

I am pretty convinced that this is one of those cases where we should
accept breakage. Daemons should only be started with "start", not
otherwise...

This patch is also quite incorrect, since it will confuse tracking of
the main PID from systemd.

I am not sure I grok why this all would be a problem at all, given that
on Fedora/RHEL we redirect those verbs to systemctl anyway, and
systemctl handles reload/restart on its own anyway... What am I missing?


> ---
>  src/core/service.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/src/core/service.c b/src/core/service.c
> index 121ddec..16d7ae0 100644
> --- a/src/core/service.c
> +++ b/src/core/service.c
> @@ -2272,7 +2272,15 @@ static void service_enter_reload(Service *s) {
>                                    !s->root_directory_start_only,
>                                    false,
>                                    false,
> +#ifdef HAVE_SYSV_COMPAT
> +                                  /* Don't create extra cgroup for SysV services.
> +                                   * Unfortunately common practice was to have reload as an alias
> +                                   * for restart and we are killing the new main process, when destroying
> +                                   * cgroup for the control process*/
> +                                  !s->is_sysv,
> +#else
>                                    true,
> +#endif
>                                    &s->control_pid);
>                  if (r < 0)
>                          goto fail;
> @@ -3082,7 +3090,10 @@ static void service_sigchld_event(Unit *u, pid_t pid, int code, int status) {
>                  /* Immediately get rid of the cgroup, so that the
>                   * kernel doesn't delay the cgroup empty messages for
>                   * the service cgroup any longer than necessary */
> -                service_kill_control_processes(s);
> +#ifdef HAVE_SYSV_COMPAT
> +                if (!s->is_sysv)
> +#endif
> +                        service_kill_control_processes(s);
>  
>                  if (s->control_command &&
>                      s->control_command->command_next &&


Lennart

-- 
Lennart Poettering, Red Hat