[Xen-changelog] [xen-unstable] tools: libxl: hide selection of hvmloader by default.

[Xen patchbot-unstable <patchbot@xxxxxxx>]

# HG changeset patch
# User Ian Campbell <ian.campbell@xxxxxxxxxx>
# Date 1303200984 -3600
# Node ID 0710f53cef4a0622ca44c4591ac07fb0ee008270
# Parent  7745109e3c6ee130df5f58a1893801a87a9b57c9
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@xxxxxxxxxx>
Acked-by: Ian Jackson <ian.jackson.citrix.com>
Committed-by: Ian Jackson <ian.jackson.citrix.com>
---


diff -r 7745109e3c6e -r 0710f53cef4a tools/libxl/libxl.idl
--- a/tools/libxl/libxl.idl     Wed Apr 27 13:36:15 2011 +0100
+++ b/tools/libxl/libxl.idl     Tue Apr 19 09:16:24 2011 +0100
@@ -95,12 +95,12 @@
     ("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 @@
                                         ("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 -r 7745109e3c6e -r 0710f53cef4a tools/libxl/libxl_bootloader.c
--- a/tools/libxl/libxl_bootloader.c    Wed Apr 27 13:36:15 2011 +0100
+++ b/tools/libxl/libxl_bootloader.c    Tue Apr 19 09:16:24 2011 +0100
@@ -44,8 +44,9 @@
 
     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 @@
 {
     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 -r 7745109e3c6e -r 0710f53cef4a tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c        Wed Apr 27 13:36:15 2011 +0100
+++ b/tools/libxl/libxl_create.c        Tue Apr 19 09:16:24 2011 +0100
@@ -84,8 +84,8 @@
     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 @@
         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 @@
         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 @@
     }
 
 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 -r 7745109e3c6e -r 0710f53cef4a tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c    Wed Apr 27 13:36:15 2011 +0100
+++ b/tools/libxl/libxl_dm.c    Tue Apr 19 09:16:24 2011 +0100
@@ -535,7 +535,8 @@
     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 -r 7745109e3c6e -r 0710f53cef4a tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c   Wed Apr 27 13:36:15 2011 +0100
+++ b/tools/libxl/libxl_dom.c   Tue Apr 19 09:16:24 2011 +0100
@@ -160,13 +160,17 @@
         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 @@
     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 -r 7745109e3c6e -r 0710f53cef4a tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c  Wed Apr 27 13:36:15 2011 +0100
+++ b/tools/libxl/xl_cmdimpl.c  Tue Apr 19 09:16:24 2011 +0100
@@ -337,7 +337,7 @@
     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 @@
         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 @@
     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 @@
         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 @@
         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);
         }

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog