On Thu, 31.10.13 15:12, Vaclav Pavlin (vpav...@redhat.com) wrote:
> From: Václav Pavlín <vpav...@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
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
