Welcome to little lamb

Code » anopa » commit 880d5ad

stop: Process "needs" in reverse order

author Olivier Brunel
2015-07-21 19:50:33 UTC
committer Olivier Brunel
2015-07-22 17:53:06 UTC
parent c602de631a83a2aaf38ee791e1cc9a720d3dd7b6

stop: Process "needs" in reverse order

We used to only stop whatever was specified, processing dependencies
("needs") only as ordering directives (i.e. as "after"), which obviously
isn't right.

Now they're correctly processed "in reverse," i.e. if A needs B then we'll
set it as B needs A, i.e. to stop B we also need to stop A (and the
matching "after" will be there to order stopping A before B).

doc/aa-stop.pod +14 -6
src/anopa/aa-stop.c +120 -37
src/libanopa/service.c +22 -8
src/libanopa/service_stop.c +17 -0

diff --git a/doc/aa-stop.pod b/doc/aa-stop.pod
index 1a856ab..51779da 100644
--- a/doc/aa-stop.pod
+++ b/doc/aa-stop.pod
@@ -15,7 +15,14 @@ B<aa-stop> [B<-D>] [B<-r> I<repodir>] [B<-l> I<listdir>] [B<-a>]
 
 Stops all running/started services. This option is intended to be used during
 stage 3; When used, you shouldn't specify any service on the command line.
-See below for its implications.
+
+Note that when used, failing dependencies will not cause not to stop services.
+That is, is A needs B, stopping B would depend on stopping A first, and if that
+failed B wouldn't be stopped (Stopping failed: Failed dependency: A).
+However with this option B would be stopped, as if A had been successfully
+stopped.
+
+Also see below for more implications.
 
 =item B<-D, --double-output>
 
@@ -64,12 +71,13 @@ Show version information and exit.
 
 =head1 DESCRIPTION
 
-B<aa-stop>(1) allows to stop one or more services. Unlike B<aa-start>(1) it
-doesn't take into account any dependency relation, and the only services that
-will be stopped are those asked to be stopped.
+B<aa-stop>(1) allows to stop one or more services. It works in similar fashion
+to B<aa-start>(1) but processing order and dependencies "in reverse" so to
+speak.
 
-It does however accounts for I<after> and I<before> to stop services in reverse
-order they were started in, treating I<needs> exactly as it does I<after>.
+That is to say if service A was to be started after service B, then it will be
+stopped before B. And if A had a dependency (I<needs>) on C, then stopping C
+will also cause for A to be stopped.
 
 Refer to B<anopa>(1) for descriptions of servicedirs and service dependencies.
 
diff --git a/src/anopa/aa-stop.c b/src/anopa/aa-stop.c
index 72007a9..fe19b71 100644
--- a/src/anopa/aa-stop.c
+++ b/src/anopa/aa-stop.c
@@ -52,6 +52,7 @@
 
 
 static genalloc ga_unknown = GENALLOC_ZERO;
+static genalloc ga_depend = GENALLOC_ZERO;
 static genalloc ga_io = GENALLOC_ZERO;
 static aa_mode mode = AA_MODE_STOP;
 static int rc = 0;
@@ -64,7 +65,7 @@ check_essential (int si)
 }
 
 static int
-add_service (const char *name)
+preload_service (const char *name)
 {
     int si = -1;
     int type;
@@ -73,18 +74,67 @@ add_service (const char *name)
     if (skip && str_equal (name, skip))
         return 0;
 
-    type = aa_get_service (name, &si, 1);
+    type = aa_get_service (name, &si, 0);
     if (type < 0)
         r = type;
     else
         r = aa_ensure_service_loaded (si, AA_MODE_STOP, 0, NULL);
     if (r < 0)
     {
-        if (type == AA_SERVICE_FROM_MAIN)
+        /* there should be much errors possible here... basically only ERR_IO or
+         * ERR_NOT_UP should be possible, and the later should be silently
+         * ignored... so: */
+        if (r == -ERR_IO)
         {
-            add_to_list (&aa_tmp_list, si, 1);
-            remove_from_list (&aa_main_list, si);
+            /* ERR_IO from aa_get_service() means we don't have a si (it is
+             * actually set to the return value); but from aa_mark_service()
+             * (e.g. to read "needs") then we do */
+            if (si < 0)
+                put_err_service (name, ERR_IO, 1);
+            else
+            {
+                int e = errno;
+
+                put_err_service (name, ERR_IO, 0);
+                add_err (": ");
+                add_err (error_str (e));
+                end_err ();
+            }
         }
+    }
+
+    return 0;
+}
+
+static int
+it_preload (direntry *d, void *data)
+{
+    if (*d->d_name == '.' || d->d_type != DT_DIR)
+        return 0;
+
+    tain_now_g ();
+    preload_service (d->d_name);
+
+    return 0;
+}
+
+static int
+add_service (const char *name)
+{
+    int si = -1;
+    int type;
+    int r;
+
+    if (skip && str_equal (name, skip))
+        return 0;
+
+    type = aa_get_service (name, &si, 0);
+    if (type < 0)
+    {
+        r = type;
+
+        /* since everything was preloaded (and so we don't ensure_loaded here),
+         * it can only be ERR_UNKNOWN or possibly ERR_IO, nothing else */
 
         if (r == -ERR_UNKNOWN)
         {
@@ -113,52 +163,46 @@ add_service (const char *name)
                 genalloc_append (int, &ga_failed, &si);
             }
         }
-        else if (r == -ERR_NOT_UP)
+    }
+    else if (type == AA_SERVICE_FROM_TMP)
+    {
+        /* it could be an ERR_NOT_UP, that was on tmp from preload */
+        if (aa_service (si)->ls == AA_LOAD_FAIL)
         {
-            if (!(mode & AA_MODE_STOP_ALL))
+            /* sanity check */
+            if (aa_service (si)->st.code != ERR_NOT_UP)
             {
-                if (!(mode & AA_MODE_IS_DRY))
-                    put_title (1, name, errmsg[-r], 1);
-                ++nb_already;
+                errno = EINVAL;
+                strerr_diefu1sys (ERR_IO, "add service");
             }
+
+            if (!(mode & AA_MODE_IS_DRY))
+                put_title (1, name, errmsg[ERR_NOT_UP], 1);
+            ++nb_already;
             r = 0;
         }
         else
         {
-            aa_service *s = aa_service (si);
-            const char *msg = aa_service_status_get_msg (&s->st);
-            int has_msg;
+            int i;
 
-            has_msg = s->st.event == AA_EVT_ERROR && s->st.code == -r && !!msg;
-            put_err_service (name, -r, !has_msg);
-            if (has_msg)
+            add_to_list (&aa_main_list, si, 0);
+            remove_from_list (&aa_tmp_list, si);
+
+            for (i = 0; i < genalloc_len (int, &aa_service (si)->needs); ++i)
             {
-                add_err (": ");
-                add_err (msg);
-                end_err ();
+                int sni = list_get (&aa_service (si)->needs, i);
+                add_service (aa_service_name (aa_service (sni)));
             }
-
-            genalloc_append (int, &ga_failed, &si);
         }
     }
-    else
-    {
-        if (type == AA_SERVICE_FROM_TMP)
-        {
-            add_to_list (&aa_main_list, si, 1);
-            remove_from_list (&aa_tmp_list, si);
-        }
 
-        r = 0;
-    }
-
-    return r;
+    return 0;
 }
 
 static int
 it_stop (direntry *d, void *data)
 {
-    if (*d->d_name == '.' || (data && d->d_type != DT_DIR))
+    if (*d->d_name == '.')
         return 0;
 
     tain_now_g ();
@@ -167,6 +211,16 @@ it_stop (direntry *d, void *data)
     return 0;
 }
 
