|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH 23/24] libxl: child processes cleanups
Abolish libxl_fork. Its only callers were in xl. Its functionality
is now moved elsewhere, as follows:
* The "logging version of fork", which is what it was billed as, is now
xl_fork, which also dies on failure.
* Closing the xenstore handle in the child is now done in
libxl__ev_child_fork, which is the only remaining place where fork
is called in libxl.
* We provide a new function libxl__ev_child_xenstore_reopen for
in-libxl children to make the ctx useable for xenstore again.
* Consequently libxl__spawn_record_pid now no longer needs to mess
about with its own xenstore handle. As a bonus it can now just use
libxl__xs_write.
Also, since we have now converted all the forkers in libxl, we can
always honour the fork_replacement childproc hook - so do so.
Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
---
tools/libxl/libxl_exec.c | 35 ++++++++++++++++-------------------
tools/libxl/libxl_fork.c | 25 ++++++++++++++++++++++++-
tools/libxl/libxl_internal.h | 5 +++++
tools/libxl/libxl_utils.c | 21 ---------------------
tools/libxl/libxl_utils.h | 3 +--
tools/libxl/xl.c | 12 ++++++++++++
tools/libxl/xl.h | 1 +
tools/libxl/xl_cmdimpl.c | 5 ++---
8 files changed, 61 insertions(+), 46 deletions(-)
diff --git a/tools/libxl/libxl_exec.c b/tools/libxl/libxl_exec.c
index d44b702..d681736 100644
--- a/tools/libxl/libxl_exec.c
+++ b/tools/libxl/libxl_exec.c
@@ -127,34 +127,23 @@ void libxl_report_child_exitstatus(libxl_ctx *ctx,
}
}
-int libxl__spawn_record_pid(libxl__gc *gc, libxl__spawn_state *spawn,
- pid_t innerchild)
+int libxl__spawn_record_pid(libxl__gc *gc, libxl__spawn_state *spawn, pid_t
pid)
{
- struct xs_handle *xsh = NULL;
- const char *pid = NULL;
- int rc, xsok;
+ int r, rc;
- pid = GCSPRINTF("%d", innerchild);
+ rc = libxl__ev_child_xenstore_reopen(gc, spawn->what);
+ if (rc) goto out;
- /* we mustn't use the parent's handle in the child */
- xsh = xs_daemon_open();
- if (!xsh) {
- LOGE(ERROR, "write %s = %s: xenstore reopen failed",
- spawn->pidpath, pid);
- rc = ERROR_FAIL; goto out;
- }
-
- xsok = xs_write(xsh, XBT_NULL, spawn->pidpath, pid, strlen(pid));
- if (!xsok) {
+ r = libxl__xs_write(gc, XBT_NULL, spawn->pidpath, "%d", pid);
+ if (r) {
LOGE(ERROR,
- "write %s = %s: xenstore write failed", spawn->pidpath, pid);
+ "write %s = %d: xenstore write failed", spawn->pidpath, pid);
rc = ERROR_FAIL; goto out;
}
rc = 0;
out:
- if (xsh) xs_daemon_close(xsh);
return rc ? SIGTERM : 0;
}
@@ -302,7 +291,15 @@ int libxl__spawn_spawn(libxl__egc *egc, libxl__spawn_state
*ss)
/* we are now the middle process */
- child = fork();
+ pid_t (*fork_replacement)(void*) =
+ CTX->childproc_hooks
+ ? CTX->childproc_hooks->fork_replacement
+ : 0;
+ child =
+ fork_replacement
+ ? fork_replacement(CTX->childproc_user)
+ : fork();
+
if (child == -1)
exit(255);
if (!child) {
diff --git a/tools/libxl/libxl_fork.c b/tools/libxl/libxl_fork.c
index 35c8bdd..9ff99e0 100644
--- a/tools/libxl/libxl_fork.c
+++ b/tools/libxl/libxl_fork.c
@@ -347,7 +347,12 @@ pid_t libxl__ev_child_fork(libxl__gc *gc, libxl__ev_child
*ch,
if (!pid) {
/* woohoo! */
- return 0; /* Yes, CTX is left locked in the child. */
+ if (CTX->xsh) {
+ xs_daemon_destroy_postfork(CTX->xsh);
+ CTX->xsh = NULL; /* turns mistakes into crashes */
+ }
+ /* Yes, CTX is left locked in the child. */
+ return 0;
}
ch->pid = pid;
@@ -395,6 +400,24 @@ const libxl_childproc_hooks libxl__childproc_default_hooks
= {
libxl_sigchld_owner_libxl, 0, 0
};
+int libxl__ev_child_xenstore_reopen(libxl__gc *gc, const char *what) {
+ int rc;
+
+ assert(!CTX->xsh);
+ CTX->xsh = xs_daemon_open();
+ if (!CTX->xsh) {
+ LOGE(ERROR, "%s: xenstore reopen failed", what);
+ rc = ERROR_FAIL; goto out;
+ }
+
+ libxl_fd_set_cloexec(CTX, xs_fileno(CTX->xsh), 1);
+
+ return 0;
+
+ out:
+ return rc;
+}
+
/*
* Local variables:
* mode: C
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index fb4dee8..3e90ac4 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -627,6 +627,11 @@ static inline void libxl__ev_child_init(libxl__ev_child
*childw_out)
static inline int libxl__ev_child_inuse(libxl__ev_child *childw_out)
{ return childw_out->pid >= 0; }
+/* Useable (only) in the child to once more make the ctx useable for
+ * xenstore operations. logs failure in the form "what: <error
+ * message>". */
+_hidden int libxl__ev_child_xenstore_reopen(libxl__gc *gc, const char *what);
+
/*
* Other event-handling support provided by the libxl event core to
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index f0d94c6..858410e 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -444,27 +444,6 @@ int libxl__remove_directory(libxl__gc *gc, const char
*dirpath)
return rc;
}
-pid_t libxl_fork(libxl_ctx *ctx)
-{
- pid_t pid;
-
- pid = fork();
- if (pid == -1) {
- LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "fork failed");
- 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;
-}
-
int libxl_pipe(libxl_ctx *ctx, int pipes[2])
{
if (pipe(pipes) < 0) {
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index 2b47622..74beb52 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -47,9 +47,8 @@ int libxl_write_exactly(libxl_ctx *ctx, int fd, const void
*data,
* logged using filename (which is only used for logging) and what
* (which may be 0). */
-pid_t libxl_fork(libxl_ctx *ctx);
int libxl_pipe(libxl_ctx *ctx, int pipes[2]);
- /* Just like fork(2), pipe(2), but log errors. */
+ /* Just like pipe(2), but log errors. */
void libxl_report_child_exitstatus(libxl_ctx *ctx, xentoollog_level,
const char *what, pid_t pid, int status);
diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c
index a6ffd25..d4db1f8 100644
--- a/tools/libxl/xl.c
+++ b/tools/libxl/xl.c
@@ -105,6 +105,18 @@ void postfork(void)
}
}
+pid_t xl_fork(libxl_ctx *ctx) {
+ pid_t pid;
+
+ pid = fork();
+ if (pid == -1) {
+ perror("fork failed");
+ exit(-1);
+ }
+
+ return pid;
+}
+
int main(int argc, char **argv)
{
int opt = 0;
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index 7e258d5..c031bb3 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -105,6 +105,7 @@ struct cmd_spec *cmdtable_lookup(const char *s);
extern libxl_ctx *ctx;
extern xentoollog_logger_stdiostream *logger;
+pid_t xl_fork(libxl_ctx *ctx); /* like fork, but prints and dies if it fails */
void postfork(void);
/* global options */
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index ce52599..ebfd4fe 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1709,7 +1709,7 @@ start:
pid_t child1, got_child;
int nullfd;
- child1 = libxl_fork(ctx);
+ child1 = xl_fork(ctx);
if (child1) {
printf("Daemon running with PID %d\n", child1);
@@ -2748,8 +2748,7 @@ static void migrate_domain(const char *domain_spec, const
char *rune,
MUST( libxl_pipe(ctx, sendpipe) );
MUST( libxl_pipe(ctx, recvpipe) );
- child = libxl_fork(ctx);
- if (child==-1) exit(1);
+ child = xl_fork(ctx);
if (!child) {
dup2(sendpipe[0], 0);
--
1.7.2.5
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |