Print this page
7267 SMF is fast and loose with optional dependencies (fixes)
Reviewed by: Dan McDonald <danmcd@omniti.com>
Reviewed by: Jerry Jelinek <jerry.jelinek@joyent.com>
Reviewed by: Albert Lee <trisk@omniti.com>

@@ -209,10 +209,14 @@
 
 #define is_depgrp_bypassed(v) ((v->gv_type == GVT_GROUP) && \
         ((v->gv_depgroup == DEPGRP_EXCLUDE_ALL) || \
         (v->gv_restart < RERR_RESTART)))
 
+#define is_inst_bypassed(v) ((v->gv_type == GVT_INST) && \
+        ((v->gv_flags & GV_TODISABLE) || \
+        (v->gv_flags & GV_TOOFFLINE)))
+
 static uu_list_pool_t *graph_edge_pool, *graph_vertex_pool;
 static uu_list_t *dgraph;
 static pthread_mutex_t dgraph_lock;
 
 /*

@@ -1296,11 +1300,11 @@
 
                 if (satbility && s == 0)
                         satisfiable = B_TRUE;
         }
 
-        return (!satbility || satisfiable ? 0 : -1);
+        return ((!satbility || satisfiable) ? 0 : -1);
 }
 
 /*
  * An optional_all dependency only considers elements which are configured,
  * enabled, and not in maintenance.  If any are unsatisfied, then the dependency

@@ -1335,25 +1339,19 @@
 
                         if (v->gv_state == RESTARTER_STATE_MAINT)
                                 continue;
 
                         any_qualified = B_TRUE;
-                        if (v->gv_state == RESTARTER_STATE_OFFLINE) {
+                        if (v->gv_state == RESTARTER_STATE_OFFLINE ||
+                            v->gv_state == RESTARTER_STATE_DISABLED) {
                                 /*
-                                 * For offline dependencies, treat unsatisfiable
-                                 * as satisfied.
+                                 * For offline/disabled dependencies,
+                                 * treat unsatisfiable as satisfied.
                                  */
                                 i = dependency_satisfied(v, B_TRUE);
                                 if (i == -1)
                                         i = 1;
-                        } else if (v->gv_state == RESTARTER_STATE_DISABLED) {
-                                /*
-                                 * If the instance is transitioning out of
-                                 * disabled the dependency is temporarily
-                                 * unsatisfied (not unsatisfiable).
-                                 */
-                                i = v->gv_flags & GV_ENABLED ? 0 : 1;
                         } else {
                                 i = dependency_satisfied(v, satbility);
                         }
                         break;
 

@@ -1544,31 +1542,39 @@
                         }
                         return (-1);
                 }
 
                 /*
-                 * Any vertex with the GV_TODISABLE flag set is guaranteed
-                 * to have its dependencies unsatisfiable.  Any vertex with
-                 * GV_TOOFFLINE may be satisfied after it transitions.
+                 * Vertices may be transitioning so we try to figure out if
+                 * the end state is likely to satisfy the dependency instead
+                 * of assuming the dependency is unsatisfied/unsatisfiable.
+                 *
+                 * Support for optional_all dependencies depends on us getting
+                 * this right because unsatisfiable dependencies are treated
+                 * as being satisfied.
                  */
+                switch (v->gv_state) {
+                case RESTARTER_STATE_ONLINE:
+                case RESTARTER_STATE_DEGRADED:
                 if (v->gv_flags & GV_TODISABLE)
                         return (-1);
                 if (v->gv_flags & GV_TOOFFLINE)
                         return (0);
-
-                switch (v->gv_state) {
-                case RESTARTER_STATE_ONLINE:
-                case RESTARTER_STATE_DEGRADED:
                         return (1);
 
                 case RESTARTER_STATE_OFFLINE:
-                        if (!satbility)
-                                return (0);
+                        if (!satbility || v->gv_flags & GV_TODISABLE)
+                                return (satbility ? -1 : 0);
                         return (instance_satisfied(v, satbility) != -1 ?
                             0 : -1);
 
                 case RESTARTER_STATE_DISABLED:
+                        if (!satbility || !(v->gv_flags & GV_ENABLED))
+                                return (satbility ? -1 : 0);
+                        return (instance_satisfied(v, satbility) != -1 ?
+                            0 : -1);
+
                 case RESTARTER_STATE_MAINT:
                         return (-1);
 
                 case RESTARTER_STATE_UNINIT:
                         return (0);

@@ -1667,11 +1673,11 @@
  */
 /*ARGSUSED*/
 static int
 satbility_cb(graph_vertex_t *v, void *arg)
 {
-        if (v->gv_flags & GV_TOOFFLINE)
+        if (is_inst_bypassed(v))
                 return (UU_WALK_NEXT);
 
         if (v->gv_type == GVT_INST)
                 graph_start_if_satisfied(v);
 

@@ -1698,11 +1704,11 @@
 static void
 propagate_start(graph_vertex_t *v, void *arg)
 {
         restarter_error_t err = (restarter_error_t)arg;
 
-        if (v->gv_flags & GV_TOOFFLINE)
+        if (is_inst_bypassed(v))
                 return;
 
         switch (v->gv_type) {
         case GVT_INST:
                 /* Restarter */

@@ -1758,11 +1764,11 @@
 static void
 propagate_stop(graph_vertex_t *v, void *arg)
 {
         restarter_error_t err = (restarter_error_t)arg;
 
-        if (v->gv_flags & GV_TOOFFLINE)
+        if (is_inst_bypassed(v))
                 return;
 
         switch (v->gv_type) {
         case GVT_INST:
                 /* Restarter */

@@ -1788,11 +1794,12 @@
                 abort();
                 /* NOTREACHED */
 
         case GVT_GROUP:
                 if (v->gv_depgroup == DEPGRP_EXCLUDE_ALL) {
-                        graph_walk_dependents(v, propagate_start, NULL);
+                        graph_walk_dependents(v, propagate_start,
+                            (void *)RERR_NONE);
                         break;
                 }
 
                 if (err == RERR_NONE || err > v->gv_restart)
                         break;

@@ -4356,11 +4363,11 @@
                                 return (B_FALSE);
                 } else {
                         /*
                          * Skip all excluded dependents and decide whether
                          * to offline the service based on the restart_on
-                         * on attribute.
+                         * attribute.
                          */
                         if (is_depgrp_bypassed(vv))
                                 continue;
 
                         /*

@@ -5408,12 +5415,12 @@
         if (v->gv_flags & GV_TOOFFLINE)
                 return (UU_WALK_NEXT);
 
         switch (v->gv_type) {
         case GVT_INST:
-                /* If the instance is already disabled, skip it. */
-                if (!(v->gv_flags & GV_ENABLED))
+                /* If the instance is already offline, skip it. */
+                if (!inst_running(v))
                         return (UU_WALK_NEXT);
 
                 v->gv_flags |= GV_TOOFFLINE;
                 log_framework(LOG_DEBUG, "%s added to subtree\n", v->gv_name);
                 break;