[Xen-devel] [PATCH 4/7] netbuffer: use async exec API to exec the netbuffer script

[Lai Jiangshan <laijs@xxxxxxxxxxxxxx>]

Signed-off-by: Lai Jiangshan <laijs@xxxxxxxxxxxxxx>
Signed-off-by: Wen Congyang <wency@xxxxxxxxxxxxxx>
---
 tools/libxl/libxl.c           |    2 +-
 tools/libxl/libxl_internal.h  |    3 +-
 tools/libxl/libxl_netbuffer.c |  139 ++++++++++++-----------------------------
 3 files changed, 43 insertions(+), 101 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index c4a4751..1596146 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -747,7 +747,7 @@ int libxl_domain_remus_start(libxl_ctx *ctx, 
libxl_domain_remus_info *info,
     /* convenience shorthand */
     libxl__remus_state *remus_state = dss->remus_state;
     remus_state->dss = dss;
-    libxl__ev_child_init(&remus_state->child);
+    remus_state->egc = egc;
 
     /* TBD: enable disk buffering */
 
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index ab82334..bf92975 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2438,14 +2438,15 @@ typedef struct libxl__remus_state {
     /* Script to setup/teardown network buffers */
     const char *netbufscript;
     libxl__domain_suspend_state *dss;
+    libxl__egc *egc;
 
     /* private */
     int saved_rc;
     int dev_id;
     /* Opaque context containing network buffer related stuff */
     void *netbuf_state;
+    /* used for checkpoint */
     libxl__ev_time timeout;
-    libxl__ev_child child;
 } libxl__remus_state;
 
 _hidden int libxl__netbuffer_enabled(libxl__gc *gc);
diff --git a/tools/libxl/libxl_netbuffer.c b/tools/libxl/libxl_netbuffer.c
index c9c1ba7..d996832 100644
--- a/tools/libxl/libxl_netbuffer.c
+++ b/tools/libxl/libxl_netbuffer.c
@@ -32,7 +32,7 @@ typedef struct libxl__remus_netbuf_state {
     const char **vif_list;
     const char **ifb_list;
     uint32_t num_netbufs;
-    uint32_t unused;
+    libxl_async_exec async_exec;
 } libxl__remus_netbuf_state;
 
 int libxl__netbuffer_enabled(libxl__gc *gc)
@@ -238,52 +238,20 @@ static int init_qdiscs(libxl__gc *gc,
     return ERROR_FAIL;
 }
 
-static void netbuf_setup_timeout_cb(libxl__egc *egc,
-                                    libxl__ev_time *ev,
-                                    const struct timeval *requested_abs)
-{
-    libxl__remus_state *remus_state = CONTAINER_OF(ev, *remus_state, timeout);
-
-    /* Convenience aliases */
-    const int devid = remus_state->dev_id;
-    libxl__remus_netbuf_state *const netbuf_state = remus_state->netbuf_state;
-    const char *const vif = netbuf_state->vif_list[devid];
-
-    STATE_AO_GC(remus_state->dss->ao);
-
-    libxl__ev_time_deregister(gc, &remus_state->timeout);
-    assert(libxl__ev_child_inuse(&remus_state->child));
-
-    LOG(DEBUG, "killing hotplug script %s (on vif %s) because of timeout",
-        remus_state->netbufscript, vif);
-
-    if (kill(remus_state->child.pid, SIGKILL)) {
-        LOGEV(ERROR, errno, "unable to kill hotplug script %s [%ld]",
-              remus_state->netbufscript,
-              (unsigned long)remus_state->child.pid);
-    }
-
-    return;
-}
-
 /* the script needs the following env & args
  * $vifname
  * $XENBUS_PATH (/libxl/<domid>/remus/netbuf/<devid>/)
  * $IFB (for teardown)
  * setup/teardown as command line arg.
- * In return, the script writes the name of IFB device (during setup) to be
- * used for output buffering into XENBUS_PATH/ifb
  */
-static int exec_netbuf_script(libxl__gc *gc, libxl__remus_state *remus_state,
-                              char *op, libxl__ev_child_callback *death)
+static void setup_env(libxl_async_exec *async_exec, char *op,
+                      libxl__remus_state *remus_state)
 {
     int arraysize, nr = 0;
     char **env = NULL, **args = NULL;
-    pid_t pid;
+    STATE_AO_GC(remus_state->dss->ao);
 
     /* Convenience aliases */
-    libxl__ev_child *const child = &remus_state->child;
-    libxl__ev_time *const timeout = &remus_state->timeout;
     char *const script = libxl__strdup(gc, remus_state->netbufscript);
     const uint32_t domid = remus_state->dss->domid;
     const int devid = remus_state->dev_id;
@@ -312,40 +280,17 @@ static int exec_netbuf_script(libxl__gc *gc, 
libxl__remus_state *remus_state,
     args[nr++] = NULL;
     assert(nr == arraysize);
 
-    /* Set hotplug timeout */
-    if (libxl__ev_time_register_rel(gc, timeout,
-                                    netbuf_setup_timeout_cb,
-                                    LIBXL_HOTPLUG_TIMEOUT * 1000)) {
-        LOG(ERROR, "unable to register timeout for "
-            "netbuf setup script %s on vif %s", script, vif);
-        return ERROR_FAIL;
-    }
-
-    LOG(DEBUG, "Calling netbuf script: %s %s on vif %s",
-        script, op, vif);
-
-    /* Fork and exec netbuf script */
-    pid = libxl__ev_child_fork(gc, child, death);
-    if (pid == -1) {
-        LOG(ERROR, "unable to fork netbuf script %s", script);
-        return ERROR_FAIL;
-    }
-
-    if (!pid) {
-        /* child: Launch netbuf script */
-        libxl__exec(gc, -1, -1, -1, args[0], args, env);
-        /* notreached */
-        abort();
-    }
-
-    return 0;
+    async_exec->env = env;
+    async_exec->args = args;
 }
 
-static void netbuf_setup_script_cb(libxl__egc *egc,
-                                   libxl__ev_child *child,
-                                   pid_t pid, int status)
+/*
+ * In return, the script writes the name of IFB device (during setup) to be
+ * used for output buffering into XENBUS_PATH/ifb
+ */
+static void netbuf_setup_script_cb(void *opaque, int status)
 {
-    libxl__remus_state *remus_state = CONTAINER_OF(child, *remus_state, child);
+    libxl__remus_state *remus_state = opaque;
     const char *out_path_base, *hotplug_error = NULL;
     int rc = ERROR_FAIL;
 
@@ -358,7 +303,8 @@ static void netbuf_setup_script_cb(libxl__egc *egc,
 
     STATE_AO_GC(remus_state->dss->ao);
 
-    libxl__ev_time_deregister(gc, &remus_state->timeout);
+    if (status)
+        goto out;
 
     out_path_base = GCSPRINTF("%s/remus/netbuf/%d",
                               libxl__xs_libxl_path(gc, domid), devid);
@@ -377,14 +323,6 @@ static void netbuf_setup_script_cb(libxl__egc *egc,
         goto out;
     }
 
-    if (status) {
-        libxl_report_child_exitstatus(CTX, LIBXL__LOG_ERROR,
-                                      remus_state->netbufscript,
-                                      pid, status);
-        rc = ERROR_FAIL;
-        goto out;
-    }
-
     rc = libxl__xs_read_checked(gc, XBT_NULL,
                                 GCSPRINTF("%s/remus/netbuf/%d/ifb",
                                           libxl__xs_libxl_path(gc, domid),
@@ -403,9 +341,8 @@ static void netbuf_setup_script_cb(libxl__egc *egc,
     LOG(DEBUG, "%s will buffer packets from vif %s", *ifb, vif);
     remus_state->dev_id++;
     if (remus_state->dev_id < netbuf_state->num_netbufs) {
-        rc = exec_netbuf_script(gc, remus_state,
-                                "setup", netbuf_setup_script_cb);
-        if (rc)
+        setup_env(&netbuf_state->async_exec, "setup", remus_state);
+        if (libxl_async_exec_script(gc, &netbuf_state->async_exec))
             goto out;
 
         return;
@@ -413,7 +350,7 @@ static void netbuf_setup_script_cb(libxl__egc *egc,
 
     rc = init_qdiscs(gc, remus_state);
  out:
-    libxl__remus_netbuf_setup_done(egc, remus_state->dss, rc);
+    libxl__remus_netbuf_setup_done(remus_state->egc, remus_state->dss, rc);
 }
 
 /* Scan through the list of vifs belonging to domid and
@@ -444,12 +381,19 @@ void libxl__remus_netbuf_setup(libxl__egc *egc,
 
     if (num_netbufs < 0) goto out;
 
+    libxl__ev_child_init(&netbuf_state->async_exec.child);
+
     GCNEW_ARRAY(netbuf_state->ifb_list, num_netbufs);
     netbuf_state->num_netbufs = num_netbufs;
     remus_state->netbuf_state = netbuf_state;
     remus_state->dev_id = 0;
-    if (exec_netbuf_script(gc, remus_state, "setup",
-                           netbuf_setup_script_cb))
+
+    netbuf_state->async_exec.timeout = LIBXL_HOTPLUG_TIMEOUT;
+    netbuf_state->async_exec.opaque = remus_state;
+    netbuf_state->async_exec.finish_cb = netbuf_setup_script_cb;
+    netbuf_state->async_exec.ao = ao;
+    setup_env(&netbuf_state->async_exec, "setup", remus_state);
+    if (libxl_async_exec_script(gc, &netbuf_state->async_exec))
         goto out;
     return;
 
@@ -457,35 +401,25 @@ void libxl__remus_netbuf_setup(libxl__egc *egc,
     libxl__remus_netbuf_setup_done(egc, dss, rc);
 }
 
-static void netbuf_teardown_script_cb(libxl__egc *egc,
-                                      libxl__ev_child *child,
-                                      pid_t pid, int status)
+static void netbuf_teardown_script_cb(void *opaque, int status)
 {
-    libxl__remus_state *remus_state = CONTAINER_OF(child, *remus_state, child);
+    libxl__remus_state *remus_state = opaque;
 
     /* Convenience aliases */
     libxl__remus_netbuf_state *const netbuf_state = remus_state->netbuf_state;
 
     STATE_AO_GC(remus_state->dss->ao);
 
-    libxl__ev_time_deregister(gc, &remus_state->timeout);
-
-    if (status) {
-        libxl_report_child_exitstatus(CTX, LIBXL__LOG_ERROR,
-                                      remus_state->netbufscript,
-                                      pid, status);
-    }
-
     remus_state->dev_id++;
     if (remus_state->dev_id < netbuf_state->num_netbufs) {
-        if (exec_netbuf_script(gc, remus_state,
-                               "teardown", netbuf_teardown_script_cb))
+        setup_env(&netbuf_state->async_exec, "teardown", remus_state);
+        if (libxl_async_exec_script(gc, &netbuf_state->async_exec))
             goto out;
         return;
     }
 
  out:
-    libxl__remus_netbuf_teardown_done(egc, remus_state->dss);
+    libxl__remus_netbuf_teardown_done(remus_state->egc, remus_state->dss);
 }
 
 /* Note: This function will be called in the same gc context as
@@ -501,11 +435,18 @@ void libxl__remus_netbuf_teardown(libxl__egc *egc,
 
     STATE_AO_GC(dss->ao);
 
+    libxl__ev_child_init(&netbuf_state->async_exec.child);
+
     free_qdiscs(netbuf_state);
 
+    netbuf_state->async_exec.timeout = LIBXL_HOTPLUG_TIMEOUT;
+    netbuf_state->async_exec.opaque = remus_state;
+    netbuf_state->async_exec.finish_cb = netbuf_teardown_script_cb;
+    netbuf_state->async_exec.ao = ao;
     remus_state->dev_id = 0;
-    if (exec_netbuf_script(gc, remus_state, "teardown",
-                           netbuf_teardown_script_cb))
+    setup_env(&netbuf_state->async_exec, "teardown", remus_state);
+
+    if (libxl_async_exec_script(gc, &netbuf_state->async_exec))
         libxl__remus_netbuf_teardown_done(egc, dss);
 }
 
-- 
1.7.4.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel