[systemd-devel] [PATCH v3] log: be more verbose if dbus job fails

[Michal Sekletar]

On Fri, Apr 10, 2015 at 12:50:35PM +0200, Lennart Poettering wrote:
> On Fri, 10.04.15 12:40, Michal Sekletar (msekleta at redhat.com) wrote:
> 
> > diff --git a/src/shared/log.c b/src/shared/log.c
> > index 646a1d6..839051a 100644
> > --- a/src/shared/log.c
> > +++ b/src/shared/log.c
> > @@ -1061,3 +1061,45 @@ void log_received_signal(int level, const struct signalfd_siginfo *si) {
> >  void log_set_upgrade_syslog_to_journal(bool b) {
> >          upgrade_syslog_to_journal = b;
> >  }
> 
> Hmm, what's the rationale for adding this to log.c? I mean, so far
> log.c contained mostly generic logging infrastructure.

I put it in log.c because function is called from sd-bus/bus-util.c and it
seemed wrong to me that code from there would call function in systemctl.c.

> 
> Is this used at more than one place? If not, can't this stay in systemctl.c?
> 
> > +        _cleanup_free_ char *service_shell_quoted = NULL;
> > +
> > +        assert(service);
> > +        assert(result);
> > +
> > +        service_shell_quoted = shell_maybe_quote(service);
> > +
> > +        for (i = 0; i < ELEMENTSOF(explanations); ++i)
> > +                if (streq(result, explanations[i].result))
> > +                        break;
> > +
> > +        assert(i < ELEMENTSOF(explanations));
> > +
> > +        log_error("Job for %s failed because %s. See \"systemctl status %s\" and \"journalctl -xe\" for details.\n",
> > +                  service,
> > +                  explanations[i].explanation,
> > +                  service_shell_quoted);
> 
> servce_shell_quoted should probably be enclosed in strna(), since the
> call may actually fail due to OOM. Which is unlikely, and of course a
> pity, but then again, I think it's Ok if we still print the rest of
> the message but not abort in this case...

Will do that.

> 
> > +
> > +        /* For some results maybe additional explanation is required */
> > +        if (streq_ptr(result, "start-limit"))
> > +                log_info("To force a start please invoke \"systemctl reset-failed %s\" followed by \"systemctl start %s\" again.",
> > +                         service_shell_quoted,
> > +                         service_shell_quoted);
> 
> Similar here.
> 
> Lennart

Thanks,

Michal

> 
> -- 
> Lennart Poettering, Red Hat