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>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
[ ijc -- reduced log message in libxl__build_pv from INFO to DEBUG ]
Committed-by: Ian Campbell <ian.campbell@citrix.com>

15 files changed, 130 insertions, 179 deletions

diff --git a/tools/libxl/gentest.py b/tools/libxl/gentest.py
index 04248ee8b1..233defa640 100644
--- a/tools/libxl/gentest.py
+++ b/tools/libxl/gentest.py
@@ -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_init(libxl_cpuid_policy_list *pp)
     *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 --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index e3d05c2150..0e281a635d 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -3665,12 +3665,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 --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 316d290579..48c87d3eff 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -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]
@@ -421,7 +409,8 @@ enum {
  * of course check the rc value for errors.
  *
  * *ao_how does not need to remain valid after the initiating function
- * returns.
+ * returns. All other parameters must remain valid for the lifetime of
+ * the asynchronous operation, unless otherwise specified.
  *
  * Callbacks may occur on any thread in which the application calls
  * libxl.
@@ -543,27 +532,6 @@ int libxl_domain_preserve(libxl_ctx *ctx, uint32_t domid, libxl_domain_create_in
 /* 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 --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c
index f3a590be59..6d83bb561e 100644
--- a/tools/libxl/libxl_bootloader.c
+++ b/tools/libxl/libxl_bootloader.c
@@ -43,7 +43,8 @@ static void bootloader_arg(libxl__bootloader_state *bl, const char *arg)
     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__gc *gc, libxl__bootloader_state *bl)
 
 #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__egc *egc,
     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__egc *egc,
 #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 *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 *egc, libxl__bootloader_state *bl)
         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 = libxl__strdup(NULL, 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__egc *egc, libxl__ev_child *child,
     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 --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 14721eb50a..e5999c0a20 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -242,6 +242,7 @@ static int init_console_info(libxl__device_console *console, int dev_num)
         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, libxl_domain_build_info *info,
         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, libxl_domain_build_info *info,
 
 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__egc *egc,
     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,11 @@ static void domcreate_bootloader_done(libxl__egc *egc,
 
     if (ret) goto error_out;
 
+    /* consume bootloader outputs. state->pv_{kernel,ramdisk} have
+     * been initialised by the bootloader already.
+     */
+    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 --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 5bf9a0b14b..d987347d0b 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -715,10 +715,6 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
     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 *egc, libxl__stub_dm_spawn_state *sdss)
     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 --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index e42730f846..06dbc926f3 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -240,36 +240,37 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid,
 
     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(DEBUG, "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, uint32_t domid,
     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 --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
index b89aef71ce..8139520b78 100644
--- a/tools/libxl/libxl_internal.c
+++ b/tools/libxl/libxl_internal.c
@@ -216,7 +216,7 @@ char *libxl__abs_path(libxl__gc *gc, const char *s, const char *path)
 }
 
 
-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 --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 52f54353e8..bbc21496a1 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -721,6 +721,20 @@ int libxl__self_pipe_eatall(int fd); /* returns 0 or -1 setting errno */
 _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);
@@ -739,6 +753,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,
@@ -1273,9 +1291,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 */
@@ -1792,9 +1807,17 @@ 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:
+     *  - caller must initialise kernel and ramdisk to point to file
+     *    references, these will be updated and mapped;
+     *  - caller must initialise cmdline to NULL, it will be updated with a
+     *    string allocated from the gc;
+     */
+    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 */
@@ -1838,7 +1861,6 @@ struct libxl__domain_create_state {
          * for the non-stubdom device model. */
 };
 
-
 /*
  * Convenience macros.
  */
diff --git a/tools/libxl/libxl_json.c b/tools/libxl/libxl_json.c
index 7c068d3e81..f91ecae4ed 100644
--- a/tools/libxl/libxl_json.c
+++ b/tools/libxl/libxl_json.c
@@ -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 --git a/tools/libxl/libxl_json.h b/tools/libxl/libxl_json.h
index 0e3836e5f2..a9721a7007 100644
--- a/tools/libxl/libxl_json.h
+++ b/tools/libxl/libxl_json.h
@@ -32,8 +32,6 @@ yajl_gen_status libxl_cpuid_policy_list_gen_json(yajl_gen hand,
 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 --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index a21bd85fa8..5b81ee978d 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -15,8 +15,6 @@ libxl_cpuid_policy_list = Builtin("cpuid_policy_list", dispose_fn="libxl_cpuid_d
 
 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_params",[
     ("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_build_info",[
                                        ("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 --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index dbe5633c74..8c8d02b7b7 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -844,7 +844,7 @@ static void parse_config_data(const char *config_source,
         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);
@@ -883,13 +883,13 @@ static void parse_config_data(const char *config_source,
             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 --git a/tools/libxl/xl_sxp.c b/tools/libxl/xl_sxp.c
index c68b6df698..caee1a92c5 100644
--- a/tools/libxl/xl_sxp.c
+++ b/tools/libxl/xl_sxp.c
@@ -146,9 +146,9 @@ void printf_info_sexp(int domid, libxl_domain_config *d_config)
         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 --git a/tools/python/xen/lowlevel/xl/xl.c b/tools/python/xen/lowlevel/xl/xl.c
index a729e040ac..1db777dce3 100644
--- a/tools/python/xen/lowlevel/xl/xl.c
+++ b/tools/python/xen/lowlevel/xl/xl.c
@@ -243,11 +243,6 @@ int attrib__libxl_cpumap_set(PyObject *v, libxl_cpumap *pptr)
     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_cpumap *pptr)
     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");