[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-changelog] [xen-unstable] xenstore, libxl: cleanup of xenstore connections across fork()



# HG changeset patch
# User Keir Fraser <keir.fraser@xxxxxxxxxx>
# Date 1271090518 -3600
# Node ID d005aa895b5a051e778fc57d20107e617a8c0b41
# Parent  86e82ab8d4de0f48248f5028c246d72b03526245
xenstore,libxl: cleanup of xenstore connections across fork()

Provide a new function xs_daemon_destroy_postfork which can be called
by a libxenstore user who has called fork, to close the fd for the
connection to xenstored and free the memory, without trying to do
anything to any threads which libxenstore may have created.

Use this new function in libxl_fork, to avoid accidental use of a
xenstore connection in both parent and child.

Also, fix the doc comment for libxl_spawn_spawn to have the success
return codes the right way round.

Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
---
 tools/libxl/libxl.c          |    2 -
 tools/libxl/libxl.h          |    1 
 tools/libxl/libxl_internal.h |    4 +--
 tools/libxl/libxl_utils.c    |   15 ++++++++++++
 tools/xenstore/xs.c          |   52 +++++++++++++++++++++++++++----------------
 tools/xenstore/xs.h          |    3 ++
 6 files changed, 55 insertions(+), 22 deletions(-)

diff -r 86e82ab8d4de -r d005aa895b5a tools/libxl/libxl.c
--- a/tools/libxl/libxl.c       Mon Apr 12 17:41:05 2010 +0100
+++ b/tools/libxl/libxl.c       Mon Apr 12 17:41:58 2010 +0100
@@ -66,7 +66,7 @@ int libxl_ctx_free(struct libxl_ctx *ctx
     libxl_free_all(ctx);
     free(ctx->alloc_ptrs);
     xc_interface_close(ctx->xch);
-    xs_daemon_close(ctx->xsh); 
+    if (ctx->xsh) xs_daemon_close(ctx->xsh); 
     return 0;
 }
 
diff -r 86e82ab8d4de -r d005aa895b5a tools/libxl/libxl.h
--- a/tools/libxl/libxl.h       Mon Apr 12 17:41:05 2010 +0100
+++ b/tools/libxl/libxl.h       Mon Apr 12 17:41:58 2010 +0100
@@ -250,6 +250,7 @@ int libxl_ctx_init(struct libxl_ctx *ctx
 int libxl_ctx_init(struct libxl_ctx *ctx, int version);
 int libxl_ctx_free(struct libxl_ctx *ctx);
 int libxl_ctx_set_log(struct libxl_ctx *ctx, libxl_log_callback log_callback, 
void *log_data);
+int libxl_ctx_postfork(struct libxl_ctx *ctx);
 
 /* domain related functions */
 int libxl_domain_make(struct libxl_ctx *ctx, libxl_domain_create_info *info, 
uint32_t *domid);
diff -r 86e82ab8d4de -r d005aa895b5a tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h      Mon Apr 12 17:41:05 2010 +0100
+++ b/tools/libxl/libxl_internal.h      Mon Apr 12 17:41:58 2010 +0100
@@ -185,8 +185,8 @@ int libxl_spawn_spawn(struct libxl_ctx *
                       void (*intermediate_hook)(void *for_spawn, pid_t 
innerchild));
   /* Logs errors.  A copy of "what" is taken.  Return values:
    *  < 0   error, for_spawn need not be detached
-   *   +1   caller is now the inner child, should probably call libxl_exec
-   *    0   caller is the parent, must call detach on *for_spawn eventually
+   *   +1   caller is the parent, must call detach on *for_spawn eventually
+   *    0   caller is now the inner child, should probably call libxl_exec
    * Caller, may pass 0 for for_spawn, in which case no need to detach.
    */
 int libxl_spawn_detach(struct libxl_ctx *ctx,
diff -r 86e82ab8d4de -r d005aa895b5a tools/libxl/libxl_utils.c
--- a/tools/libxl/libxl_utils.c Mon Apr 12 17:41:05 2010 +0100
+++ b/tools/libxl/libxl_utils.c Mon Apr 12 17:41:58 2010 +0100
@@ -282,6 +282,13 @@ READ_WRITE_EXACTLY(write, 0, const)
 READ_WRITE_EXACTLY(write, 0, const)
 
 
+int libxl_ctx_postfork(struct libxl_ctx *ctx) {
+    if (ctx->xsh) xs_daemon_destroy_postfork(ctx->xsh);
+    ctx->xsh = xs_daemon_open();
+    if (!ctx->xsh) return ERROR_FAIL;
+    return 0;
+}
+
 pid_t libxl_fork(struct libxl_ctx *ctx)
 {
     pid_t pid;
@@ -292,6 +299,14 @@ pid_t libxl_fork(struct libxl_ctx *ctx)
         return -1;
     }
 
+    if (!pid) {
+        if (ctx->xsh) xs_daemon_destroy_postfork(ctx->xsh);
+        ctx->xsh = 0;
+        /* This ensures that anyone who forks but doesn't exec,
+         * and doesn't reinitialise the libxl_ctx, is OK.
+         * It also means they can safely call libxl_ctx_free. */
+    }
+
     return pid;
 }
 
diff -r 86e82ab8d4de -r d005aa895b5a tools/xenstore/xs.c
--- a/tools/xenstore/xs.c       Mon Apr 12 17:41:05 2010 +0100
+++ b/tools/xenstore/xs.c       Mon Apr 12 17:41:58 2010 +0100
@@ -223,10 +223,39 @@ struct xs_handle *xs_domain_open(void)
        return get_handle(xs_domain_dev());
 }
 