+static void
+scan_cb (int si, int sni)
+{
+    put_err_service (aa_service_name (aa_service (si)), ERR_DEPEND, 0);
+    add_err (": ");
+    add_err (aa_service_name (aa_service (sni)));
+    end_err ();
+    genalloc_append (int, &ga_depend, &si);
+}
+
 static void
 dieusage (int rc)
 {
@@ -275,17 +329,44 @@ main (int argc, char * const argv[])
     if (aa_init_repo (path_repo, AA_REPO_WRITE) < 0)
         strerr_diefu2sys (ERR_IO, "init repository ", path_repo);
 
-    if (mode & AA_MODE_STOP_ALL)
+    /* let's "preload" every services from the repo. This will have everything
+     * in tmp list, either LOAD_DONE when up, or LOAD_FAIL when not
+     * (ERR_NOT_UP).
+     * The idea is to load dependencies, so in case service A needs B, we've
+     * added into the "needs" of B the service A, i.e. stopping B means a need
+     * to also stop A (as always, an "after" will handle the ordering).
+     */
     {
         stralloc sa = STRALLOC_ZERO;
         int r;
 
         stralloc_catb (&sa, ".", 2);
-        r = aa_scan_dir (&sa, 0, it_stop, (void *) 1);
+        r = aa_scan_dir (&sa, 0, it_preload, NULL);
         stralloc_free (&sa);
         if (r < 0)
             strerr_diefu1sys (-r, "read repository directory");
     }
+
+    if (mode & AA_MODE_STOP_ALL)
+    {
+        /* to stop all (up) services, since we've preloaded everything, simply
+         * means moving all services from tmp to main list. We just need to make
+         * sure to process "valid" services, since there could be LOAD_FAIL ones
+         * (ERR_NOT_UP) that we should simply ignore.
+         */
+        for (i = 0; i < genalloc_len (int, &aa_tmp_list); )
+        {
+            int si = list_get (&aa_tmp_list, i);
+
+            if (aa_service (si)->ls == AA_LOAD_DONE)
+            {
+                add_to_list (&aa_main_list, si, 0);
+                remove_from_list (&aa_tmp_list, si);
+            }
+            else
+                ++i;
+        }
+    }
     else if (path_list)
     {
         stralloc sa = STRALLOC_ZERO;
@@ -294,7 +375,7 @@ main (int argc, char * const argv[])
         if (*path_list != '/' && *path_list != '.')
             stralloc_cats (&sa, LISTDIR_PREFIX);
         stralloc_catb (&sa, path_list, strlen (path_list) + 1);
-        r = aa_scan_dir (&sa, 1, it_stop, (void *) 0);
+        r = aa_scan_dir (&sa, 1, it_stop, NULL);
         stralloc_free (&sa);
         if (r < 0)
             strerr_diefu3sys (-r, "read list directory ",
@@ -307,7 +388,7 @@ main (int argc, char * const argv[])
 
     tain_now_g ();
 
-    mainloop (mode, NULL);
+    mainloop (mode, scan_cb);
 
     if (!(mode & AA_MODE_IS_DRY))
     {
@@ -317,6 +398,7 @@ main (int argc, char * const argv[])
         aa_show_stat_nb (nb_done, "Stopped", ANSI_HIGHLIGHT_GREEN_ON);
         show_stat_service_names (&ga_timedout, "Timed out", ANSI_HIGHLIGHT_RED_ON);
         show_stat_service_names (&ga_failed, "Failed", ANSI_HIGHLIGHT_RED_ON);
+        show_stat_service_names (&ga_depend, "Dependency failed", ANSI_HIGHLIGHT_RED_ON);
         aa_show_stat_names (aa_names.s, &ga_io, "I/O error", ANSI_HIGHLIGHT_RED_ON);
         aa_show_stat_names (aa_names.s, &ga_unknown, "Unknown", ANSI_HIGHLIGHT_RED_ON);
     }
@@ -324,6 +406,7 @@ main (int argc, char * const argv[])
     genalloc_free (int, &ga_timedout);
     genalloc_free (int, &ga_failed);
     genalloc_free (int, &ga_unknown);
+    genalloc_free (int, &ga_depend);
     genalloc_free (int, &ga_io);
     genalloc_free (pid_t, &ga_pid);
     genalloc_free (int, &aa_tmp_list);
diff --git a/src/libanopa/service.c b/src/libanopa/service.c
index b12a30c..5eb87d3 100644
--- a/src/libanopa/service.c
+++ b/src/libanopa/service.c
@@ -251,7 +251,7 @@ aa_ensure_service_loaded (int si, aa_mode mode, int no_wants, aa_load_fail_cb lf
 
     stralloc_catb (&sa, "/needs", strlen ("/needs") + 1);
     r = aa_scan_dir (&sa, 1,
-            (mode & AA_MODE_START) ? _it_start_needs : _it_stop_after,
+            (mode & AA_MODE_START) ? _it_start_needs : _it_stop_needs,
             &it_data);
     /* we can get ERR_IO either from aa_scan_dir() itself, or from the iterator
      * function. But since we haven't checked that the directory (needs) does
@@ -456,14 +456,23 @@ aa_prepare_mainlist (aa_prepare_cb prepare_cb, aa_exec_cb exec_cb)
 }
 
 static int
-service_is_ok (aa_service *s)
+service_is_ok (aa_mode mode, aa_service *s)
 {
     aa_service_status *svst = &s->st;
     s6_svstatus_t st6 = S6_SVSTATUS_ZERO;
+    aa_evt event;
     int r;
 
+    /* if DRY we assume it's ok, since it wasn't really started/stopped.
+     * if STOP_ALL we pretend it's ok since we're trying to stop everything. */
+    if (mode & (AA_MODE_IS_DRY | AA_MODE_STOP_ALL))
+        return 1;
+
     if (svst->type == AA_TYPE_ONESHOT)
-        return (svst->event == AA_EVT_STARTED) ? 1 : 0;
+    {
+        event = (mode & AA_MODE_START) ? AA_EVT_STARTED : AA_EVT_STOPPED;
+        return (svst->event == event) ? 1 : 0;
+    }
 
     /* TYPE_LONGRUN -- we make assumptions here:
      * - we have a local status, since we started the service
@@ -474,8 +483,9 @@ service_is_ok (aa_service *s)
      *   for the 'U' event (ready)). Actually we'll allow for our event to be
      *   EVT_STARTING because there's a possible race condition there.
      */
+    event = (mode & AA_MODE_START) ? AA_EVT_STARTING : AA_EVT_STOPPING;
     if (s6_svstatus_read (aa_service_name (s), &st6)
-            && (tain_less (&svst->stamp, &st6.stamp) || svst->event == AA_EVT_STARTING))
+            && (tain_less (&svst->stamp, &st6.stamp) || svst->event == event))
         r = 1;
     else
         r = 0;
@@ -501,6 +511,7 @@ aa_scan_mainlist (aa_scan_cb scan_cb, aa_mode mode)
         for (j = 0; j < genalloc_len (int, &s->needs); )
         {
             int sni;
+            aa_service_status *svst;
 
             sni = list_get (&s->needs, j);
             if (is_in_list (&aa_main_list, sni))
@@ -509,16 +520,19 @@ aa_scan_mainlist (aa_scan_cb scan_cb, aa_mode mode)
                 continue;
             }
 
-            /* if DRY we assume it's ok, since it wasn't really started */
-            if ((mode & AA_MODE_IS_DRY) || service_is_ok (aa_service (sni)))
+            if (service_is_ok (mode, aa_service (sni)))
             {
                 remove_from_list (&s->needs, sni);
                 remove_from_list (&s->after, sni);
                 continue;
             }
 
-            aa_service_status_set_err (&s->st, ERR_DEPEND, aa_service_name (aa_service (sni)));
-            if (aa_service_status_write (&s->st, aa_service_name (s)) < 0)
+            svst = &s->st;
+            svst->event = (mode & AA_MODE_START) ? AA_EVT_STARTING_FAILED: AA_EVT_STOPPING_FAILED;
+            svst->code = ERR_DEPEND;
+            tain_copynow (&svst->stamp);
+            aa_service_status_set_msg (svst,aa_service_name (aa_service (sni)));
+            if (aa_service_status_write (svst, aa_service_name (s)) < 0)
                 strerr_warnwu2sys ("write service status file for ", aa_service_name (s));
 
             remove_from_list (&aa_main_list, si);
diff --git a/src/libanopa/service_stop.c b/src/libanopa/service_stop.c
index 9c32679..6564144 100644
--- a/src/libanopa/service_stop.c
+++ b/src/libanopa/service_stop.c
@@ -25,6 +25,23 @@
 #include <anopa/ga_int_list.h>
 #include "service_internal.h"
 
+int
+_it_stop_needs (direntry *d, void *data)
+{
+    struct it_data *it_data = data;
+    int sni;
+    int r;
+
+    tain_now_g ();
+    r = aa_get_service (d->d_name, &sni, 0);
+    if (r < 0)
+        return 0;
+
+    add_to_list (&aa_service (sni)->needs, it_data->si, 0);
+    add_to_list (&aa_service (sni)->after, it_data->si, 1);
+    return 0;
+}
+
 int
 _it_stop_after (direntry *d, void *data)
 {