[PATCH] libxl: do not overwrite user supplied config when running bootloader

* [PATCH] libxl: do not overwrite user supplied config when running bootloader
@ 2012-05-17 16:51 Ian Campbell
  2012-05-18  8:25 ` Ian Campbell
  2012-05-18 11:35 ` Ian Jackson
  0 siblings, 2 replies; 9+ messages in thread
From: Ian Campbell @ 2012-05-17 16:51 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1337273492 -3600
# Node ID cdb947baea102aa6a1d53472f8a3e5f2d6cc485e
# Parent  ac45608496cd85b0bf1aed6e5b869b4a86ca672f
libxl: do not overwrite user supplied config when running bootloader.

Currently when running the bootloader libxl will update b_info->u.pv.kernel,
.ramdisk, .cmdline and .bootloader.  This can expose internal details, such
as temporary paths in /var/run/xen/bootloader.*/ to the user. This is
problematic because it means that the user cannot re-use the struct as is.

This does not effect xl in Xen 4.2+ since it always reparses the guest config
and reinitialises the build info, however it did cause issues with reboot in
4.1 (reported by Dmitry Morozhnikov) and may cause issues for other users of
libxl.

Instead make the libxl bootloader infrastructure provide output to its caller
which is slurped into the internal libxl__domain_build_state datastructure. If
no bootloader is configured then the bootloader instead propagates the user
supplied b_info config.

In order to simplify this push the error handling for the case where there is
no bootdisk down into libxl__bootloader_run. In principal there is no reason
why it shouldn't be possible to do a pure netboot guest with a suitable
bootloader, but I don't fix that here.

This change allow us to make the libxl_file_reference an internal API, and
eventually we might be able to get rid of it.

Also removes the publix libxl_run_bootloader interface, neither xl nor libvirt
use it.

I am proposing this for 4.2 due to the API change.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r ac45608496cd -r cdb947baea10 tools/libxl/gentest.py

--- a/tools/libxl/gentest.py	Thu May 17 16:39:51 2012 +0100
+++ b/tools/libxl/gentest.py	Thu May 17 17:51:32 2012 +0100
@@ -21,8 +21,7 @@ def randomize_enum(e):
     return random.choice([v.name for v in e.values])
 
 handcoded = ["libxl_cpumap", "libxl_key_value_list",
-             "libxl_cpuid_policy_list", "libxl_file_reference",
-             "libxl_string_list"]
+             "libxl_cpuid_policy_list", "libxl_string_list"]
 
 def gen_rand_init(ty, v, indent = "    ", parent = None):
     s = ""
@@ -179,13 +178,6 @@ static void libxl_cpuid_policy_list_rand
     *pp = p;
 }
 
-static void libxl_file_reference_rand_init(libxl_file_reference *p)
-{
-    memset(p, 0, sizeof(*p));
-    if (rand() % 8)
-        p->path = rand_str();
-}
-
 static void libxl_string_list_rand_init(libxl_string_list *p)
 {
     int i, nr = rand() % 16;
diff -r ac45608496cd -r cdb947baea10 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Thu May 17 16:39:51 2012 +0100
+++ b/tools/libxl/libxl.c	Thu May 17 17:51:32 2012 +0100
@@ -3623,12 +3623,6 @@ int libxl_tmem_freeable(libxl_ctx *ctx)
     return rc;
 }
 