+static void close_free_msgs(struct xs_handle *h) {
+       struct xs_stored_msg *msg, *tmsg;
+
+       list_for_each_entry_safe(msg, tmsg, &h->reply_list, list) {
+               free(msg->body);
+               free(msg);
+       }
+
+       list_for_each_entry_safe(msg, tmsg, &h->watch_list, list) {
+               free(msg->body);
+               free(msg);
+       }
+}
+
+static void close_fds_free(struct xs_handle *h) {
+       if (h->watch_pipe[0] != -1) {
+               close(h->watch_pipe[0]);
+               close(h->watch_pipe[1]);
+       }
+
+        close(h->fd);
+        
+       free(h);
+}
+
+void xs_daemon_destroy_postfork(struct xs_handle *h)
+{
+        close_free_msgs(h);
+        close_fds_free(h);
+}
+
 void xs_daemon_close(struct xs_handle *h)
 {
-       struct xs_stored_msg *msg, *tmsg;
-
        mutex_lock(&h->request_mutex);
        mutex_lock(&h->reply_mutex);
        mutex_lock(&h->watch_mutex);
@@ -239,28 +268,13 @@ void xs_daemon_close(struct xs_handle *h
        }
 #endif
 
-       list_for_each_entry_safe(msg, tmsg, &h->reply_list, list) {
-               free(msg->body);
-               free(msg);
-       }
-
-       list_for_each_entry_safe(msg, tmsg, &h->watch_list, list) {
-               free(msg->body);
-               free(msg);
-       }
+        close_free_msgs(h);
 
        mutex_unlock(&h->request_mutex);
        mutex_unlock(&h->reply_mutex);
        mutex_unlock(&h->watch_mutex);
 
-       if (h->watch_pipe[0] != -1) {
-               close(h->watch_pipe[0]);
-               close(h->watch_pipe[1]);
-       }
-
-       close(h->fd);
-
-       free(h);
+        close_fds_free(h);
 }
 
 static bool read_all(int fd, void *data, unsigned int len)
diff -r 86e82ab8d4de -r d005aa895b5a tools/xenstore/xs.h
--- a/tools/xenstore/xs.h       Mon Apr 12 17:41:05 2010 +0100
+++ b/tools/xenstore/xs.h       Mon Apr 12 17:41:58 2010 +0100
@@ -47,6 +47,9 @@ struct xs_handle *xs_daemon_open_readonl
 
 /* Close the connection to the xs daemon. */
 void xs_daemon_close(struct xs_handle *);
+
+/* Throw away the connection to the xs daemon, for use after fork(). */
+void xs_daemon_destroy_postfork(struct xs_handle *);
 
 /* Get contents of a directory.
  * Returns a malloced array: call free() on it after use.

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.