|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH] xl: track child processes for the benefit of libxl
Each time xl forks, it needs to record the pid, so that its exit
status can be preserved if it happens that libxl's event loop reaped
it. Consequently we also have a new wrapper for waitpid, which in
that case returns the previously-reaped status.
When we get a console ready callback from libxl, check to see if we
have spawned but not reaped a previous console client, and if so reap
it now. (This is necessary to prevent improper use of the xlchild
struct, but has the happy consequence of checking the exit status of
the first console client in the pygrub case.)
Refactor the two calls to libxl_ctx_alloc into a new function
xl_ctx_alloc which also sets the child reaped handler callback.
All of this has the effect of suppressing a message
unknown child [nnnn] unexpected exited status zero
which would sometimes (depending on a race) appear with `xl create -c'
and pygrub.
Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Reported-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
Cc: Roger Pau Monne <roger.pau@xxxxxxxxxx>
---
tools/libxl/xl.c | 80 ++++++++++++++++++++++++++++++++++++++-------
tools/libxl/xl.h | 29 +++++++++++++++-
tools/libxl/xl_cmdimpl.c | 80 ++++++++++++++++++++++++++-------------------
3 files changed, 140 insertions(+), 49 deletions(-)
diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c
index 750a61c..66499dd 100644
--- a/tools/libxl/xl.c
+++ b/tools/libxl/xl.c
@@ -102,22 +102,79 @@ void postfork(void)
libxl_postfork_child_noexec(ctx); /* in case we don't exit/exec */
ctx = 0;
- if (libxl_ctx_alloc(&ctx, LIBXL_VERSION, 0, (xentoollog_logger*)logger)) {
- fprintf(stderr, "cannot reinit xl context after fork\n");
- exit(-1);
- }
+ xl_ctx_alloc();
}
-pid_t xl_fork(libxl_ctx *ctx) {
- pid_t pid;
+pid_t xl_fork(xlchild *ch) {
+ assert(!ch->pid);
+ ch->reaped = 0;
- pid = fork();
- if (pid == -1) {
+ ch->pid = fork();
+ if (ch->pid == -1) {
perror("fork failed");
exit(-1);
}
- return pid;
+ if (!ch->pid) {
+#define CHILD_FORGET(other) \
+ other.pid = 0;
+CHILD_LIST(CHILD_FORGET)
+ }
+
+ return ch->pid;
+}
+
+pid_t xl_waitpid(xlchild *child, int *status, int flags)
+{
+ pid_t got = child->pid;
+ assert(got);
+ if (child->reaped) {
+ *status = child->status;
+ child->pid = 0;
+ return got;
+ }
+ for (;;) {
+ got = waitpid(child->pid, status, flags);
+ if (got < 0 && errno == EINTR) continue;
+ if (got > 0) {
+ assert(got == child->pid);
+ child->pid = 0;
+ }
+ return got;
+ }
+}
+
+static int reaped_callback_child_check(xlchild *ch, pid_t got, int status)
+{
+ if (ch->pid != got)
+ return 0;
+ ch->reaped = 1;
+ ch->status = status;
+ return 1;
+}
+
+static int xl_reaped_callback(pid_t got, int status, void *user)
+{
+ assert(got);
+ return (
+#define CHILD_CHECK(ch) \
+ reaped_callback_child_check(&ch, got, status) ||
+CHILD_LIST(CHILD_CHECK)
+ 0) ? 0 : ERROR_UNKNOWN_CHILD;
+}
+
+static const libxl_childproc_hooks childproc_hooks = {
+ .chldowner = libxl_sigchld_owner_libxl,
+ .reaped_callback = xl_reaped_callback,
+};
+
+void xl_ctx_alloc(void) {
+ if (libxl_ctx_alloc(&ctx, LIBXL_VERSION, 0, (xentoollog_logger*)logger)) {
+ fprintf(stderr, "cannot init xl context\n");
+ exit(1);
+ }
+
+ libxl_childproc_setmode(ctx, &childproc_hooks, 0);
}
int main(int argc, char **argv)
@@ -159,10 +216,7 @@ int main(int argc, char **argv)
logger = xtl_createlogger_stdiostream(stderr, minmsglevel, 0);
if (!logger) exit(1);
- if (libxl_ctx_alloc(&ctx, LIBXL_VERSION, 0, (xentoollog_logger*)logger)) {
- fprintf(stderr, "cannot init xl context\n");
- exit(1);
- }
+ xl_ctx_alloc();
/* Read global config file options */
ret = asprintf(&config_file, "%s/xl.conf", libxl_xen_config_dir_path());
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index 5d0d504..a00309c 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -15,6 +15,8 @@
#ifndef XL_H
#define XL_H
+#include <assert.h>
+
#include "xentoollog.h"
struct cmd_spec {
@@ -105,8 +107,31 @@ 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);
+
+void xl_ctx_alloc(void);
+
+/* child processes */
+
+typedef struct {
+ /* every struct like this must be in XLCHILD_LIST */
+ pid_t pid; /* 0: not in use */
+ int reaped;
+ int status; /* valid iff pid==-1 */
+} xlchild;
+
+/* our child processes */
+#define CHILD_LIST(_) \
+ _(child_console) \
+ _(child_waitdaemon) \
+ _(migration_child)
+
+#define CHILD_DECLARE(ch) \
+extern xlchild ch;
+CHILD_LIST(CHILD_DECLARE);
+
+pid_t xl_fork(xlchild*); /* like fork, but prints and dies if it fails */
+void postfork(void); /* needed only if we aren't going to exec right away */
+pid_t xl_waitpid(xlchild*, int *status, int flags); /* handles EINTR */
/* global options */
extern int autoballoon;
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 9f182c2..de1f2d8 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -17,7 +17,6 @@
#include "libxl_osdeps.h"
#include <stdio.h>
-#include <assert.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
@@ -66,6 +65,13 @@ int logfile = 2;
/* every libxl action in xl uses this same libxl context */
libxl_ctx *ctx;
+#define CHILD_DEFINE(ch) \
+xlchild ch;
+CHILD_LIST(CHILD_DEFINE);
+
+xlchild child_console;
+xlchild child_waitdaemon;
+
/* when we operate on a domain, it is this one: */
static uint32_t domid;
static const char *common_domname;
@@ -1457,19 +1463,33 @@ static int freemem(libxl_domain_build_info *b_info)
return ERROR_NOMEM;
}
+static void console_child_report(void)
+{
+ if (child_console.pid) {
+ int status;
+ pid_t got = xl_waitpid(&child_console, &status, 0);
+ if (got < 0)
+ perror("xl: warning, failed to waitpid for console child");
+ else if (status)
+ libxl_report_child_exitstatus(ctx, XTL_ERROR, "console child",
+ child_console.pid, status);
+ }
+}
+
static void autoconnect_console(libxl_ctx *ctx_ignored,
libxl_event *ev, void *priv)
{
- pid_t *pid = priv;
uint32_t bldomid = ev->domid;
libxl_event_free(ctx, ev);
- *pid = fork();
- if (*pid < 0) {
+ console_child_report();
+
+ pid_t pid = xl_fork(&child_console);
+ if (pid < 0) {
perror("unable to fork xenconsole");
return;
- } else if (*pid > 0)
+ } else if (pid > 0)
return;
postfork();
@@ -1538,7 +1558,6 @@ static int create_domain(struct domain_create *dom_info)
int restore_fd = -1;
int status = 0;
const libxl_asyncprogress_how *autoconnect_console_how;
- pid_t child_console_pid = -1;
struct save_file_header hdr;
libxl_domain_config_init(&d_config);
@@ -1694,7 +1713,6 @@ start:
libxl_asyncprogress_how autoconnect_console_how_buf;
if ( dom_info->console_autoconnect ) {
autoconnect_console_how_buf.callback = autoconnect_console;
- autoconnect_console_how_buf.for_callback = &child_console_pid;
autoconnect_console_how = &autoconnect_console_how_buf;
}else{
autoconnect_console_how = 0;
@@ -1738,19 +1756,17 @@ start:
pid_t child1, got_child;
int nullfd;
- child1 = xl_fork(ctx);
+ child1 = xl_fork(&child_waitdaemon);
if (child1) {
printf("Daemon running with PID %d\n", child1);
for (;;) {
- got_child = waitpid(child1, &status, 0);
+ got_child = xl_waitpid(&child_waitdaemon, &status, 0);
if (got_child == child1) break;
assert(got_child == -1);
- if (errno != EINTR) {
- perror("failed to wait for daemonizing child");
- ret = ERROR_FAIL;
- goto out;
- }
+ perror("failed to wait for daemonizing child");
+ ret = ERROR_FAIL;
+ goto out;
}
if (status) {
libxl_report_child_exitstatus(ctx, XTL_ERROR,
@@ -1911,10 +1927,7 @@ out:
free(config_data);
-waitpid_out:
- if (child_console_pid > 0 &&
- waitpid(child_console_pid, &status, 0) < 0 && errno == EINTR)
- goto waitpid_out;
+ console_child_report();
if (deathw)
libxl_evdisable_domain_death(ctx, deathw);
@@ -2692,31 +2705,30 @@ static int migrate_read_fixedmessage(int fd, const void
*msg, int msgsz,
return 0;
}
-static void migration_child_report(pid_t migration_child, int recv_fd) {
- pid_t child;
+static void migration_child_report(int recv_fd) {
+ pid_t child = migration_child.pid;
int status, sr;
struct timeval now, waituntil, timeout;
static const struct timeval pollinterval = { 0, 1000 }; /* 1ms */
- if (!migration_child) return;
+ if (!child) return;
CHK_ERRNO( gettimeofday(&waituntil, 0) );
waituntil.tv_sec += 2;
for (;;) {
- child = waitpid(migration_child, &status, WNOHANG);
+ child = xl_waitpid(&migration_child, &status, WNOHANG);
- if (child == migration_child) {
+ if (child == migration_child.pid) {
if (status)
libxl_report_child_exitstatus(ctx, XTL_INFO,
"migration target process",
- migration_child, status);
+ child, status);
break;
}
if (child == -1) {
- if (errno == EINTR) continue;
fprintf(stderr, "wait for migration child [%ld] failed: %s\n",
- (long)migration_child, strerror(errno));
+ (long)migration_child.pid, strerror(errno));
break;
}
assert(child == 0);
@@ -2725,7 +2737,7 @@ static void migration_child_report(pid_t migration_child,
int recv_fd) {
if (timercmp(&now, &waituntil, >)) {
fprintf(stderr, "migration child [%ld] not exiting, no longer"
" waiting (exit status will be unreported)\n",
- (long)migration_child);
+ (long)migration_child.pid);
break;
}
timersub(&waituntil, &now, &timeout);
@@ -2749,12 +2761,12 @@ static void migration_child_report(pid_t
migration_child, int recv_fd) {
if (errno != EINTR) {
fprintf(stderr, "migration child [%ld] exit wait select"
" failed unexpectedly: %s\n",
- (long)migration_child, strerror(errno));
+ (long)migration_child.pid, strerror(errno));
break;
}
}
}
- migration_child = 0;
+ migration_child.pid = 0;
}
static void migrate_domain(const char *domain_spec, const char *rune,
@@ -2782,7 +2794,7 @@ static void migrate_domain(const char *domain_spec, const
char *rune,
MUST( libxl_pipe(ctx, sendpipe) );
MUST( libxl_pipe(ctx, recvpipe) );
- child = xl_fork(ctx);
+ child = xl_fork(&migration_child);
if (!child) {
dup2(sendpipe[0], 0);
@@ -2808,7 +2820,7 @@ static void migrate_domain(const char *domain_spec, const
char *rune,
"banner", rune);
if (rc) {
close(send_fd);
- migration_child_report(child, recv_fd);
+ migration_child_report(recv_fd);
exit(-rc);
}
@@ -2905,13 +2917,13 @@ static void migrate_domain(const char *domain_spec,
const char *rune,
failed_suspend:
close(send_fd);
- migration_child_report(child, recv_fd);
+ migration_child_report(recv_fd);
fprintf(stderr, "Migration failed, failed to suspend at sender.\n");
exit(-ERROR_FAIL);
failed_resume:
close(send_fd);
- migration_child_report(child, recv_fd);
+ migration_child_report(recv_fd);
fprintf(stderr, "Migration failed, resuming at sender.\n");
libxl_domain_resume(ctx, domid);
exit(-ERROR_FAIL);
@@ -2926,7 +2938,7 @@ static void migrate_domain(const char *domain_spec, const
char *rune,
" responsibility to avoid that. Sorry.\n");
close(send_fd);
- migration_child_report(child, recv_fd);
+ migration_child_report(recv_fd);
exit(-ERROR_BADFAIL);
}
--
tg: (95117b6..) t/xen/xl.event.fix.xlchildren (depends on:
t/xen/xl.event.fix.console)
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |