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

[Xen-devel] [PATCH 11/24] libxl: ao: Convert libxl_run_bootloader



Convert libxl_run_bootloader to an ao_how-taking function.

It's implemented in terms of libxl__bootloader_run, which can be used
internally.  The resulting code is pretty much a rewrite.

Significant changes include:

- We direct the bootloader's results to a file, not a pipe.  This
  makes it simpler to deal with as we don't have to read it
  concurrently along with everything else.

- We now issue a warning if we find unexpected statements in the
  bootloader results.

- The arrangements for buffering of bootloader input and output
  are completely changed.  Now we have a fixed limit of 64k
  on output, and 4k on input, and discard the oldest data when
  this overflows (which it shouldn't).  There is no timeout
  any more.

Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

Changes since v6:
 * Use libxl__ev_child_inuse rather than testing pid directly.
 * Fix a code style error.
 * Properly initialise the sub-operations' aos in _init.
 * Bugfixes.
---
 tools/libxl/libxl.c            |    4 +
 tools/libxl/libxl.h            |    3 +-
 tools/libxl/libxl_bootloader.c |  713 +++++++++++++++++++++-------------------
 tools/libxl/libxl_create.c     |    8 +-
 tools/libxl/libxl_internal.h   |   34 ++
 5 files changed, 419 insertions(+), 343 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 42ac89f..54f3813 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1748,6 +1748,10 @@ int libxl_device_disk_local_detach(libxl_ctx *ctx, 
libxl_device_disk *disk)
      * For other device types assume that the blktap2 process is
      * needed by the soon to be started domain and do nothing.
      */
+    /*
+     * FIXME
+     * This appears to leak the disk in failure paths
+     */
 
     return 0;
 }
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 03e71f6..477b72a 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -494,7 +494,8 @@ int libxl_get_max_cpus(libxl_ctx *ctx);
 int libxl_run_bootloader(libxl_ctx *ctx,
                          libxl_domain_build_info *info,
                          libxl_device_disk *disk,
-                         uint32_t domid);
+                         uint32_t domid,
+                         libxl_asyncop_how *ao_how);
 
   /* 0 means ERROR_ENOMEM, which we have logged */
 
diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c
index b50944a..bdc4cb4 100644
--- a/tools/libxl/libxl_bootloader.c
+++ b/tools/libxl/libxl_bootloader.c
@@ -15,6 +15,7 @@
 #include "libxl_osdeps.h" /* must come before any other headers */
 
 #include <termios.h>
+#include <utmp.h>
 
 #ifdef INCLUDE_LIBUTIL_H
 #include INCLUDE_LIBUTIL_H
@@ -22,67 +23,80 @@
 
 #include "libxl_internal.h"
 
-#define XENCONSOLED_BUF_SIZE 16
-#define BOOTLOADER_BUF_SIZE 4096
-#define BOOTLOADER_TIMEOUT 1
+#define BOOTLOADER_BUF_OUT 65536
+#define BOOTLOADER_BUF_IN   4096
 
-static char **make_bootloader_args(libxl__gc *gc,
-                                   libxl_domain_build_info *info,
-                                   uint32_t domid,
-                                   const char *fifo, char *disk)
+static void bootloader_gotptys(libxl__egc *egc, libxl__openpty_state *op);
+static void bootloader_keystrokes_copyfail(libxl__egc *egc,
+       libxl__datacopier_state *dc, int onwrite, int errnoval);
+static void bootloader_display_copyfail(libxl__egc *egc,
+       libxl__datacopier_state *dc, int onwrite, int errnoval);
+static void bootloader_finished(libxl__egc *egc, libxl__ev_child *child,
+                                pid_t pid, int status);
+
+/*----- bootloader arguments -----*/
+
+static void bootloader_arg(libxl__bootloader_state *bl, const char *arg)
+{
+    assert(bl->nargs < bl->argsspace);
+    bl->args[bl->nargs++] = arg;
+}
+
+static void make_bootloader_args(libxl__gc *gc, libxl__bootloader_state *bl)
 {
-    flexarray_t *args;
-    int nr = 0;
+    const libxl_domain_build_info *info = bl->info;
 
-    args = flexarray_make(1, 1);
-    if (!args)
-        return NULL;
+    bl->argsspace = 7 + libxl_string_list_length(&info->u.pv.bootloader_args);
 
-    flexarray_set(args, nr++, (char *)info->u.pv.bootloader);
+    GCNEW_ARRAY(bl->args, bl->argsspace);
+
+#define ARG(arg) bootloader_arg(bl, (arg))
+
+    ARG(info->u.pv.bootloader);
 
     if (info->u.pv.kernel.path)
-        flexarray_set(args, nr++, libxl__sprintf(gc, "--kernel=%s",
-                                                 info->u.pv.kernel.path));
+        ARG(libxl__sprintf(gc, "--kernel=%s", info->u.pv.kernel.path));
     if (info->u.pv.ramdisk.path)
-        flexarray_set(args, nr++, libxl__sprintf(gc, "--ramdisk=%s", 
info->u.pv.ramdisk.path));
+        ARG(libxl__sprintf(gc, "--ramdisk=%s", info->u.pv.ramdisk.path));
     if (info->u.pv.cmdline && *info->u.pv.cmdline != '\0')
-        flexarray_set(args, nr++, libxl__sprintf(gc, "--args=%s", 
info->u.pv.cmdline));
+        ARG(libxl__sprintf(gc, "--args=%s", info->u.pv.cmdline));
 
-    flexarray_set(args, nr++, libxl__sprintf(gc, "--output=%s", fifo));
-    flexarray_set(args, nr++, "--output-format=simple0");
-    flexarray_set(args, nr++, libxl__sprintf(gc, "--output-directory=%s", 
"/var/run/libxl/"));
+    ARG(libxl__sprintf(gc, "--output=%s", bl->outputpath));
+    ARG("--output-format=simple0");
+    ARG(libxl__sprintf(gc, "--output-directory=%s", bl->outputdir));
 
     if (info->u.pv.bootloader_args) {
         char **p = info->u.pv.bootloader_args;
         while (*p) {
-            flexarray_set(args, nr++, *p);
+            ARG(*p);
             p++;
         }
     }
 
-    flexarray_set(args, nr++, disk);
+    ARG(bl->diskpath);
 
-    /* Sentinal for execv */
-    flexarray_set(args, nr++, NULL);
+    /* Sentinel for execv */
+    ARG(NULL);
 
-    return (char **) flexarray_contents(args); /* Frees args */
+#undef ARG
 }
 
-static int open_xenconsoled_pty(int *master, int *slave, char *slave_path, 
size_t slave_path_len)
+/*----- synchronous subroutines -----*/
+
+static int setup_xenconsoled_pty(libxl__egc *egc, libxl__bootloader_state *bl,
+                                 char *slave_path, size_t slave_path_len)
 {
+    STATE_AO_GC(bl->ao);
     struct termios termattr;
-    int ret;
-
-    ret = openpty(master, slave, NULL, NULL, NULL);
-    if (ret < 0)
-        return -1;
-
-    ret = ttyname_r(*slave, slave_path, slave_path_len);
-    if (ret == -1) {
-        close(*master);
-        close(*slave);
-        *master = *slave = -1;
-        return -1;
+    int r, rc;
+    int slave = libxl__carefd_fd(bl->ptys[1].slave);
+    int master = libxl__carefd_fd(bl->ptys[1].master);
+
+    r = ttyname_r(slave, slave_path, slave_path_len);
+    if (r == -1) {
+        LOGE(ERROR,"ttyname_r failed");
+        rc = ERROR_FAIL;
+        goto out;
     }
 
     /*
@@ -95,310 +109,217 @@ static int open_xenconsoled_pty(int *master, int *slave, 
char *slave_path, size_
      * semantics on Solaris, so don't try to set any attributes
      * for it.
      */
-#if !defined(__sun__) && !defined(__NetBSD__)
-    tcgetattr(*master, &termattr);
+    tcgetattr(master, &termattr);
     cfmakeraw(&termattr);
-    tcsetattr(*master, TCSANOW, &termattr);
-
-    close(*slave);
-    *slave = -1;
-#else
-    tcgetattr(*slave, &termattr);
-    cfmakeraw(&termattr);
-    tcsetattr(*slave, TCSANOW, &termattr);
-#endif
-
-    fcntl(*master, F_SETFL, O_NDELAY);
-    fcntl(*master, F_SETFD, FD_CLOEXEC);
+    tcsetattr(master, TCSANOW, &termattr);
 
     return 0;
+
+ out:
+    return rc;
 }
 
-static pid_t fork_exec_bootloader(int *master, const char *arg0, char **args)
-{
-    struct termios termattr;
-    pid_t pid = forkpty(master, NULL, NULL, NULL);
-    if (pid == -1)
-        return -1;
-    else if (pid == 0) {
-        setenv("TERM", "vt100", 1);
-        libxl__exec(-1, -1, -1, arg0, args);
-        return -1;
-    }
+static const char *bootloader_result_command(libxl__gc *gc, const char *buf,
+                         const char *prefix, size_t prefixlen) {
+    if (strncmp(buf, prefix, prefixlen))
+        return 0;
 
-    /*
-     * On Solaris, the master pty side does not have terminal semantics,
-     * so don't try to set any attributes, as it will fail.
-     */
-#if !defined(__sun__)
-    tcgetattr(*master, &termattr);
-    cfmakeraw(&termattr);
-    tcsetattr(*master, TCSANOW, &termattr);
-#endif
+    const char *rhs = buf + prefixlen;
+    if (!CTYPE(isspace,*rhs))
+        return 0;
 
-    fcntl(*master, F_SETFL, O_NDELAY);
+    while (CTYPE(isspace,*rhs))
+        rhs++;
 
-    return pid;
+    LOG(DEBUG,"bootloader output contained %s %s", prefix, rhs);
+
+    return rhs;
 }
 
-/*
- * filedescriptors:
- *   fifo_fd        - bootstring output from the bootloader
- *   xenconsoled_fd - input/output from/to xenconsole
- *   bootloader_fd  - input/output from/to pty that controls the bootloader
- * The filedescriptors are NDELAY, so it's ok to try to read
- * bigger chunks than may be available, to keep e.g. curses
- * screen redraws in the bootloader efficient. xenconsoled_fd is the side that
- * gets xenconsole input, which will be keystrokes, so a small number
- * is sufficient. bootloader_fd is pygrub output, which will be curses screen
- * updates, so a larger number (1024) is appropriate there.
- *
- * For writeable descriptors, only include them in the set for select
- * if there is actual data to write, otherwise this would loop too fast,
- * eating up CPU time.
- */
-static char * bootloader_interact(libxl__gc *gc, int xenconsoled_fd, int 
bootloader_fd, int fifo_fd)
+static int parse_bootloader_result(libxl__egc *egc,
+                                   libxl__bootloader_state *bl)
 {
-    int ret;
-
-    size_t nr_out = 0, size_out = 0;
-    char *output = NULL;
-    struct timeval wait;
-
-    /* input from xenconsole. read on xenconsoled_fd write to bootloader_fd */
-    int xenconsoled_prod = 0, xenconsoled_cons = 0;
-    char xenconsoled_buf[XENCONSOLED_BUF_SIZE];
-    /* output from bootloader. read on bootloader_fd write to xenconsoled_fd */
-    int bootloader_prod = 0, bootloader_cons = 0;
-    char bootloader_buf[BOOTLOADER_BUF_SIZE];
-
-    while(1) {
-        fd_set wsel, rsel;
-        int nfds;
-
-        /* Set timeout to 1s before starting to discard data */
-        wait.tv_sec = BOOTLOADER_TIMEOUT;
-        wait.tv_usec = 0;
-
-        /* Move buffers around to drop already consumed data */
-        if (xenconsoled_cons > 0) {
-            xenconsoled_prod -= xenconsoled_cons;
-            memmove(xenconsoled_buf, &xenconsoled_buf[xenconsoled_cons],
-                    xenconsoled_prod);
-            xenconsoled_cons = 0;
-        }
-        if (bootloader_cons > 0) {
-            bootloader_prod -= bootloader_cons;
-            memmove(bootloader_buf, &bootloader_buf[bootloader_cons],
-                    bootloader_prod);
-            bootloader_cons = 0;
-        }
-
-        FD_ZERO(&rsel);
-        FD_SET(fifo_fd, &rsel);
-        nfds = fifo_fd + 1;
-        if (xenconsoled_prod < XENCONSOLED_BUF_SIZE) {
-            /* The buffer is not full, try to read more data */
-            FD_SET(xenconsoled_fd, &rsel);
-            nfds = xenconsoled_fd + 1 > nfds ? xenconsoled_fd + 1 : nfds;
-        } 
-        if (bootloader_prod < BOOTLOADER_BUF_SIZE) {
-            /* The buffer is not full, try to read more data */
-            FD_SET(bootloader_fd, &rsel);
-            nfds = bootloader_fd + 1 > nfds ? bootloader_fd + 1 : nfds;
-        }
-
-        FD_ZERO(&wsel);
-        if (bootloader_prod > 0) {
-            /* The buffer has data to consume */
-            FD_SET(xenconsoled_fd, &wsel);
-            nfds = xenconsoled_fd + 1 > nfds ? xenconsoled_fd + 1 : nfds;
-        }
-        if (xenconsoled_prod > 0) {
-            /* The buffer has data to consume */
-            FD_SET(bootloader_fd, &wsel);
-            nfds = bootloader_fd + 1 > nfds ? bootloader_fd + 1 : nfds;
-        }
-
-        if (xenconsoled_prod == XENCONSOLED_BUF_SIZE ||
-            bootloader_prod == BOOTLOADER_BUF_SIZE)
-            ret = select(nfds, &rsel, &wsel, NULL, &wait);
-        else
-            ret = select(nfds, &rsel, &wsel, NULL, NULL);
-        if (ret < 0) {
-            if (errno == EINTR)
-                continue;
-            goto out_err;
-        }
-
-        /* Input from xenconsole, read xenconsoled_fd, write bootloader_fd */
-        if (ret == 0 && xenconsoled_prod == XENCONSOLED_BUF_SIZE) {
-            /* Drop the buffer */
-            xenconsoled_prod = 0;
-            xenconsoled_cons = 0;
-        } else if (FD_ISSET(xenconsoled_fd, &rsel)) {
-            ret = read(xenconsoled_fd, &xenconsoled_buf[xenconsoled_prod], 
XENCONSOLED_BUF_SIZE - xenconsoled_prod);
-            if (ret < 0 && errno != EIO && errno != EAGAIN)
-                goto out_err;
-            if (ret > 0)
-                xenconsoled_prod += ret;
-        }
-        if (FD_ISSET(bootloader_fd, &wsel)) {
-            ret = write(bootloader_fd, &xenconsoled_buf[xenconsoled_cons], 
xenconsoled_prod - xenconsoled_cons);
-            if (ret < 0 && errno != EIO && errno != EAGAIN)
-                goto out_err;
-            if (ret > 0)
-                xenconsoled_cons += ret;
-        }
+    STATE_AO_GC(bl->ao);
+    char buf[PATH_MAX*2];
+    FILE *f = 0;
+    int rc = ERROR_FAIL;
+    libxl_domain_build_info *info = bl->info;
+
+    f = fopen(bl->outputpath, "r");
+    if (!f) {
+        LOGE(ERROR,"open bootloader output file %s", bl->outputpath);
+        goto out;
+    }
 
-        /* Input from bootloader, read bootloader_fd, write xenconsoled_fd */
-        if (ret == 0 && bootloader_prod == BOOTLOADER_BUF_SIZE) {
-            /* Drop the buffer */
-            bootloader_prod = 0;
-            bootloader_cons = 0;
-        } else if (FD_ISSET(bootloader_fd, &rsel)) {
-            ret = read(bootloader_fd, &bootloader_buf[bootloader_prod], 
BOOTLOADER_BUF_SIZE - bootloader_prod);
-            if (ret < 0 && errno != EIO && errno != EAGAIN)
-                goto out_err;
-            if (ret > 0)
-                bootloader_prod += ret;
+    for (;;) {
+        /* Read a nul-terminated "line" and put the result in
+         * buf, and its length (not including the nul) in l */
+        int l = 0, c;
+        while ((c = getc(f)) != EOF && c != '\0') {
+            if (l < sizeof(buf)-1)
+                buf[l] = c;
+            l++;
         }
-        if (FD_ISSET(xenconsoled_fd, &wsel)) {
-            ret = write(xenconsoled_fd, &bootloader_buf[bootloader_cons], 
bootloader_prod - bootloader_cons);
-            if (ret < 0 && errno != EIO && errno != EAGAIN)
-                goto out_err;
-            if (ret > 0)
-                bootloader_cons += ret;
-        }
-
-        if (FD_ISSET(fifo_fd, &rsel)) {
-            if (size_out - nr_out < 256) {
-                char *temp;
-                size_t new_size = size_out == 0 ? 32 : size_out * 2;
-
-                temp = realloc(output, new_size);
-                if (temp == NULL)
-                    goto out_err;
-                output = temp;
-                memset(output + size_out, 0, new_size - size_out);
-                size_out = new_size;
+        if (c == EOF) {
+            if (ferror(f)) {
+                LOGE(ERROR,"read bootloader output file %s", bl->outputpath);
+                goto out;
             }
-
-            ret = read(fifo_fd, output + nr_out, size_out - nr_out);
-            if (ret > 0)
-                  nr_out += ret;
-            if (ret == 0)
+            if (!l)
                 break;
         }
-    }
+        if (l >= sizeof(buf)) {
+            LOG(WARN,"bootloader output contained"
+                " overly long item `%.150s...'", buf);
+            continue;
+        }
+        buf[l] = 0;
 
-    libxl__ptr_add(gc, output);
-    return output;
+        const char *rhs;
+#define COMMAND(s) ((rhs = bootloader_result_command(gc, buf, s, sizeof(s)-1)))
 
-out_err:
-    free(output);
-    return NULL;
-}
-
-static void parse_bootloader_result(libxl__gc *gc,
-                                    libxl_domain_build_info *info,
-                                    const char *o)
-{
-    while (*o != '\0') {
-        if (strncmp("kernel ", o, strlen("kernel ")) == 0) {
+        if (COMMAND("kernel")) {
             free(info->u.pv.kernel.path);
-            info->u.pv.kernel.path = strdup(o + strlen("kernel "));
+            info->u.pv.kernel.path = libxl__strdup(NULL, rhs);
             libxl__file_reference_map(&info->u.pv.kernel);
             unlink(info->u.pv.kernel.path);
-        } else if (strncmp("ramdisk ", o, strlen("ramdisk ")) == 0) {
+        } else if (COMMAND("ramdisk")) {
             free(info->u.pv.ramdisk.path);
-            info->u.pv.ramdisk.path = strdup(o + strlen("ramdisk "));
+            info->u.pv.ramdisk.path = libxl__strdup(NULL, rhs);
             libxl__file_reference_map(&info->u.pv.ramdisk);
             unlink(info->u.pv.ramdisk.path);
-        } else if (strncmp("args ", o, strlen("args ")) == 0) {
+        } else if (COMMAND("args")) {
             free(info->u.pv.cmdline);
-            info->u.pv.cmdline = strdup(o + strlen("args "));
+            info->u.pv.cmdline = libxl__strdup(NULL, rhs);
+        } else if (l) {
+            LOG(WARN, "unexpected output from bootloader: `%s'", buf);
         }
-
-        o = o + strlen(o) + 1;
     }
+    rc = 0;
+
+ out:
+    if (f) fclose(f);
+    return rc;
 }
 
-int libxl_run_bootloader(libxl_ctx *ctx,
-                         libxl_domain_build_info *info,
-                         libxl_device_disk *disk,
-                         uint32_t domid)
+
+/*----- init and cleanup -----*/
+
+void libxl__bootloader_init(libxl__bootloader_state *bl)
+{
+    assert(bl->ao);
+    bl->diskpath = NULL;
+    bl->openpty.ao = bl->ao;
+    bl->ptys[0].master = bl->ptys[0].slave = 0;
+    bl->ptys[1].master = bl->ptys[1].slave = 0;
+    libxl__ev_child_init(&bl->child);
+    bl->keystrokes.ao = bl->ao;  libxl__datacopier_init(&bl->keystrokes);
+    bl->display.ao = bl->ao;     libxl__datacopier_init(&bl->display);
+}
+
+static void bootloader_cleanup(libxl__egc *egc, libxl__bootloader_state *bl)
 {
-    GC_INIT(ctx);
-    int ret, rc = 0;
-    char *fifo = NULL;
-    char *diskpath = NULL;
-    char **args = NULL;
+    STATE_AO_GC(bl->ao);
+    int i;
 
-    char tempdir_template[] = "/var/run/libxl/bl.XXXXXX";
-    char *tempdir;
+    if (bl->outputpath) libxl__remove_file(gc, bl->outputpath);
+    if (bl->outputdir) libxl__remove_directory(gc, bl->outputdir);
 
-    char *dom_console_xs_path;
-    char dom_console_slave_tty_path[PATH_MAX];
+    if (bl->diskpath) {
+        libxl_device_disk_local_detach(CTX, bl->disk);
+        free(bl->diskpath);
+        bl->diskpath = 0;
+    }
+    libxl__datacopier_kill(&bl->keystrokes);
+    libxl__datacopier_kill(&bl->display);
+    for (i=0; i<2; i++) {
+        libxl__carefd_close(bl->ptys[i].master);
+        libxl__carefd_close(bl->ptys[i].slave);
+    }
+}
+
+static void bootloader_setpaths(libxl__gc *gc, libxl__bootloader_state *bl)
+{
+    uint32_t domid = bl->domid;
+    bl->outputdir = GCSPRINTF(XEN_RUN_DIR "/bootloader.%"PRIu32".d", domid);
+    bl->outputpath = GCSPRINTF(XEN_RUN_DIR "/bootloader.%"PRIu32".out", domid);
+}
 
-    int xenconsoled_fd = -1, xenconsoled_slave = -1;
-    int bootloader_fd = -1, fifo_fd = -1;
+static void bootloader_callback(libxl__egc *egc, libxl__bootloader_state *bl,
+                                int rc)
+{
+    bootloader_cleanup(egc, bl);
+    bl->callback(egc, bl, rc);
+}
 
-    int blrc;
-    pid_t pid;
-    char *blout;
+/*----- main flow of control -----*/
 
-    struct stat st_buf;
+void libxl__bootloader_run(libxl__egc *egc, libxl__bootloader_state *bl)
+{
+    STATE_AO_GC(bl->ao);
+    libxl_domain_build_info *info = bl->info;
+    int rc, r;
 
-    rc = libxl__domain_build_info_setdefault(gc, info);
-    if (rc) goto out;
+    libxl__bootloader_init(bl);
 
-    if (info->type != LIBXL_DOMAIN_TYPE_PV || !info->u.pv.bootloader)
-        goto out;
+    if (info->type != LIBXL_DOMAIN_TYPE_PV || !info->u.pv.bootloader) {
+        rc = 0;
+        goto out_ok;
+    }
 
-    rc = libxl__domain_build_info_setdefault(gc, info);
-    if (rc) goto out;
+    bootloader_setpaths(gc, bl);
 
-    rc = ERROR_INVAL;
-    if (!disk)
+    for (;;) {
+        r = mkdir(bl->outputdir, 0600);
+        if (!r) break;
+        if (errno == EINTR) continue;
+        if (errno == EEXIST) break;
+        LOGE(ERROR, "failed to create bootloader dir %s", bl->outputdir);
+        rc = ERROR_FAIL;
         goto out;
+    }
 
-    rc = ERROR_FAIL;
-    ret = mkdir("/var/run/libxl/", S_IRWXU);
-    if (ret < 0 && errno != EEXIST)
+    for (;;) {
+        r = open(bl->outputpath, O_WRONLY|O_CREAT|O_TRUNC, 0600);
+        if (r>=0) { close(r); break; }
+        if (errno == EINTR) continue;
+        LOGE(ERROR, "failed to precreate bootloader output %s", 
bl->outputpath);
+        rc = ERROR_FAIL;
         goto out;
+    }
 
-    ret = stat("/var/run/libxl/", &st_buf);
-    if (ret < 0)
+    bl->diskpath = libxl_device_disk_local_attach(CTX, bl->disk);
+    if (!bl->diskpath) {
+        rc = ERROR_FAIL;
         goto out;
+    }
 
-    if (!S_ISDIR(st_buf.st_mode))
-        goto out;
+    make_bootloader_args(gc, bl);
 
-    tempdir = mkdtemp(tempdir_template);
-    if (tempdir == NULL)
-        goto out;
+    bl->openpty.ao = ao;
+    bl->openpty.callback = bootloader_gotptys;
+    bl->openpty.count = 2;
+    bl->openpty.results = bl->ptys;
+    rc = libxl__openptys(&bl->openpty, 0,0);
+    if (rc) goto out;
 
-    ret = asprintf(&fifo, "%s/fifo", tempdir);
-    if (ret < 0) {
-        fifo = NULL;
-        goto out_close;
-    }
+    return;
 
-    ret = mkfifo(fifo, 0600);
-    if (ret < 0) {
-        goto out_close;
-    }
+ out:
+    assert(rc);
+ out_ok:
+    bootloader_callback(egc, bl, rc);
+}
 
-    diskpath = libxl_device_disk_local_attach(ctx, disk);
-    if (!diskpath) {
-        goto out_close;
-    }
+static void bootloader_gotptys(libxl__egc *egc, libxl__openpty_state *op)
+{
+    libxl__bootloader_state *bl = CONTAINER_OF(op, *bl, openpty);
+    STATE_AO_GC(bl->ao);
+    int rc, r;
 
-    args = make_bootloader_args(gc, info, domid, fifo, diskpath);
-    if (args == NULL) {
-        rc = ERROR_NOMEM;
-        goto out_close;
+    if (bl->openpty.rc) {
+        rc = bl->openpty.rc;
+        goto out;
     }
 
     /*
@@ -411,76 +332,188 @@ int libxl_run_bootloader(libxl_ctx *ctx,
      * where we copy characters between the two master fds, as well as
      * listening on the bootloader's fifo for the results.
      */
-    ret = open_xenconsoled_pty(&xenconsoled_fd, &xenconsoled_slave,
+
+    char *dom_console_xs_path;
+    char dom_console_slave_tty_path[PATH_MAX];
+    rc = setup_xenconsoled_pty(egc, bl,
                                &dom_console_slave_tty_path[0],
                                sizeof(dom_console_slave_tty_path));
-    if (ret < 0) {
-        goto out_close;
+    if (rc) goto out;
+
+    char *dompath = libxl__xs_get_dompath(gc, bl->domid);
+    if (!dompath) { rc = ERROR_FAIL; goto out; }
+
+    dom_console_xs_path = GCSPRINTF("%s/console/tty", dompath);
+
+    rc = libxl__xs_write(gc, XBT_NULL, dom_console_xs_path, "%s",
+                         dom_console_slave_tty_path);
+    if (rc) {
+        LOGE(ERROR,"xs write console path %s := %s failed",
+             dom_console_xs_path, dom_console_slave_tty_path);
+        rc = ERROR_FAIL;
+        goto out;
     }
 
-    dom_console_xs_path = libxl__sprintf(gc, "%s/console/tty", 
libxl__xs_get_dompath(gc, domid));
-    libxl__xs_write(gc, XBT_NULL, dom_console_xs_path, "%s", 
dom_console_slave_tty_path);
+    int bootloader_master = libxl__carefd_fd(bl->ptys[0].master);
+    int xenconsole_master = libxl__carefd_fd(bl->ptys[1].master);
+
+    libxl_fd_set_nonblock(CTX, bootloader_master, 1);
+    libxl_fd_set_nonblock(CTX, xenconsole_master, 1);
+
+    bl->keystrokes.writefd   = bl->display.readfd   = bootloader_master;
+    bl->keystrokes.writewhat = bl->display.readwhat = "bootloader pty";
+
+    bl->keystrokes.readfd   = bl->display.writefd   = xenconsole_master;
+    bl->keystrokes.readwhat = bl->display.writewhat = "xenconsole client pty";
+
+    bl->keystrokes.ao = ao;
+    bl->keystrokes.maxsz = BOOTLOADER_BUF_OUT;
+    bl->keystrokes.copywhat =
+        GCSPRINTF("bootloader input for domain %"PRIu32, bl->domid);
+    bl->keystrokes.callback = bootloader_keystrokes_copyfail;
+    rc = libxl__datacopier_start(&bl->keystrokes);
+    if (rc) goto out;
+
+    bl->display.ao = ao;
+    bl->display.maxsz = BOOTLOADER_BUF_IN;
+    bl->display.copywhat =
+        GCSPRINTF("bootloader output for domain %"PRIu32, bl->domid);
+    bl->display.callback = bootloader_display_copyfail;
+    rc = libxl__datacopier_start(&bl->display);
+    if (rc) goto out;
+
+    LOG(DEBUG, "executing bootloader: %s", bl->args[0]);
+    for (const char **blarg = bl->args;
+         *blarg;
+         blarg++)
+        LOG(DEBUG, "  bootloader arg: %s", *blarg);
+
+    struct termios termattr;
 
-    pid = fork_exec_bootloader(&bootloader_fd, info->u.pv.bootloader, args);
-    if (pid < 0) {
-        goto out_close;
+    pid_t pid = libxl__ev_child_fork(gc, &bl->child, bootloader_finished);
+    if (pid == -1) {
+        rc = ERROR_FAIL;
+        goto out;
     }
 
-    while (1) {
-        if (waitpid(pid, &blrc, WNOHANG) == pid)
-            goto out_close;
+    if (!pid) {
+        /* child */
+        r = login_tty(libxl__carefd_fd(bl->ptys[0].slave));
+        if (r) { LOGE(ERROR, "login_tty failed"); exit(-1); }
+        setenv("TERM", "vt100", 1);
+        libxl__exec(-1, -1, -1, bl->args[0], (char**)bl->args);
+        exit(-1);
+    }
 
-        fifo_fd = open(fifo, O_RDONLY);
-        if (fifo_fd > -1)
-            break;
+    /* parent */
 
-        if (errno == EINTR)
-            continue;
+    /*
+     * On Solaris, the master pty side does not have terminal semantics,
+     * so don't try to set any attributes, as it will fail.
+     */
+#if !defined(__sun__)
+    tcgetattr(bootloader_master, &termattr);
+    cfmakeraw(&termattr);
+    tcsetattr(bootloader_master, TCSANOW, &termattr);
+#endif
+
+    return;
+
+ out:
+    bootloader_callback(egc, bl, rc);
+}
 
-        goto out_close;
+/* perhaps one of these will be called, but perhaps not */
+static void bootloader_copyfail(libxl__egc *egc, const char *which,
+       libxl__bootloader_state *bl, int onwrite, int errnoval)
+{
+    STATE_AO_GC(bl->ao);
+    int r;
+
+    if (!onwrite && !errnoval)
+        LOG(ERROR, "unexpected eof copying %s", which);
+    libxl__datacopier_kill(&bl->keystrokes);
+    libxl__datacopier_kill(&bl->display);
+    if (libxl__ev_child_inuse(&bl->child)) {
+        r = kill(bl->child.pid, SIGTERM);
+        if (r) LOGE(WARN, "after failure, failed to kill bootloader [%lu]",
+                    (unsigned long)bl->child.pid);
     }
+    bl->rc = ERROR_FAIL;
+}
+static void bootloader_keystrokes_copyfail(libxl__egc *egc,
+       libxl__datacopier_state *dc, int onwrite, int errnoval)
+{
+    libxl__bootloader_state *bl = CONTAINER_OF(dc, *bl, keystrokes);
+    bootloader_copyfail(egc, "bootloader input", bl, onwrite, errnoval);
+}
+static void bootloader_display_copyfail(libxl__egc *egc,
+       libxl__datacopier_state *dc, int onwrite, int errnoval)
+{
+    libxl__bootloader_state *bl = CONTAINER_OF(dc, *bl, display);
+    bootloader_copyfail(egc, "bootloader output", bl, onwrite, errnoval);
+}
 
-    fcntl(fifo_fd, F_SETFL, O_NDELAY);
+static void bootloader_finished(libxl__egc *egc, libxl__ev_child *child,
+                                pid_t pid, int status)
+{
+    libxl__bootloader_state *bl = CONTAINER_OF(child, *bl, child);
+    STATE_AO_GC(bl->ao);
+    int rc;
 
-    blout = bootloader_interact(gc, xenconsoled_fd, bootloader_fd, fifo_fd);
-    if (blout == NULL) {
-        goto out_close;
+    libxl__datacopier_kill(&bl->keystrokes);
+    libxl__datacopier_kill(&bl->display);
+
+    if (status) {
+        libxl_report_child_exitstatus(CTX, XTL_ERROR, "bootloader",
+                                      pid, status);
+        rc = ERROR_FAIL;
+        goto out;
+    } else {
+        LOG(DEBUG, "bootloader completed");
     }
 
-    pid = waitpid(pid, &blrc, 0);
-    if (pid == -1 || (pid > 0 && WIFEXITED(blrc) && WEXITSTATUS(blrc) != 0)) {
-        goto out_close;
+    if (bl->rc) {
+        /* datacopier went wrong */
+        rc = bl->rc;
+        goto out;
     }
 
-    parse_bootloader_result(gc, info, blout);
+    rc = parse_bootloader_result(egc, bl);
+    if (rc) goto out;
 
     rc = 0;
-out_close:
-    if (diskpath) {
-        libxl_device_disk_local_detach(ctx, disk);
-        free(diskpath);
-    }
-    if (fifo_fd > -1)
-        close(fifo_fd);
-    if (bootloader_fd > -1)
-        close(bootloader_fd);
-    if (xenconsoled_fd > -1)
-        close(xenconsoled_fd);
-    if (xenconsoled_slave > -1)
-        close(xenconsoled_slave);
-
-    if (fifo) {
-        unlink(fifo);
-        free(fifo);
-    }
+    LOG(DEBUG, "bootloader execution successful");
 
-    rmdir(tempdir);
+ out:
+    bootloader_callback(egc, bl, rc);
+}
 
-    free(args);
+/*----- entrypoint for external callers -----*/
 
-out:
-    GC_FREE;
-    return rc;
+static void run_bootloader_done(libxl__egc *egc,
+                                libxl__bootloader_state *st, int rc)
+{
+    libxl__ao_complete(egc, st->ao, rc);
+}
+
+int libxl_run_bootloader(libxl_ctx *ctx,
+                         libxl_domain_build_info *info,
+                         libxl_device_disk *disk,
+                         uint32_t domid,
+                         libxl_asyncop_how *ao_how)
+{
+    AO_CREATE(ctx,domid,ao_how);
+    libxl__bootloader_state *bl;
+
+    GCNEW(bl);
+    bl->ao = ao;
+    bl->callback = run_bootloader_done;
+    bl->info = info;
+    bl->disk = disk;
+    bl->domid = domid;
+    libxl__bootloader_run(egc, bl);
+    return AO_INPROGRESS;
 }
 
 /*
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 3675afe..dbc3cf0 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -572,8 +572,12 @@ static int do_domain_create(libxl__gc *gc, 
libxl_domain_config *d_config,
         if (ret) goto error_out;
     }
 
-    if ( restore_fd < 0 ) {
-        ret = libxl_run_bootloader(ctx, &d_config->b_info, d_config->num_disks 
> 0 ? &d_config->disks[0] : NULL, domid);
+    libxl_device_disk *bootdisk =
+        d_config->num_disks > 0 ? &d_config->disks[0] : NULL;
+
+    if (restore_fd < 0 && bootdisk) {
+        ret = libxl_run_bootloader(ctx, &d_config->b_info, bootdisk, domid,
+                                   0 /* fixme-ao */);
         if (ret) {
             LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
                        "failed to run bootloader: %d", ret);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 9af99ec..b3f84ba 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -42,6 +42,7 @@
 #include <sys/time.h>
 #include <sys/types.h>
 #include <sys/wait.h>
+#include <sys/socket.h>
 
 #include <xs.h>
 #include <xenctrl.h>
@@ -449,6 +450,7 @@ _hidden int libxl__remove_file(libxl__gc *gc, const char 
*path);
 _hidden int libxl__remove_directory(libxl__gc *gc, const char *path);
 _hidden int libxl__remove_file_or_directory(libxl__gc *gc, const char *path);
 
+
 _hidden char **libxl__xs_kvs_of_flexarray(libxl__gc *gc, flexarray_t *array, 
int length);
 
 _hidden int libxl__xs_writev(libxl__gc *gc, xs_transaction_t t,
@@ -1534,6 +1536,38 @@ int libxl__openptys(libxl__openpty_state *op,
                     const struct winsize *winp);
 
 
+/*----- bootloader -----*/
+
+typedef struct libxl__bootloader_state libxl__bootloader_state;
+typedef void libxl__run_bootloader_callback(libxl__egc*,
+                                libxl__bootloader_state*, int rc);
+
+struct libxl__bootloader_state {
+    /* caller must fill these in, and they must all remain valid */
+    libxl__ao *ao;
+    libxl__run_bootloader_callback *callback;
+    libxl_domain_build_info *info; /* u.pv.{kernel,ramdisk,cmdline} updated */
+    libxl_device_disk *disk;
+    uint32_t domid;
+    /* private to libxl__run_bootloader */
+    char *outputpath, *outputdir;
+    char *diskpath; /* not from gc, represents actually attached disk */
+    libxl__openpty_state openpty;
+    libxl__openpty_result ptys[2];  /* [0] is for bootloader */
+    libxl__ev_child child;
+    int nargs, argsspace;
+    const char **args;
+    libxl__datacopier_state keystrokes, display;
+    int rc;
+};
+
+_hidden void libxl__bootloader_init(libxl__bootloader_state *bl);
+
+/* Will definitely call st->callback, perhaps reentrantly.
+ * If callback is passed rc==0, will have updated st->info appropriately */
+_hidden void libxl__bootloader_run(libxl__egc*, libxl__bootloader_state *st);
+
+
 /*
  * Convenience macros.
  */
-- 
1.7.2.5


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


 


Rackspace

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