[systemd-devel] [PATCH] systemctl: provide better error message when start limit is exceeded.

[Lennart Poettering]

On Thu, 31.10.13 15:12, Vaclav Pavlin (vpavlin at redhat.com) wrote:

> From: Václav Pavlín <vpavlin at redhat.com>

I like the idea, but I'd prefer if we wouldn't let the service failure
field spill into the job result. I don't like redundant fields I must
say...

Also, the reason why a job failed is not just interesting for the start
limit stuff, but for other cases too. Hence, I'd much prefer if when we
see a job result of "failed" that we then read the "Result" property
from the service object and show it in a nice explanation string, one of
which the is the start limit thing. Such a change would only require
reading one more prop if we got "failed" as result, and would need
changes on the client side only.

I hope this makes sene? 

> 
> ---
>  src/core/job.c            | 12 ++++++++++--
>  src/core/job.h            |  1 +
>  src/systemctl/systemctl.c |  5 +++++
>  3 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/src/core/job.c b/src/core/job.c
> index e5dcef7..44b6b23 100644
> --- a/src/core/job.c
> +++ b/src/core/job.c
> @@ -752,6 +752,7 @@ int job_finish_and_invalidate(Job *j, JobResult result, bool recursive) {
>          Unit *other;
>          JobType t;
>          Iterator i;
> +        JobResult r = result;
>  
>          assert(j);
>          assert(j->installed);
> @@ -760,7 +761,13 @@ int job_finish_and_invalidate(Job *j, JobResult result, bool recursive) {
>          u = j->unit;
>          t = j->type;
>  
> -        j->result = result;
> +        if (result == JOB_FAILED && u->type == UNIT_SERVICE) {
> +                Service *s = SERVICE(u);
> +                if (s->result == SERVICE_FAILURE_START_LIMIT)
> +                        r = JOB_FAILED_LIMIT;
> +        }
> +
> +        j->result = r;
>  
>          if (j->state == JOB_RUNNING)
>                  j->manager->n_running_jobs--;
> @@ -1140,7 +1147,8 @@ static const char* const job_result_table[_JOB_RESULT_MAX] = {
>          [JOB_TIMEOUT] = "timeout",
>          [JOB_FAILED] = "failed",
>          [JOB_DEPENDENCY] = "dependency",
> -        [JOB_SKIPPED] = "skipped"
> +        [JOB_SKIPPED] = "skipped",
> +        [JOB_FAILED_LIMIT] = "failed-limit"
>  };
>  
>  DEFINE_STRING_TABLE_LOOKUP(job_result, JobResult);
> diff --git a/src/core/job.h b/src/core/job.h
> index d90bc96..a41bfd9 100644
> --- a/src/core/job.h
> +++ b/src/core/job.h
> @@ -98,6 +98,7 @@ enum JobResult {
>          JOB_FAILED,              /* Job failed */
>          JOB_DEPENDENCY,          /* A required dependency job did not result in JOB_DONE */
>          JOB_SKIPPED,             /* JOB_RELOAD of inactive unit; negative result of JOB_VERIFY_ACTIVE */
> +        JOB_FAILED_LIMIT,        /* Service start limit exceeded */
>          _JOB_RESULT_MAX,
>          _JOB_RESULT_INVALID = -1
>  };
> diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
> index d458c65..67c25c8 100644
> --- a/src/systemctl/systemctl.c
> +++ b/src/systemctl/systemctl.c
> @@ -1602,6 +1602,11 @@ static int wait_for_jobs(DBusConnection *bus, Set *s) {
>                                  log_error("Job for %s canceled.", strna(d.name));
>                          else if (streq(d.result, "dependency"))
>                                  log_error("A dependency job for %s failed. See 'journalctl -xn' for details.", strna(d.name));
> +                        else if (streq(d.result, "failed-limit"))
> +                                log_error("Starting %s has been attempted too often too quickly,"
> +                                          " the repeated start of the unit has been refused. To force a start please"
> +                                          " invoke 'systemctl reset-failed %s' followed by 'systemctl"
> +                                          " start %s' again.", strna(d.name), strna(d.name), strna(d.name));
>                          else if (!streq(d.result, "done") && !streq(d.result, "skipped"))
>                                  log_error("Job for %s failed. See 'systemctl status %s' and 'journalctl -xn' for details.", strna(d.name), strna(d.name));
>                  }


Lennart

-- 
Lennart Poettering, Red Hat