[systemd-devel] [PATCH] systemctl: improve readability on failed commands

[Zbigniew Jędrzejewski-Szmek]

On Fri, Jan 03, 2014 at 12:37:32AM +0100, Thomas H.P. Andersen wrote:
> From: Thomas Hindoe Paaboel Andersen <phomes at gmail.com>
> 
> Not long ago a failed command would print:
> "Failed to start something.service: ..."
> regardless of whether the command was to start/stop/restart/etc.
> 
> With e3e0314 this was improved to print the method used. E.g. for stopping:
> "Failed to StopUnit something.service: ..."
> 
> This patch matches the method to a more human readable word. E.g:
> "Failed to stop something.service: ..."
> ---
>  src/systemctl/systemctl.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
> index 67bc426..0b50813 100644
> --- a/src/systemctl/systemctl.c
> +++ b/src/systemctl/systemctl.c
> @@ -2067,12 +2067,24 @@ static int start_unit_one(
>                          &reply,
>                          "ss", name, mode);
>          if (r < 0) {
> +                const char *command;
> +
>                  if (r == -ENOENT && arg_action != ACTION_SYSTEMCTL)
>                          /* There's always a fallback possible for
>                           * legacy actions. */
>                          return -EADDRNOTAVAIL;
>  
> -                log_error("Failed to %s %s: %s", method, name, bus_error_message(error, r));
> +                command =
> +                        streq(method, "StartUnit")              ? "start" :
> +                        streq(method, "StopUnit")               ? "stop" :
> +                        streq(method, "ReloadUnit")             ? "reload" :
> +                        streq(method, "RestartUnit")            ? "restart" :
> +                        streq(method, "TryRestartUnit")         ? "try-restart" :
> +                        streq(method, "ReloadOrRestartUnit")    ? "reload-or-restart" :
> +                        streq(method, "ReloadOrTryRestartUnit") ? "reload-or-try-restart" :
> +                        method;
Hm, we have the opposite mapping done by hand in parse_argv too...
Maybe we could coalesce those two into a single table? I think
reduce the chances of them getting out of sync.

Zbyszek