tools: libxl: hide selection of hvmloader by default.

This should never have been exposed to users as something they are
required to think about, unless they want to.

At the libxl API level:
      * Move libxl_domain_build_info.kernel into the PV side of the
        tagged union (using this field to specify both PV kernel and
        hvmloader is confusing)
      * Add hvmloader (a string) to the HVM side of the tagged union.
        This defaults to NULL and libxl will DTRT with that default
        but still allow libxl users to specify something explicit if
        they want.

At the xl level:
      * WARN if an HVM guest cfg uses the "kernel" config option, and
        direct users to the "hvmloader_override" option if they really
        do not want the default hvmloader.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson.citrix.com>
Committed-by: Ian Jackson <ian.jackson.citrix.com>

6 files changed, 47 insertions, 31 deletions

diff --git a/tools/libxl/libxl.idl b/tools/libxl/libxl.idl
index d342e48274..9450ee72c3 100644
--- a/tools/libxl/libxl.idl
+++ b/tools/libxl/libxl.idl
@@ -95,12 +95,12 @@ libxl_domain_build_info = Struct("domain_build_info",[
     ("video_memkb",     uint32),
     ("shadow_memkb",    uint32),
     ("disable_migrate", bool),
-    ("kernel",          libxl_file_reference),
     ("cpuid",           libxl_cpuid_policy_list),
     ("hvm",             integer),
     ("u", KeyedUnion(None, "hvm",
                 [("hvm", "%s", Struct(None,
-                                       [("pae", bool),
+                                       [("hvmloader", string),
+                                        ("pae", bool),
                                         ("apic", bool),
                                         ("acpi", bool),
                                         ("nx", bool),
@@ -112,7 +112,8 @@ libxl_domain_build_info = Struct("domain_build_info",[
                                         ("nested_hvm", bool),
                                         ])),
                  ("pv", "!%s", Struct(None,
-                                       [("slack_memkb", uint32),
+                                       [("kernel", libxl_file_reference),
+                                        ("slack_memkb", uint32),
                                         ("bootloader", string),
                                         ("bootloader_args", string),
                                         ("cmdline", string),
diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c
index 133a11e486..3eb0343bf9 100644
--- a/tools/libxl/libxl_bootloader.c
+++ b/tools/libxl/libxl_bootloader.c
@@ -44,8 +44,9 @@ static char **make_bootloader_args(libxl__gc *gc,
 
     flexarray_set(args, nr++, (char *)info->u.pv.bootloader);
 
-    if (info->kernel.path)
-        flexarray_set(args, nr++, libxl__sprintf(gc, "--kernel=%s", info->kernel.path));
+    if (info->u.pv.kernel.path)
+        flexarray_set(args, nr++, 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));
     if (info->u.pv.cmdline && *info->u.pv.cmdline != '\0')
@@ -277,10 +278,10 @@ static void parse_bootloader_result(libxl__gc *gc,
 {
     while (*o != '\0') {
         if (strncmp("kernel ", o, strlen("kernel ")) == 0) {
-            free(info->kernel.path);
-            info->kernel.path = strdup(o + strlen("kernel "));
-            libxl__file_reference_map(&info->kernel);
-            unlink(info->kernel.path);
+            free(info->u.pv.kernel.path);
+            info->u.pv.kernel.path = strdup(o + strlen("kernel "));
+            libxl__file_reference_map(&info->u.pv.kernel);
+            unlink(info->u.pv.kernel.path);
         } else if (strncmp("ramdisk ", o, strlen("ramdisk ")) == 0) {
             free(info->u.pv.ramdisk.path);
             info->u.pv.ramdisk.path = strdup(o + strlen("ramdisk "));
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index bd9ebeb8da..6dd75bb0fb 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -84,8 +84,8 @@ void libxl_init_build_info(libxl_domain_build_info *b_info, libxl_domain_create_
     b_info->shadow_memkb = 0;
     if (c_info->hvm) {
         b_info->video_memkb = 8 * 1024;
-        b_info->kernel.path = strdup("hvmloader");
         b_info->hvm = 1;
+        b_info->u.hvm.hvmloader = NULL;
         b_info->u.hvm.pae = 1;
         b_info->u.hvm.apic = 1;
         b_info->u.hvm.acpi = 1;
@@ -178,7 +178,7 @@ int libxl__domain_build(libxl__gc *gc, libxl_domain_build_info *info, uint32_t d
         vments[i++] = "image/ostype";
         vments[i++] = "linux";
         vments[i++] = "image/kernel";
-        vments[i++] = (char*) info->kernel.path;
+        vments[i++] = (char*) info->u.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) {
@@ -228,7 +228,7 @@ 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->kernel.path;
+        vments[i++] = (char*) info->u.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) {
@@ -252,9 +252,10 @@ static int domain_restore(libxl__gc *gc, libxl_domain_build_info *info,
     }
 
 out:
-    libxl__file_reference_unmap(&info->kernel);
-    if (!info->hvm)
-	    libxl__file_reference_unmap(&info->u.pv.ramdisk);
+    if (!info->hvm) {
+        libxl__file_reference_unmap(&info->u.pv.kernel);
+        libxl__file_reference_unmap(&info->u.pv.ramdisk);
+    }
 
     esave = errno;
 
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 1d7d6d2e09..07b5756eb1 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -535,7 +535,8 @@ static int libxl__create_stubdom(libxl__gc *gc,
     b_info.max_vcpus = 1;
     b_info.max_memkb = 32 * 1024;
     b_info.target_memkb = b_info.max_memkb;
-    b_info.kernel.path = libxl__abs_path(gc, "ioemu-stubdom.gz", libxl_xenfirmwaredir_path());
+    b_info.u.pv.kernel.path = libxl__abs_path(gc, "ioemu-stubdom.gz",
+                                              libxl_xenfirmwaredir_path());
     b_info.u.pv.cmdline = libxl__sprintf(gc, " -d %d", info->domid);
     b_info.u.pv.ramdisk.path = "";
     b_info.u.pv.features = "";
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index ba84d0ad13..5a8eecdc6e 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -160,13 +160,17 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid,
         return ERROR_FAIL;
     }
 
-    if (info->kernel.mapped) {
-        if ( (ret = xc_dom_kernel_mem(dom, info->kernel.data, info->kernel.size)) != 0) {
+    if (info->u.pv.kernel.mapped) {
+        ret = xc_dom_kernel_mem(dom,
+                                info->u.pv.kernel.data,
+                                info->u.pv.kernel.size);
+        if ( ret != 0) {
             LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "xc_dom_kernel_mem failed");
             goto out;
         }
     } else {
-        if ( (ret = xc_dom_kernel_file(dom, info->kernel.path)) != 0) {
+        ret = xc_dom_kernel_file(dom, info->u.pv.kernel.path);
+        if ( ret != 0) {
             LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "xc_dom_kernel_file failed");
             goto out;
         }
@@ -264,25 +268,27 @@ static int hvm_build_set_params(xc_interface *handle, uint32_t domid,
     return 0;
 }
 
+static const char *libxl__domain_hvmloader(libxl__gc *gc,
+                                           libxl_domain_build_info *info)
+{
+    return libxl__abs_path(gc,
+                           info->u.hvm.hvmloader ? : "hvmloader",
+                           libxl_xenfirmwaredir_path());
+}
+
 int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
               libxl_domain_build_info *info, libxl_domain_build_state *state)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
     int ret, rc = ERROR_INVAL;
 
-    if (info->kernel.mapped) {
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "libxl__build_hvm kernel cannot be mmapped");
-        goto out;
-    }
-
     rc = ERROR_FAIL;
     ret = xc_hvm_build_target_mem(
         ctx->xch,
         domid,
         (info->max_memkb - info->video_memkb) / 1024,
         (info->target_memkb - info->video_memkb) / 1024,
-        libxl__abs_path(gc, (char *)info->kernel.path,
-                       libxl_xenfirmwaredir_path()));
+        libxl__domain_hvmloader(gc, info));
     if (ret) {
         LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, ret, "hvm building failed");
         goto out;
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index b64fcc8ec3..916801e3f4 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -337,7 +337,7 @@ static void printf_info(int domid,
     printf("\t(image\n");
     if (c_info->hvm) {
         printf("\t\t(hvm\n");
-        printf("\t\t\t(loader %s)\n", b_info->kernel.path);
+        printf("\t\t\t(loader %s)\n", b_info->u.hvm.hvmloader);
         printf("\t\t\t(video_memkb %d)\n", b_info->video_memkb);
         printf("\t\t\t(shadow_memkb %d)\n", b_info->shadow_memkb);
         printf("\t\t\t(pae %d)\n", b_info->u.hvm.pae);
@@ -370,7 +370,7 @@ static void printf_info(int domid,
         printf("\t\t)\n");
     } else {
         printf("\t\t(linux %d)\n", b_info->hvm);
-        printf("\t\t\t(kernel %s)\n", b_info->kernel.path);
+        printf("\t\t\t(kernel %s)\n", b_info->u.pv.kernel.path);
         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)\n");
@@ -740,12 +740,16 @@ static void parse_config_data(const char *configfile_filename_report,
     if (!xlu_cfg_get_long (config, "videoram", &l))
         b_info->video_memkb = l * 1024;
 
-    xlu_cfg_replace_string (config, "kernel", &b_info->kernel.path);
-
     if (!xlu_cfg_get_long (config, "gfx_passthru", &l))
         dm_info->gfx_passthru = l;
 
     if (c_info->hvm == 1) {
+        if (!xlu_cfg_get_string (config, "kernel", &buf))
+            fprintf(stderr, "WARNING: ignoring \"kernel\" directive for HVM guest. "
+                    "Use \"hvmloader_override\" instead if you really want a non-default hvmloader\n");
+
+        xlu_cfg_replace_string (config, "hvmloader_override",
+                                &b_info->u.hvm.hvmloader);
         if (!xlu_cfg_get_long (config, "pae", &l))
             b_info->u.hvm.pae = l;
         if (!xlu_cfg_get_long (config, "apic", &l))
@@ -768,6 +772,8 @@ static void parse_config_data(const char *configfile_filename_report,
         char *cmdline = NULL;
         const char *root = NULL, *extra = "";
 
+        xlu_cfg_replace_string (config, "kernel", &b_info->u.pv.kernel.path);
+
         xlu_cfg_get_string (config, "root", &root);
         xlu_cfg_get_string (config, "extra", &extra);
 
@@ -786,7 +792,7 @@ static void parse_config_data(const char *configfile_filename_report,
         xlu_cfg_replace_string (config, "bootloader", &b_info->u.pv.bootloader);
         xlu_cfg_replace_string (config, "bootloader_args", &b_info->u.pv.bootloader_args);
 
-        if (!b_info->u.pv.bootloader && !b_info->kernel.path) {
+        if (!b_info->u.pv.bootloader && !b_info->u.pv.kernel.path) {
             fprintf(stderr, "Neither kernel nor bootloader specified\n");
             exit(1);
         }