libxl: spawn: Preserve rc in error path

Make spawn provide an rc to its caller, and either pass it through
from the timeout callback, or invent ERROR_FAIL, as applicable.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
v4: New patch in this version of the series.

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 33f9ce62462b3b34c36219d1bbdeafcf45305df4..317a8ebfcf646cdf884478c2d2635ed22cdeea91 100644 (file)
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1370,7 +1370,8 @@ static void stubdom_xswait_cb(libxl__egc *egc, libxl__xswait_state *xswait,
 static void device_model_confirm(libxl__egc *egc, libxl__spawn_state *spawn,
                                  const char *xsdata);
 static void device_model_startup_failed(libxl__egc *egc,
-                                        libxl__spawn_state *spawn);
+                                        libxl__spawn_state *spawn,
+                                        int rc);
 static void device_model_detached(libxl__egc *egc,
                                   libxl__spawn_state *spawn);
 
@@ -1540,10 +1541,11 @@ static void device_model_confirm(libxl__egc *egc, libxl__spawn_state *spawn,
 }
 
 static void device_model_startup_failed(libxl__egc *egc,
-                                        libxl__spawn_state *spawn)
+                                        libxl__spawn_state *spawn,
+                                        int rc)
 {
     libxl__dm_spawn_state *dmss = CONTAINER_OF(spawn, *dmss, spawn);
-    device_model_spawn_outcome(egc, dmss, ERROR_FAIL);
+    device_model_spawn_outcome(egc, dmss, rc);
 }
 
 static void device_model_detached(libxl__egc *egc,

diff --git a/tools/libxl/libxl_exec.c b/tools/libxl/libxl_exec.c
index 478b4c22d1cfb4253963cef49a9f3677533f3559..85cbde05cb8c601c4c8550bc4c0673583d5febf7 100644 (file)
--- a/tools/libxl/libxl_exec.c
+++ b/tools/libxl/libxl_exec.c
@@ -238,11 +238,11 @@ err:
 /*
  * Full set of possible states of a libxl__spawn_state and its _detachable:
  *
- *                   detaching failed  mid     timeout      xswatch          
+ *                   detaching rc      mid     timeout      xswatch
  *  - Undefined         undef   undef   -        undef        undef
  *  - Idle              any     any     Idle     Idle         Idle
  *  - Attached OK       0       0       Active   Active       Active
- *  - Attached Failed   0       1       Active   Idle         Idle
+ *  - Attached Failed   0       non-0   Active   Idle         Idle
  *  - Detaching         1       maybe   Active   Idle         Idle
  *  - Partial           any     any     Idle     Active/Idle  Active/Idle
  *
@@ -267,7 +267,7 @@ static void spawn_cleanup(libxl__gc *gc, libxl__spawn_state *ss);
 
 /* Precondition: Attached or Detaching; caller has logged failure reason.
  * Results: Detaching, or Attached Failed */
-static void spawn_fail(libxl__egc *egc, libxl__spawn_state *ss);
+static void spawn_fail(libxl__egc *egc, libxl__spawn_state *ss, int rc);
 
 void libxl__spawn_init(libxl__spawn_state *ss)
 {
@@ -283,7 +283,7 @@ int libxl__spawn_spawn(libxl__egc *egc, libxl__spawn_state *ss)
     int status, rc;
 
     libxl__spawn_init(ss);
-    ss->failed = ss->detaching = 0;
+    ss->rc = ss->detaching = 0;
 
     ss->xswait.ao = ao;
     ss->xswait.what = GCSPRINTF("%s startup", ss->what);
@@ -352,12 +352,13 @@ static void spawn_cleanup(libxl__gc *gc, libxl__spawn_state *ss)
 
 static void spawn_detach(libxl__gc *gc, libxl__spawn_state *ss)
 /* Precondition: Attached or Detaching, but caller must have just set
- * at least one of detaching or failed.
+ * at least one of detaching or rc.
  * Results: Detaching or Attached Failed */
 {
     int r;
 
     assert(libxl__ev_child_inuse(&ss->mid));
+    assert(ss->detaching || ss->rc);
     libxl__xswait_stop(gc, &ss->xswait);
 
     pid_t child = ss->mid.pid;
@@ -373,12 +374,13 @@ void libxl__spawn_initiate_detach(libxl__gc *gc, libxl__spawn_state *ss)
     spawn_detach(gc, ss);
 }
 
-static void spawn_fail(libxl__egc *egc, libxl__spawn_state *ss)
+static void spawn_fail(libxl__egc *egc, libxl__spawn_state *ss, int rc)
 /* Caller must have logged.  Must be last thing in calling function,
  * as it may make the callback.  Precondition: Attached or Detaching. */
 {
     EGC_GC;
-    ss->failed = 1;
+    assert(rc);
+    ss->rc = rc;
     spawn_detach(gc, ss);
 }
 
@@ -391,9 +393,10 @@ static void spawn_watch_event(libxl__egc *egc, libxl__xswait_state *xswa,
     if (rc) {
         if (rc == ERROR_TIMEDOUT)
             LOG(ERROR, "%s: startup timed out", ss->what);
-        spawn_fail(egc, ss); /* must be last */
+        spawn_fail(egc, ss, rc); /* must be last */
         return;
     }
+    LOG(DEBUG, "%s: spawn watch p=%s", ss->what, p);
     ss->confirm_cb(egc, ss, p); /* must be last */
 }
 
@@ -404,7 +407,7 @@ static void spawn_middle_death(libxl__egc *egc, libxl__ev_child *childw,
     EGC_GC;
     libxl__spawn_state *ss = CONTAINER_OF(childw, *ss, mid);
 
-    if ((ss->failed || ss->detaching) &&
+    if ((ss->rc || ss->detaching) &&
         ((WIFEXITED(status) && WEXITSTATUS(status)==0) ||
          (WIFSIGNALED(status) && WTERMSIG(status)==SIGKILL))) {
         /* as expected */
@@ -413,7 +416,7 @@ static void spawn_middle_death(libxl__egc *egc, libxl__ev_child *childw,
         const char *what =
             GCSPRINTF("%s intermediate process (startup monitor)", ss->what);
         libxl_report_child_exitstatus(CTX, loglevel, what, pid, status);
-        ss->failed = 1;
+        ss->rc = ERROR_FAIL;
     } else {
         if (!status)
             LOG(ERROR, "%s [%ld]: unexpectedly exited with exit status 0,"
@@ -432,19 +435,20 @@ static void spawn_middle_death(libxl__egc *egc, libxl__ev_child *childw,
                 LOG(ERROR, "%s [%ld]: died during startup due to unknown fatal"
                     " signal number %d", ss->what, (unsigned long)pid, sig);
         }
-        ss->failed = 1;
+        ss->rc = ERROR_FAIL;
     }
 
     spawn_cleanup(gc, ss);
 
-    if (ss->failed && !ss->detaching) {
-        ss->failure_cb(egc, ss); /* must be last */
+    if (ss->rc && !ss->detaching) {
+        ss->failure_cb(egc, ss, ss->rc); /* must be last */
         return;
     }
     
-    if (ss->failed && ss->detaching)
-        LOG(WARN,"%s underlying machinery seemed to fail,"
-            " but its function seems to have been successful", ss->what);
+    if (ss->rc && ss->detaching)
+        LOG(WARN,"%s underlying machinery seemed to fail (%d),"
+            " but its function seems to have been successful",
+            ss->what, ss->rc);
 
     assert(ss->detaching);
     ss->detached_cb(egc, ss);

diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index d4617e4daaf688b082bcf4e9969577b85cd1f287..45b8a46a1d80ee2c1af946d8a71796df587f3974 100644 (file)
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1365,7 +1365,8 @@ libxl__spawn_midproc_cb(libxl__gc*, libxl__spawn_state*, pid_t inner);
  * The spawn state will be Idle on entry to the callback (and
  * it may be reused immediately if desired).
  */
-typedef void libxl__spawn_failure_cb(libxl__egc*, libxl__spawn_state*);
+typedef void libxl__spawn_failure_cb(libxl__egc*, libxl__spawn_state*,
+                                     int rc);
 
 /*
  * Called when the xspath watch triggers.  xspath will have been read
@@ -1406,7 +1407,7 @@ struct libxl__spawn_state {
 
     /* remaining fields are private to libxl_spawn_... */
     int detaching; /* we are in Detaching */
-    int failed; /* might be true whenever we are not Idle */
+    int rc; /* might be non-0 whenever we are not Idle */
     libxl__ev_child mid; /* always in use whenever we are not Idle */
     libxl__xswait_state xswait;
 };