-void libxl_file_reference_dispose(libxl_file_reference *f)
-{
-    libxl__file_reference_unmap(f);
-    free(f->path);
-}
-
 int libxl_get_freecpus(libxl_ctx *ctx, libxl_cpumap *cpumap)
 {
     int ncpus;
diff -r ac45608496cd -r cdb947baea10 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Thu May 17 16:39:51 2012 +0100
+++ b/tools/libxl/libxl.h	Thu May 17 17:51:32 2012 +0100
@@ -288,18 +288,6 @@ typedef struct {
 } libxl_cpumap;
 void libxl_cpumap_dispose(libxl_cpumap *map);
 
-typedef struct {
-    /*
-     * Path is always set if the file reference is valid. However if
-     * mapped is true then the actual file may already be unlinked.
-     */
-    char * path;
-    int mapped;
-    void * data;
-    size_t size;
-} libxl_file_reference;
-void libxl_file_reference_dispose(libxl_file_reference *p);
-
 /* libxl_cpuid_policy_list is a dynamic array storing CPUID policies
  * for multiple leafs. It is terminated with an entry holding
  * XEN_CPUID_INPUT_UNUSED in input[0]
@@ -536,27 +524,6 @@ int libxl_domain_preserve(libxl_ctx *ctx
 /* get max. number of cpus supported by hypervisor */
 int libxl_get_max_cpus(libxl_ctx *ctx);
 
-/*
- * Run the configured bootloader for a PV domain and update
- * info->kernel, info->u.pv.ramdisk and info->u.pv.cmdline as
- * appropriate (any initial values present in these fields must have
- * been allocated with malloc).
- *
- * Is a NOP on non-PV domains or those with no bootloader configured.
- *
- * Users should call libxl_file_reference_unmap on the kernel and
- * ramdisk to cleanup or rely on libxl_domain_{build,restore} to do
- * it.
- */
-int libxl_run_bootloader(libxl_ctx *ctx,
-                         libxl_domain_build_info *info,
-                         libxl_device_disk *disk,
-                         uint32_t domid,
-                         libxl_asyncop_how *ao_how);
-
-  /* 0 means ERROR_ENOMEM, which we have logged */
-
-
 int libxl_domain_rename(libxl_ctx *ctx, uint32_t domid,
                         const char *old_name, const char *new_name);
 
diff -r ac45608496cd -r cdb947baea10 tools/libxl/libxl_bootloader.c
--- a/tools/libxl/libxl_bootloader.c	Thu May 17 16:39:51 2012 +0100
+++ b/tools/libxl/libxl_bootloader.c	Thu May 17 17:51:32 2012 +0100
@@ -43,7 +43,8 @@ static void bootloader_arg(libxl__bootlo
     bl->args[bl->nargs++] = arg;
 }
 
-static void make_bootloader_args(libxl__gc *gc, libxl__bootloader_state *bl)
+static void make_bootloader_args(libxl__gc *gc, libxl__bootloader_state *bl,
+                                 const char *bootloader_path)
 {
     const libxl_domain_build_info *info = bl->info;
 
@@ -53,12 +54,12 @@ static void make_bootloader_args(libxl__
 
 #define ARG(arg) bootloader_arg(bl, (arg))
 
-    ARG(info->u.pv.bootloader);
+    ARG(bootloader_path);
 
-    if (info->u.pv.kernel.path)
-        ARG(libxl__sprintf(gc, "--kernel=%s", info->u.pv.kernel.path));
-    if (info->u.pv.ramdisk.path)
-        ARG(libxl__sprintf(gc, "--ramdisk=%s", info->u.pv.ramdisk.path));
+    if (info->u.pv.kernel)
+        ARG(libxl__sprintf(gc, "--kernel=%s", info->u.pv.kernel));
+    if (info->u.pv.ramdisk)
+        ARG(libxl__sprintf(gc, "--ramdisk=%s", info->u.pv.ramdisk));
     if (info->u.pv.cmdline && *info->u.pv.cmdline != '\0')
         ARG(libxl__sprintf(gc, "--args=%s", info->u.pv.cmdline));
 
@@ -144,7 +145,6 @@ static int parse_bootloader_result(libxl
     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) {
@@ -180,18 +180,15 @@ static int parse_bootloader_result(libxl
 #define COMMAND(s) ((rhs = bootloader_result_command(gc, buf, s, sizeof(s)-1)))
 
         if (COMMAND("kernel")) {
-            free(info->u.pv.kernel.path);
-            info->u.pv.kernel.path = libxl__strdup(NULL, rhs);
-            libxl__file_reference_map(&info->u.pv.kernel);
-            unlink(info->u.pv.kernel.path);
+            bl->kernel->path = libxl__strdup(gc, rhs);
+            libxl__file_reference_map(bl->kernel);
+            unlink(bl->kernel->path);
         } else if (COMMAND("ramdisk")) {
-            free(info->u.pv.ramdisk.path);
-            info->u.pv.ramdisk.path = libxl__strdup(NULL, rhs);
-            libxl__file_reference_map(&info->u.pv.ramdisk);
-            unlink(info->u.pv.ramdisk.path);
+            bl->ramdisk->path = libxl__strdup(gc, rhs);
+            libxl__file_reference_map(bl->ramdisk);
+            unlink(bl->ramdisk->path);
         } else if (COMMAND("args")) {
-            free(info->u.pv.cmdline);
-            info->u.pv.cmdline = libxl__strdup(NULL, rhs);
+            bl->cmdline = libxl__strdup(gc, rhs);
         } else if (l) {
             LOG(WARN, "unexpected output from bootloader: `%s'", buf);
         }
@@ -281,18 +278,35 @@ static void bootloader_abort(libxl__egc 
 void libxl__bootloader_run(libxl__egc *egc, libxl__bootloader_state *bl)
 {
     STATE_AO_GC(bl->ao);
-    libxl_domain_build_info *info = bl->info;
+    const libxl_domain_build_info *info = bl->info;
     uint32_t domid = bl->domid;
     char *logfile_tmp = NULL;
     int rc, r;
+    const char *bootloader;
 
     libxl__bootloader_init(bl);
 
-    if (info->type != LIBXL_DOMAIN_TYPE_PV || !info->u.pv.bootloader) {
+    if (info->type != LIBXL_DOMAIN_TYPE_PV) {
+        LOG(DEBUG, "not a PV domain, skipping bootloader");
         rc = 0;
         goto out_ok;
     }
 
+    if (!info->u.pv.bootloader) {
+        LOG(DEBUG, "no bootloader configured, using user supplied kernel");
+        bl->kernel->path = bl->info->u.pv.kernel;
+        bl->ramdisk->path = bl->info->u.pv.ramdisk;
+        bl->cmdline = bl->info->u.pv.cmdline;
+        rc = 0;
+        goto out_ok;
+    }
+
+    if (!bl->disk) {
+        LOG(ERROR, "cannot run bootloader with no boot disk");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
     bootloader_setpaths(gc, bl);
 
     const char *logfile_leaf = GCSPRINTF("bootloader.%"PRIu32, domid);
@@ -342,27 +356,26 @@ void libxl__bootloader_run(libxl__egc *e
         LOG(WARN, "bootloader='/usr/bin/pygrub' is deprecated; use " \
             "bootloader='pygrub' instead");
 
+    bootloader = info->u.pv.bootloader;
+
     /* If the full path is not specified, check in the libexec path */
-    if ( info->u.pv.bootloader[0] != '/' ) {
-        char *bootloader;
+    if ( bootloader[0] != '/' ) {
+        const char *bltmp;
         struct stat st;
 
-        bootloader = libxl__abs_path(gc, info->u.pv.bootloader,
-                                     libxl__libexec_path());
+        bltmp = libxl__abs_path(gc, bootloader, libxl__libexec_path());
         /* Check to see if the file exists in this location; if not,
          * fall back to checking the path */
-        LOG(DEBUG, "Checking for bootloader in libexec path: %s", bootloader);
+        LOG(DEBUG, "Checking for bootloader in libexec path: %s", bltmp);
 
-        if ( lstat(bootloader, &st) )
+        if ( lstat(bltmp, &st) )
             LOG(DEBUG, "%s doesn't exist, falling back to config path",
-                bootloader);
-        else {
-            free(info->u.pv.bootloader);
-            info->u.pv.bootloader = strdup(bootloader);
-        }
+                bltmp);
+        else
+            bootloader = bltmp;
     }
 
-    make_bootloader_args(gc, bl);
+    make_bootloader_args(gc, bl, bootloader);
 
     bl->openpty.ao = ao;
     bl->openpty.callback = bootloader_gotptys;
@@ -565,33 +578,6 @@ static void bootloader_finished(libxl__e
     bootloader_callback(egc, bl, rc);
 }
 
-/*----- entrypoint for external callers -----*/
-
-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;
-}
-
 /*
  * Local variables:
  * mode: C
diff -r ac45608496cd -r cdb947baea10 tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c	Thu May 17 16:39:51 2012 +0100
+++ b/tools/libxl/libxl_create.c	Thu May 17 17:51:32 2012 +0100
@@ -242,6 +242,7 @@ static int init_console_info(libxl__devi
         return ERROR_NOMEM;
     return 0;
 }
+
 int libxl__domain_build(libxl__gc *gc,
                         libxl_domain_build_info *info,
                         uint32_t domid,
@@ -290,17 +291,18 @@ int libxl__domain_build(libxl__gc *gc,
         vments[i++] = "image/ostype";
         vments[i++] = "linux";
         vments[i++] = "image/kernel";
-        vments[i++] = (char*) info->u.pv.kernel.path;
+        vments[i++] = (char *) state->pv_kernel.path;
         vments[i++] = "start_time";
         vments[i++] = libxl__sprintf(gc, "%lu.%02d", start_time.tv_sec,(int)start_time.tv_usec/10000);
-        if (info->u.pv.ramdisk.path) {
+        if (state->pv_ramdisk.path) {
             vments[i++] = "image/ramdisk";
-            vments[i++] = (char*) info->u.pv.ramdisk.path;
+            vments[i++] = (char *) state->pv_ramdisk.path;
         }
-        if (info->u.pv.cmdline) {
+        if (state->pv_cmdline) {
             vments[i++] = "image/cmdline";
-            vments[i++] = (char*) info->u.pv.cmdline;
+            vments[i++] = (char *) state->pv_cmdline;
         }
+
         break;
     default:
         ret = ERROR_INVAL;
@@ -346,16 +348,16 @@ static int domain_restore(libxl__gc *gc,
         vments[i++] = "image/ostype";
         vments[i++] = "linux";
         vments[i++] = "image/kernel";
-        vments[i++] = (char*) info->u.pv.kernel.path;
+        vments[i++] = (char *) state->pv_kernel.path;
         vments[i++] = "start_time";
         vments[i++] = libxl__sprintf(gc, "%lu.%02d", start_time.tv_sec,(int)start_time.tv_usec/10000);
-        if (info->u.pv.ramdisk.path) {
+        if (state->pv_ramdisk.path) {
             vments[i++] = "image/ramdisk";
-            vments[i++] = (char*) info->u.pv.ramdisk.path;
+            vments[i++] = (char *) state->pv_ramdisk.path;
         }
-        if (info->u.pv.cmdline) {
+        if (state->pv_cmdline) {
             vments[i++] = "image/cmdline";
-            vments[i++] = (char*) info->u.pv.cmdline;
+            vments[i++] = (char *) state->pv_cmdline;
         }
         break;
     default:
@@ -374,8 +376,8 @@ static int domain_restore(libxl__gc *gc,
 
 out:
     if (info->type == LIBXL_DOMAIN_TYPE_PV) {
-        libxl__file_reference_unmap(&info->u.pv.kernel);
-        libxl__file_reference_unmap(&info->u.pv.ramdisk);
+        libxl__file_reference_unmap(&state->pv_kernel);
+        libxl__file_reference_unmap(&state->pv_ramdisk);
     }
 
     esave = errno;
@@ -625,16 +627,21 @@ static void initiate_domain_create(libxl
     libxl_device_disk *bootdisk =
         d_config->num_disks > 0 ? &d_config->disks[0] : NULL;
 
-    if (restore_fd < 0 && bootdisk) {
+    if (restore_fd >= 0) {
+        LOG(DEBUG, "restoring, not running bootloader\n");
+        domcreate_bootloader_done(egc, &dcs->bl, 0);
+    } else  {
+        LOG(DEBUG, "running bootloader");
         dcs->bl.callback = domcreate_bootloader_done;
         dcs->bl.console_available = domcreate_bootloader_console_available;
-        dcs->bl.info = &d_config->b_info,
+        dcs->bl.info = &d_config->b_info;
         dcs->bl.disk = bootdisk;
         dcs->bl.domid = dcs->guest_domid;
-            
+
+        dcs->bl.kernel = &dcs->build_state.pv_kernel;
+        dcs->bl.ramdisk = &dcs->build_state.pv_ramdisk;
+
         libxl__bootloader_run(egc, &dcs->bl);
-    } else {
-        domcreate_bootloader_done(egc, &dcs->bl, 0);
     }
     return;
 
@@ -675,6 +682,12 @@ static void domcreate_bootloader_done(li
 
     if (ret) goto error_out;
 
+    /* consume bootloader outputs. state->pv_{kernel,ramdisk} have
+     * been initialised by the bootloader already.
+     */
+    LOG(INFO, "bootloader cmdline %s", bl->cmdline);
+    state->pv_cmdline = bl->cmdline;
+
     /* We might be going to call libxl__spawn_local_dm, or _spawn_stub_dm.
      * Fill in any field required by either, including both relevant
      * callbacks (_spawn_stub_dm will overwrite our trespass if needed). */
diff -r ac45608496cd -r cdb947baea10 tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c	Thu May 17 16:39:51 2012 +0100
+++ b/tools/libxl/libxl_dm.c	Thu May 17 17:51:32 2012 +0100
@@ -715,10 +715,6 @@ void libxl__spawn_stub_dm(libxl__egc *eg
     dm_config->b_info.max_memkb = 32 * 1024;
     dm_config->b_info.target_memkb = dm_config->b_info.max_memkb;
 
-    dm_config->b_info.u.pv.kernel.path = libxl__abs_path(gc, "ioemu-stubdom.gz",
-                                              libxl__xenfirmwaredir_path());
-    dm_config->b_info.u.pv.cmdline = libxl__sprintf(gc, " -d %d", guest_domid);
-    dm_config->b_info.u.pv.ramdisk.path = "";
     dm_config->b_info.u.pv.features = "";
 
     dm_config->b_info.device_model_version =
@@ -746,6 +742,11 @@ void libxl__spawn_stub_dm(libxl__egc *eg
     dm_config->vkbs = &vkb;
     dm_config->num_vkbs = 1;
 
+    stubdom_state->pv_kernel.path
+        = libxl__abs_path(gc, "ioemu-stubdom.gz", libxl__xenfirmwaredir_path());
+    stubdom_state->pv_cmdline = libxl__sprintf(gc, " -d %d", guest_domid);
+    stubdom_state->pv_ramdisk.path = "";
+
     /* fixme: this function can leak the stubdom if it fails */
     ret = libxl__domain_make(gc, &dm_config->c_info, &sdss->pvqemu.guest_domid);
     if (ret)
diff -r ac45608496cd -r cdb947baea10 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c	Thu May 17 16:39:51 2012 +0100
+++ b/tools/libxl/libxl_dom.c	Thu May 17 17:51:32 2012 +0100
@@ -240,36 +240,37 @@ int libxl__build_pv(libxl__gc *gc, uint3
 
     xc_dom_loginit(ctx->xch);
 
-    dom = xc_dom_allocate(ctx->xch, info->u.pv.cmdline, info->u.pv.features);
+    dom = xc_dom_allocate(ctx->xch, state->pv_cmdline, info->u.pv.features);
     if (!dom) {
         LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "xc_dom_allocate failed");
         return ERROR_FAIL;
     }
 
-    if (info->u.pv.kernel.mapped) {
+    LOG(INFO, "pv kernel mapped %d path %s\n", state->pv_kernel.mapped, state->pv_kernel.path);
+    if (state->pv_kernel.mapped) {
         ret = xc_dom_kernel_mem(dom,
-                                info->u.pv.kernel.data,
-                                info->u.pv.kernel.size);
+                                state->pv_kernel.data,
+                                state->pv_kernel.size);
         if ( ret != 0) {
             LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "xc_dom_kernel_mem failed");
             goto out;
         }
     } else {
-        ret = xc_dom_kernel_file(dom, info->u.pv.kernel.path);
+        ret = xc_dom_kernel_file(dom, state->pv_kernel.path);
         if ( ret != 0) {
             LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "xc_dom_kernel_file failed");
             goto out;
         }
     }
 
-    if ( info->u.pv.ramdisk.path && strlen(info->u.pv.ramdisk.path) ) {
-        if (info->u.pv.ramdisk.mapped) {
-            if ( (ret = xc_dom_ramdisk_mem(dom, info->u.pv.ramdisk.data, info->u.pv.ramdisk.size)) != 0 ) {
+    if ( state->pv_ramdisk.path && strlen(state->pv_ramdisk.path) ) {
+        if (state->pv_ramdisk.mapped) {
+            if ( (ret = xc_dom_ramdisk_mem(dom, state->pv_ramdisk.data, state->pv_ramdisk.size)) != 0 ) {
                 LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "xc_dom_ramdisk_mem failed");
                 goto out;
             }
         } else {
-            if ( (ret = xc_dom_ramdisk_file(dom, info->u.pv.ramdisk.path)) != 0 ) {
+            if ( (ret = xc_dom_ramdisk_file(dom, state->pv_ramdisk.path)) != 0 ) {
                 LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "xc_dom_ramdisk_file failed");
                 goto out;
             }
@@ -314,6 +315,9 @@ int libxl__build_pv(libxl__gc *gc, uint3
     state->console_mfn = xc_dom_p2m_host(dom, dom->console_pfn);
     state->store_mfn = xc_dom_p2m_host(dom, dom->xenstore_pfn);
 
+    libxl__file_reference_unmap(&state->pv_kernel);
+    libxl__file_reference_unmap(&state->pv_ramdisk);
+
     ret = 0;
 out:
     xc_dom_release(dom);
diff -r ac45608496cd -r cdb947baea10 tools/libxl/libxl_internal.c
--- a/tools/libxl/libxl_internal.c	Thu May 17 16:39:51 2012 +0100
+++ b/tools/libxl/libxl_internal.c	Thu May 17 17:51:32 2012 +0100
@@ -216,7 +216,7 @@ char *libxl__abs_path(libxl__gc *gc, con
 }
 
 
-int libxl__file_reference_map(libxl_file_reference *f)
+int libxl__file_reference_map(libxl__file_reference *f)
 {
     struct stat st_buf;
     int ret, fd;
@@ -249,7 +249,7 @@ out:
     return ret == 0 ? 0 : ERROR_FAIL;
 }
 
-int libxl__file_reference_unmap(libxl_file_reference *f)
+int libxl__file_reference_unmap(libxl__file_reference *f)
 {
     int ret;
 
diff -r ac45608496cd -r cdb947baea10 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Thu May 17 16:39:51 2012 +0100
+++ b/tools/libxl/libxl_internal.h	Thu May 17 17:51:32 2012 +0100
@@ -713,6 +713,20 @@ int libxl__self_pipe_eatall(int fd); /* 
 _hidden int libxl__atfork_init(libxl_ctx *ctx);
 
 
+/* File references */
+typedef struct {
+    /*
+     * Path is always set if the file reference is valid. However if
+     * mapped is true then the actual file may already be unlinked.
+     */
+    const char * path;
+    int mapped;
+    void * data;
+    size_t size;
+} libxl__file_reference;
+_hidden int libxl__file_reference_map(libxl__file_reference *f);
+_hidden int libxl__file_reference_unmap(libxl__file_reference *f);
+
 /* from xl_dom */
 _hidden libxl_domain_type libxl__domain_type(libxl__gc *gc, uint32_t domid);
 _hidden int libxl__domain_shutdown_reason(libxl__gc *gc, uint32_t domid);
@@ -731,6 +745,10 @@ typedef struct {
     unsigned long vm_generationid_addr;
 
     char *saved_state;
+
+    libxl__file_reference pv_kernel;
+    libxl__file_reference pv_ramdisk;
+    const char * pv_cmdline;
 } libxl__domain_build_state;
 
 _hidden int libxl__build_pre(libxl__gc *gc, uint32_t domid,
@@ -1246,9 +1264,6 @@ struct libxl__xen_console_reader {
 
 _hidden int libxl__error_set(libxl__gc *gc, int code);
 
-_hidden int libxl__file_reference_map(libxl_file_reference *f);
-_hidden int libxl__file_reference_unmap(libxl_file_reference *f);
-
 _hidden int libxl__e820_alloc(libxl__gc *gc, uint32_t domid, libxl_domain_config *d_config);
 
 /* parse the string @s as a sequence of 6 colon separated bytes in to @mac */
@@ -1757,9 +1772,14 @@ struct libxl__bootloader_state {
     libxl__ao *ao;
     libxl__run_bootloader_callback *callback;
     libxl__bootloader_console_callback *console_available;
-    libxl_domain_build_info *info; /* u.pv.{kernel,ramdisk,cmdline} updated */
+    const libxl_domain_build_info *info;
     libxl_device_disk *disk;
     uint32_t domid;
+    /* outputs, call must set kernel and ramdisk to point to a file
+     * reference which will be updated and mapped, cmdline will be
+     * updated */
+    libxl__file_reference *kernel, *ramdisk;
+    const char *cmdline;
     /* private to libxl__run_bootloader */
     char *outputpath, *outputdir, *logfile;
     char *diskpath; /* not from gc, represents actually attached disk */
@@ -1803,7 +1823,6 @@ struct libxl__domain_create_state {
          * for the non-stubdom device model. */
 };
 
-
 /*
  * Convenience macros.
  */
diff -r ac45608496cd -r cdb947baea10 tools/libxl/libxl_json.c
--- a/tools/libxl/libxl_json.c	Thu May 17 16:39:51 2012 +0100
+++ b/tools/libxl/libxl_json.c	Thu May 17 17:51:32 2012 +0100
@@ -252,15 +252,6 @@ out:
     return s;
 }
 
-yajl_gen_status libxl_file_reference_gen_json(yajl_gen hand,
-                                              libxl_file_reference *p)
-{
-    if (p->path)
-        return libxl__yajl_gen_asciiz(hand, p->path);
-    else
-        return yajl_gen_null(hand);
-}
-
 yajl_gen_status libxl__string_gen_json(yajl_gen hand,
                                        const char *p)
 {
diff -r ac45608496cd -r cdb947baea10 tools/libxl/libxl_json.h
--- a/tools/libxl/libxl_json.h	Thu May 17 16:39:51 2012 +0100
+++ b/tools/libxl/libxl_json.h	Thu May 17 17:51:32 2012 +0100
@@ -32,8 +32,6 @@ yajl_gen_status libxl_cpuid_policy_list_
 yajl_gen_status libxl_string_list_gen_json(yajl_gen hand, libxl_string_list *p);
 yajl_gen_status libxl_key_value_list_gen_json(yajl_gen hand,
                                               libxl_key_value_list *p);
-yajl_gen_status libxl_file_reference_gen_json(yajl_gen hand,
-                                              libxl_file_reference *p);
 yajl_gen_status libxl_hwcap_gen_json(yajl_gen hand, libxl_hwcap *p);
 
 #include <_libxl_types_json.h>
diff -r ac45608496cd -r cdb947baea10 tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl	Thu May 17 16:39:51 2012 +0100
+++ b/tools/libxl/libxl_types.idl	Thu May 17 17:51:32 2012 +0100
@@ -15,8 +15,6 @@ libxl_cpuid_policy_list = Builtin("cpuid
 
 libxl_string_list = Builtin("string_list", dispose_fn="libxl_string_list_dispose", passby=PASS_BY_REFERENCE)
 libxl_key_value_list = Builtin("key_value_list", dispose_fn="libxl_key_value_list_dispose", passby=PASS_BY_REFERENCE)
-libxl_file_reference = Builtin("file_reference", dispose_fn="libxl_file_reference_dispose", passby=PASS_BY_REFERENCE)
-
 libxl_hwcap = Builtin("hwcap", passby=PASS_BY_REFERENCE)
 
 #
@@ -235,11 +233,6 @@ libxl_sched_params = Struct("sched_param
     ("extratime",    integer),
     ], dir=DIR_IN)
 
-# Instances of libxl_file_reference contained in this struct which
-# have been mapped (with libxl_file_reference_map) will be unmapped
-# by libxl_domain_build/restore. If either of these are never called
-# then the user is responsible for calling
-# libxl_file_reference_unmap.
 libxl_domain_build_info = Struct("domain_build_info",[
     ("max_vcpus",       integer),
     ("cur_vcpus",       integer),
@@ -305,12 +298,12 @@ libxl_domain_build_info = Struct("domain
                                        ("soundhw",          string),
                                        ("xen_platform_pci", libxl_defbool),
                                        ])),
-                 ("pv", Struct(None, [("kernel", libxl_file_reference),
+                 ("pv", Struct(None, [("kernel", string),
                                       ("slack_memkb", MemKB),
                                       ("bootloader", string),
                                       ("bootloader_args", libxl_string_list),
                                       ("cmdline", string),
-                                      ("ramdisk", libxl_file_reference),
+                                      ("ramdisk", string),
                                       ("features", string, {'const': True}),
                                       # Use host's E820 for PCI passthrough.
                                       ("e820_host", libxl_defbool),
diff -r ac45608496cd -r cdb947baea10 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Thu May 17 16:39:51 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c	Thu May 17 17:51:32 2012 +0100
@@ -843,7 +843,7 @@ static void parse_config_data(const char
         char *cmdline = NULL;
         const char *root = NULL, *extra = "";
 
-        xlu_cfg_replace_string (config, "kernel", &b_info->u.pv.kernel.path, 0);
+        xlu_cfg_replace_string (config, "kernel", &b_info->u.pv.kernel, 0);
 
         xlu_cfg_get_string (config, "root", &root, 0);
         xlu_cfg_get_string (config, "extra", &extra, 0);
@@ -882,13 +882,13 @@ static void parse_config_data(const char
             exit(-ERROR_FAIL);
         }
 
-        if (!b_info->u.pv.bootloader && !b_info->u.pv.kernel.path) {
+        if (!b_info->u.pv.bootloader && !b_info->u.pv.kernel) {
             fprintf(stderr, "Neither kernel nor bootloader specified\n");
             exit(1);
         }
 
         b_info->u.pv.cmdline = cmdline;
-        xlu_cfg_replace_string (config, "ramdisk", &b_info->u.pv.ramdisk.path, 0);
+        xlu_cfg_replace_string (config, "ramdisk", &b_info->u.pv.ramdisk, 0);
         break;
     }
     default:
diff -r ac45608496cd -r cdb947baea10 tools/libxl/xl_sxp.c
--- a/tools/libxl/xl_sxp.c	Thu May 17 16:39:51 2012 +0100
+++ b/tools/libxl/xl_sxp.c	Thu May 17 17:51:32 2012 +0100
@@ -146,9 +146,9 @@ void printf_info_sexp(int domid, libxl_d
         break;
     case LIBXL_DOMAIN_TYPE_PV:
         printf("\t\t(linux %d)\n", 0);
-        printf("\t\t\t(kernel %s)\n", b_info->u.pv.kernel.path);
+        printf("\t\t\t(kernel %s)\n", b_info->u.pv.kernel);
         printf("\t\t\t(cmdline %s)\n", b_info->u.pv.cmdline);
-        printf("\t\t\t(ramdisk %s)\n", b_info->u.pv.ramdisk.path);
+        printf("\t\t\t(ramdisk %s)\n", b_info->u.pv.ramdisk);
         printf("\t\t\t(e820_host %s)\n",
                libxl_defbool_to_string(b_info->u.pv.e820_host));
         printf("\t\t)\n");
diff -r ac45608496cd -r cdb947baea10 tools/python/xen/lowlevel/xl/xl.c
--- a/tools/python/xen/lowlevel/xl/xl.c	Thu May 17 16:39:51 2012 +0100
+++ b/tools/python/xen/lowlevel/xl/xl.c	Thu May 17 17:51:32 2012 +0100
@@ -243,11 +243,6 @@ int attrib__libxl_cpumap_set(PyObject *v
     return 0;
 }
 
-int attrib__libxl_file_reference_set(PyObject *v, libxl_file_reference *pptr)
-{
-    return genwrap__string_set(v, &pptr->path);
-}
-
 int attrib__libxl_hwcap_set(PyObject *v, libxl_hwcap *pptr)
 {
     PyErr_SetString(PyExc_NotImplementedError, "Setting hwcap");
@@ -315,11 +310,6 @@ PyObject *attrib__libxl_cpumap_get(libxl
     return cpulist;
 }
 
-PyObject *attrib__libxl_file_reference_get(libxl_file_reference *pptr)
-{
-    return genwrap__string_get(&pptr->path);
-}
-
 PyObject *attrib__libxl_hwcap_get(libxl_hwcap *pptr)
 {
     PyErr_SetString(PyExc_NotImplementedError, "Getting hwcap");

^ permalink raw reply	[flat|nested] 9+ messages in thread