Re: wrong exit code for sv status in lsb mode

["David Mountney via supervision" <supervision@list.skarnet.org>]

[-- Attachment #1: Type: text/plain, Size: 4958 bytes --]

The error was introduced in commit 5fe1bc773c2d979093fe4b1f3ecbbed5e6acdaf0
"sv.c: properly format status command's output on failure cases."

Where the log status is being saved to the same variable as the service
status. The quick fix would be to not record the log status for now:

$ git diff src/sv.c
diff --git a/src/sv.c b/src/sv.c
index 9003142..1676227 100644
--- a/src/sv.c
+++ b/src/sv.c
@@ -167,7 +167,7 @@ int status(char *unused) {
   }
   else {
     outs("; ");
-    if (svstatus_get()) { rc =svstatus_print("log"); outs("\n"); }
+    if (svstatus_get()) { svstatus_print("log"); outs("\n"); }
   }
   islog =0;
   flush("");
$

On Tue, Jan 8, 2019 at 12:18 AM Gerrit Pape <pape@smarden.org> wrote:

> Hi,
>
> donations for my free software increased recently, not sure why, but
> thanks.
>
> runit upstream sources with history still are available through
>
>  $ git clone http://smarden.org/git/runit.git
>
> Below is the diff for sv.c from 2.1.1 to 2.1.2.  Possibly anyone can
> spot the error and create a patch if that issue is annoying.
>
> Regards, Gerrit.
>
>
> $ git diff v2.1.1..v2.1.2 src/sv.c
> diff --git a/src/sv.c b/src/sv.c
> index d126cee..9003142 100644
> --- a/src/sv.c
> +++ b/src/sv.c
> @@ -41,6 +41,7 @@ unsigned int lsb;
>  unsigned int verbose =0;
>  unsigned long wait =7;
>  unsigned int kll =0;
> +unsigned int islog =0;
>  struct taia tstart, tnow, tdiff;
>  struct tai tstatus;
>
> @@ -67,6 +68,7 @@ void fatal2(char *m1, char *m2) {
>  void out(char *p, char *m1) {
>    buffer_puts(buffer_1, p);
>    buffer_puts(buffer_1, *service);
> +  if (islog) buffer_puts(buffer_1, "/log");
>    buffer_puts(buffer_1, ": ");
>    buffer_puts(buffer_1, m1);
>    if (errno) {
> @@ -153,20 +155,22 @@ int status(char *unused) {
>    int rc;
>
>    rc =svstatus_get();
> -  switch(r) { case -1: if (lsb) done(4); case 0: return(0); }
> +  switch(rc) { case -1: if (lsb) done(4); case 0: return(0); }
>    rc =svstatus_print(*service);
> +  islog =1;
>    if (chdir("log") == -1) {
>      if (errno != error_noent) {
> -      outs("; log: "); outs(WARN);
> -      outs("unable to change to log service directory: ");
> -      outs(error_str(errno));
> +      outs("; ");
> +      warn("unable to change directory");
>      }
> +    else outs("\n");
>    }
> -  else
> -    if (svstatus_get()) {
> -      outs("; "); svstatus_print("log");
> -    }
> -  flush("\n");
> +  else {
> +    outs("; ");
> +    if (svstatus_get()) { rc =svstatus_print("log"); outs("\n"); }
> +  }
> +  islog =0;
> +  flush("");
>    if (lsb) switch(rc) { case 1: done(0); case 2: done(3); case 0:
> done(4); }
>    return(rc);
>  }
> @@ -305,9 +309,11 @@ int main(int argc, char **argv) {
>      acts ="d"; kll =1; cbk =&check; break;
>    case 'T':
>      acts ="tc"; kll =1; cbk =&check; break;
> +  case 't':
> +    if (!str_diff(action, "try-restart")) { acts ="tc"; cbk =&check;
> break; }
>    case 'c':
>      if (!str_diff(action, "check")) { act =0; acts ="C"; cbk =&check;
> break; }
> -  case 'u': case 'd': case 'o': case 't': case 'p': case 'h':
> +  case 'u': case 'd': case 'o': case 'p': case 'h':
>    case 'a': case 'i': case 'k': case 'q': case '1': case '2':
>      action[1] =0; acts =action; break;
>    case 's':
> @@ -318,6 +324,7 @@ int main(int argc, char **argv) {
>      act =&status; cbk =0; break;
>    case 'r':
>      if (!str_diff(action, "restart")) { acts ="tcu"; cbk =&check; break; }
> +    if (!str_diff(action, "reload")) { acts ="h"; cbk =&check; break; }
>      usage();
>    case 'f':
>      if (!str_diff(action, "force-reload"))
> $
>
>
> On Fri, Jan 04, 2019 at 05:10:13PM -0800, David Mountney via supervision
> wrote:
> > This looks like it's still a problem introduced in runit 2.1.2
> >
> > If you have defined a log handler, only the status of the log handler is
> > returned by sv status.
> >
> > Its not clear to me from the changelog why this change was made.
> >
> > I guess, ideally if the service status was successful, you could then
> also
> > check the log status, but as it is now, if the service is down, but the
> log
> > is up, sv status reports all is well for its exit codes.
> >
> > I've seen other references to this bug, where projects have reverted to
> the
> > previous runit version:
> https://github.com/chef/omnibus-software/pull/793
> >
> > It would be ideal if we could just get this fixed.
> >
> >
> > > From: Fabian Ruff
> > > Date: Tue, 18 Aug 2015 23:33:50 +0200
> > >  Tue, 18 Aug 2015 23:33:50 +0200
> > > Hi,
> > > I just noticed that the exit code of sv status in "lsb" mode is not 3
> in
> > > all cases when
> > > the service is "down".
> > > The problem arises when a service defines a log handler. In that case
> the
> > > return code
> > > of svstatus print for the log process overwrites the return code of
> > > svstatus print of
> > > the actual supervised process (line 170 of sv.c).
> > > Is this intentional? Looks like a bug to me.
> > > Kind regards,
>