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>

1 files changed, 29 insertions, 17 deletions

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). */