Re: sv check exit code not always helpful

From: James Byrne <james.byrne_at_origamienergy.com>
Date: Fri, 20 Feb 2015 17:52:11 +0000

Hi Buck,

On 09/02/15 23:59, Buck Evan wrote:
> James, I've taken your patch and also "fixed" the "up, want down" case
> as well.
> I've tried pretty hard to match surrounding style, in the hope that this
> can be upstreamed.
>
> https://github.com/bukzor/runit/issues/1#issue-57074153
>
> From: Buck Evan <buck_at_yelp.com <mailto:buck_at_yelp.com>>
> Date: Mon, 9 Feb 2015 14:08:48 -0800
> Subject: [PATCH] the service is not ready if the current state is not wanted
>
> fixes bukzor/runit#1
> ---
> src/sv.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/src/sv.c b/src/sv.c
> index 7c6f6ed..701912c 100644
> --- a/src/sv.c
> +++ b/src/sv.c
> _at_@ -227,7 +227,11 @@ int check(char *a) {
> if (!checkscript()) return(0);
> break;
> case 'd': if (pid || svstatus[19] != 0) return(0); break;
> - case 'C': if (pid) if (!checkscript()) return(0); break;
> + case 'C':
> + switch (svstatus[17]) {
> + case 'u': if (!pid || !checkscript()) return(0); break;
> + case 'd': if (pid) return(0); break;
> + }
> case 't':
> case 'k':
> if (!pid && svstatus[17] == 'd') break;

I like your change, because it makes "sv check" behave as documented
(check that services are in the requested state), whereas my version
made it check that services are up, which is not the same.

There is a subtle coding flaw in your patch though as there is no
"break" for the "case 'C'" switch, so if svstatus[17] is 'd' and pid is
zero, the code will fall through to the case 't' code below, which is
not what you want.

Here is a rewritten version of the patch (against the original 2.1.2
code) where I've fixed this in the style of the original code:

From: James Byrne <james.byrne_at_origamienergy.com>
Date: Fri, 20 Feb 2015 17:35:55 +0000
Subject: [PATCH] Make "sv check" perform as documented.

This makes 'sv check' behave as documented - it checks that the services
are in the 'wanted' state (either up or down). If all the services
specified are in the requested state it will exit with exit code 0, and
if not it will wait until the timeout for them to reach the requested
state, and return a non-zero exit code if any are not.

---
  src/sv.c | 8 +++++++-
  1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/sv.c b/src/sv.c
index 0125795..3064711 100644
--- a/src/sv.c
+++ b/src/sv.c
_at_@ -227,7 +227,13 @@ int check(char *a) {
        if (!checkscript()) return(0);
        break;
      case 'd': if (pid || svstatus[19] != 0) return(0); break;
-    case 'C': if (pid) if (!checkscript()) return(0); break;
+    case 'C':
+      if (svstatus[17] == 'd') {
+        if (pid) return(0);
+      }
+      else
+        if (!pid || !checkscript()) return(0);
+      break;
      case 't':
      case 'k':
        if (!pid && svstatus[17] == 'd') break;

Received on Fri Feb 20 2015 - 17:52:11 UTC