[PATCH 17 of 19] tools: libxl: hide selection of device-model, hvmloader and BIOS by default

[Ian Campbell <ian.campbell@citrix.com>]

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1302607225 -3600
# Node ID 20dcc3b31fd416636a69dc7c528ef486adc9dacc
# Parent  7bddcc4cfc8789cffe511b0b38645479d8d3490d
tools: libxl: hide selection of device-model, hvmloader and BIOS by default.

These 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.
      * Add libxl_device_model_info.device_model_version allowing the
        user to say which qemu version (e.g. old qemu-xen or qemu
        upstream) they want for a domain.
      * Default both the hvmloader and device_model fields to NULL and
        DTRT when building a domain with that value in those fields,
        but still allow libxl users to specify something explicit if
        they want.
      * Note that libxl_device_model_info.device_model, if specified,
        must now be a complete path.

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.
      * WARN if an HVM guest cfg uses the "device_model" config option,
        and again direct users to the "device_model_override" option if
        they really do not want the default.
      * Support a new "device_model_version" option which sets the new
        libxl_device_model_info.device_model_version field. This
        option is mandatory if device_model_override is used

(other toolstacks would do something similar presumably)

The default qemu remains the existing qemu-xen based qemu-dm.

I suspect that the vast majority of users only have these config options
because they've copied them from somewhere and they normally have no
interest in which specific hvmloader or device model is used. Renaming
the fields and warning when they are used makes these decisions
internal. This will allow us to make decisions at a platform level
regarding the preferred hvmloader, device model, stub domain etc without
requiring everyone to change their configuration files.

Adding a device model version to the API is intended to make it easy
for users to select what they need without having to know about the
paths to specific binaries etc. Most importantly it gets rid of the
parsing of the output of qemu -h...

It's not clear where upstream qemu will eventually be installed, I
went with /usr/bin/qemu for now.

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

diff -r 7bddcc4cfc87 -r 20dcc3b31fd4 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Tue Apr 12 11:23:27 2011 +0100
+++ b/tools/libxl/libxl.c	Tue Apr 12 12:20:25 2011 +0100
@@ -2276,15 +2276,17 @@ out:
 int libxl_domain_need_memory(libxl_ctx *ctx, libxl_domain_build_info *b_info,
         libxl_device_model_info *dm_info, uint32_t *need_memkb)
 {
+    libxl__gc gc = LIBXL_INIT_GC(ctx);
     *need_memkb = b_info->target_memkb;
     if (b_info->hvm) {
         *need_memkb += b_info->shadow_memkb + LIBXL_HVM_EXTRA_MEMORY;
-        if (strstr(dm_info->device_model, "stubdom-dm"))
+        if (strstr(libxl__domain_device_model(&gc, dm_info), "stubdom-dm"))
             *need_memkb += 32 * 1024;
     } else
         *need_memkb += b_info->shadow_memkb + LIBXL_PV_EXTRA_MEMORY;
     if (*need_memkb % (2 * 1024))
         *need_memkb += (2 * 1024) - (*need_memkb % (2 * 1024));
+    libxl__free_all(&gc);
     return 0;
 }
 
diff -r 7bddcc4cfc87 -r 20dcc3b31fd4 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Tue Apr 12 11:23:27 2011 +0100
+++ b/tools/libxl/libxl.h	Tue Apr 12 12:20:25 2011 +0100
@@ -257,6 +257,12 @@ typedef enum libxl_action_on_shutdown {
     LIBXL_ACTION_COREDUMP_RESTART,
 } libxl_action_on_shutdown;
 
+/*
+ * 1 - Historical qemu-xen device model (qemu-dm)
+ * 2 - Upstream qemu
+ */
+#define LIBXL_DEVICE_MODEL_VERSION_DEFAULT 1
+
 typedef struct {
     libxl_domain_create_info c_info;
     libxl_domain_build_info b_info;
diff -r 7bddcc4cfc87 -r 20dcc3b31fd4 tools/libxl/libxl.idl
--- a/tools/libxl/libxl.idl	Tue Apr 12 11:23:27 2011 +0100
+++ b/tools/libxl/libxl.idl	Tue Apr 12 12:20:25 2011 +0100
@@ -95,12 +95,12 @@ libxl_domain_build_info = Struct("domain
     ("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
                                         ("nested_hvm", bool),
                                         ])),
                  ("pv", "!%s", Struct(None,
-                                       [("slack_memkb", uint32),
+                                       [("kernel", libxl_file_reference),
+                                        ("slack_memkb", uint32),
                                         ("bootloader", string),
                                         ("bootloader_args", string),
                                         ("cmdline", string),
@@ -139,7 +140,8 @@ libxl_device_model_info = Struct("device
     ("domid",            integer),
     ("uuid",             libxl_uuid,  False, "this is use only with stubdom, and must be different from the domain uuid"),
     ("dom_name",         string),
-    ("device_model",     string),
+    ("device_model_version", integer),
+    ("device_model",     string, False, "if you set this you must set device_model_version too"),
     ("saved_state",      string),
     ("type",             libxl_qemu_machine_type),
     ("target_ram",       uint32),
diff -r 7bddcc4cfc87 -r 20dcc3b31fd4 tools/libxl/libxl_bootloader.c
--- a/tools/libxl/libxl_bootloader.c	Tue Apr 12 11:23:27 2011 +0100
+++ b/tools/libxl/libxl_bootloader.c	Tue Apr 12 12:20:25 2011 +0100
@@ -44,8 +44,9 @@ static char **make_bootloader_args(libxl
 
     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(libx
 {
     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 7bddcc4cfc87 -r 20dcc3b31fd4 tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c	Tue Apr 12 11:23:27 2011 +0100
+++ b/tools/libxl/libxl_create.c	Tue Apr 12 12:20:25 2011 +0100
@@ -88,8 +88,8 @@ void libxl_init_build_info(libxl_domain_
     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;
@@ -112,7 +112,8 @@ void libxl_init_dm_info(libxl_device_mod
     libxl_uuid_generate(&dm_info->uuid);
 
     dm_info->dom_name = strdup(c_info->name);
-    dm_info->device_model = strdup("qemu-dm");
+    dm_info->device_model_version = LIBXL_DEVICE_MODEL_VERSION_DEFAULT;
+    dm_info->device_model = NULL;
     dm_info->target_ram = libxl__sizekb_to_mb(b_info->target_memkb);
     dm_info->videoram = libxl__sizekb_to_mb(b_info->video_memkb);
     dm_info->apic = b_info->u.hvm.apic;
@@ -182,7 +183,7 @@ int libxl__domain_build(libxl__gc *gc, l
         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) {
@@ -232,7 +233,7 @@ static int domain_restore(libxl__gc *gc,
         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) {
@@ -256,9 +257,10 @@ static int domain_restore(libxl__gc *gc,
     }
 
 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 7bddcc4cfc87 -r 20dcc3b31fd4 tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c	Tue Apr 12 11:23:27 2011 +0100
+++ b/tools/libxl/libxl_dm.c	Tue Apr 12 12:20:25 2011 +0100
@@ -38,7 +38,36 @@ static const char *libxl_tapif_script(li
 #endif
 }
 
+const char *libxl__domain_device_model(libxl__gc *gc,
+                                       libxl_device_model_info *info)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    const char *dm;
+
+    if (info->device_model) {
+        dm = libxl__strdup(gc, info->device_model);
+    } else {
+        switch (info->device_model_version) {
+        case 1:
+            dm = libxl__abs_path(gc, "qemu-dm", libxl_libexec_path());
+            break;
+        case 2:
+            dm = libxl__strdup(gc, "/usr/bin/qemu");
+            break;
+        default:
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                       "invalid device model version %d\n",
+                       info->device_model_version);
+            dm = NULL;
+            break;
+        }
+    }
+
+    return dm;
+}
+
 static char ** libxl__build_device_model_args_old(libxl__gc *gc,
+                                                  const char *dm,
                                                   libxl_device_model_info *info,
                                                   libxl_device_disk *disks, int num_disks,
                                                   libxl_device_nic *vifs, int num_vifs)
@@ -50,7 +79,8 @@ static char ** libxl__build_device_model
     if (!dm_args)
         return NULL;
 
-    flexarray_vappend(dm_args, "qemu-dm", "-d", libxl__sprintf(gc, "%d", info->domid), NULL);
+    flexarray_vappend(dm_args, dm,
+                      "-d", libxl__sprintf(gc, "%d", info->domid), NULL);
 
     if (info->dom_name)
         flexarray_vappend(dm_args, "-domain-name", info->dom_name, NULL);
@@ -183,6 +213,7 @@ static const char *qemu_disk_format_stri
 }
 
 static char ** libxl__build_device_model_args_new(libxl__gc *gc,
+                                                  const char *dm,
                                                   libxl_device_model_info *info,
                                                   libxl_device_disk *disks, int num_disks,
                                                   libxl_device_nic *vifs, int num_vifs)
@@ -195,8 +226,8 @@ static char ** libxl__build_device_model
     if (!dm_args)
         return NULL;
 
-    flexarray_vappend(dm_args, libxl__strdup(gc, info->device_model),
-                        "-xen-domid", libxl__sprintf(gc, "%d", info->domid), NULL);
+    flexarray_vappend(dm_args, dm,
+                      "-xen-domid", libxl__sprintf(gc, "%d", info->domid), NULL);
 
     if (info->type == XENPV) {
         flexarray_append(dm_args, "-xen-attach");
@@ -385,19 +416,26 @@ static char ** libxl__build_device_model
 }
 
 static char ** libxl__build_device_model_args(libxl__gc *gc,
+                                              const char *dm,
                                               libxl_device_model_info *info,
                                               libxl_device_disk *disks, int num_disks,
                                               libxl_device_nic *vifs, int num_vifs)
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
-    int new_qemu;
 
-    new_qemu = libxl_check_device_model_version(ctx, info->device_model);
-
-    if (new_qemu == 1) {
-        return libxl__build_device_model_args_new(gc, info, disks, num_disks, vifs, num_vifs);
-    } else {
-        return libxl__build_device_model_args_old(gc, info, disks, num_disks, vifs, num_vifs);
+    switch (info->device_model_version) {
+    case 1:
+        return libxl__build_device_model_args_old(gc, dm, info,
+                                                  disks, num_disks,
+                                                  vifs, num_vifs);
+    case 2:
+        return libxl__build_device_model_args_new(gc, dm, info,
+                                                  disks, num_disks,
+                                                  vifs, num_vifs);
+    default:
+        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unknown device model version %d",
+                         info->device_model_version);
+        return NULL;
     }
 }
 
@@ -499,6 +537,7 @@ retry_transaction:
 }
 
 static int libxl__create_stubdom(libxl__gc *gc,
+                                 const char *dm,
                                  libxl_device_model_info *info,
                                  libxl_device_disk *disks, int num_disks,
                                  libxl_device_nic *vifs, int num_vifs,
@@ -518,7 +557,8 @@ static int libxl__create_stubdom(libxl__
     xs_transaction_t t;
     libxl__device_model_starting *dm_starting = 0;
 
-    args = libxl__build_device_model_args(gc, info, disks, num_disks,
+    args = libxl__build_device_model_args(gc, dm, info,
+                                          disks, num_disks,
                                           vifs, num_vifs);
     if (!args) {
         ret = ERROR_FAIL;
@@ -535,7 +575,8 @@ static int libxl__create_stubdom(libxl__
     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 = "";
@@ -683,17 +724,27 @@ int libxl__create_device_model(libxl__gc
     xs_transaction_t t; 
     char *vm_path;
     char **pass_stuff;
+    const char *dm;
 
-    if (strstr(info->device_model, "stubdom-dm")) {
+    dm = libxl__domain_device_model(gc, info);
+    if (!dm) {
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    if (strstr(dm, "stubdom-dm")) {
         libxl_device_vfb vfb;
         libxl_device_vkb vkb;
 
         libxl__vfb_and_vkb_from_device_model_info(gc, info, &vfb, &vkb);
-        rc = libxl__create_stubdom(gc, info, disks, num_disks, vifs, num_vifs, &vfb, &vkb, starting_r);
+        rc = libxl__create_stubdom(gc, dm, info,
+                                   disks, num_disks,
+                                   vifs, num_vifs,
+                                   &vfb, &vkb, starting_r);
         goto out;
     }
 
-    args = libxl__build_device_model_args(gc, info, disks, num_disks,
+    args = libxl__build_device_model_args(gc, dm, info, disks, num_disks,
                                           vifs, num_vifs);
     if (!args) {
         rc = ERROR_FAIL;
@@ -751,8 +802,8 @@ retry_transaction:
     if (!rc) { /* inner child */
         setsid();
         libxl__exec(null, logfile_w, logfile_w,
-                   libxl__abs_path(gc, info->device_model, libxl_libexec_path()),
-                   args);
+                    libxl__domain_device_model(gc, info),
+                    args);
     }
 
     rc = 0;
@@ -851,7 +902,8 @@ static int libxl__build_xenpv_qemu_args(
         info->nographic = 1;
     info->domid = domid;
     info->dom_name = libxl_domid_to_name(ctx, domid);
-    info->device_model = libxl__abs_path(gc, "qemu-dm", libxl_libexec_path());
+    info->device_model_version = LIBXL_DEVICE_MODEL_VERSION_DEFAULT;
+    info->device_model = NULL;
     info->type = XENPV;
     return 0;
 }
diff -r 7bddcc4cfc87 -r 20dcc3b31fd4 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c	Tue Apr 12 11:23:27 2011 +0100
+++ b/tools/libxl/libxl_dom.c	Tue Apr 12 12:20:25 2011 +0100
@@ -160,13 +160,17 @@ int libxl__build_pv(libxl__gc *gc, uint3
         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_inter
     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 7bddcc4cfc87 -r 20dcc3b31fd4 tools/libxl/libxl_internal.c
--- a/tools/libxl/libxl_internal.c	Tue Apr 12 11:23:27 2011 +0100
+++ b/tools/libxl/libxl_internal.c	Tue Apr 12 12:20:25 2011 +0100
@@ -184,10 +184,10 @@ void libxl__log(libxl_ctx *ctx, xentooll
     va_end(ap);
 }
 
-char *libxl__abs_path(libxl__gc *gc, char *s, const char *path)
+char *libxl__abs_path(libxl__gc *gc, const char *s, const char *path)
 {
     if (!s || s[0] == '/')
-        return s;
+        return libxl__strdup(gc, s);
     return libxl__sprintf(gc, "%s/%s", path, s);
 }
 
diff -r 7bddcc4cfc87 -r 20dcc3b31fd4 tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Tue Apr 12 11:23:27 2011 +0100
+++ b/tools/libxl/libxl_internal.h	Tue Apr 12 12:20:25 2011 +0100
@@ -235,6 +235,8 @@ _hidden int libxl__domain_make(libxl__gc
 _hidden int libxl__domain_build(libxl__gc *gc, libxl_domain_build_info *info, uint32_t domid, /* out */ libxl_domain_build_state *state);
 
 /* for device model creation */
+_hidden const char *libxl__domain_device_model(libxl__gc *gc,
+                                               libxl_device_model_info *info);
 _hidden int libxl__create_device_model(libxl__gc *gc,
                               libxl_device_model_info *info,
                               libxl_device_disk *disk, int num_disks,
@@ -290,7 +292,7 @@ _hidden void libxl__exec(int stdinfd, in
 _hidden void libxl__log_child_exitstatus(libxl__gc *gc,
                                 const char *what, pid_t pid, int status);
 
-_hidden char *libxl__abs_path(libxl__gc *gc, char *s, const char *path);
+_hidden char *libxl__abs_path(libxl__gc *gc, const char *s, const char *path);
 
 #define LIBXL__LOG_DEBUG   XTL_DEBUG
 #define LIBXL__LOG_INFO    XTL_INFO
diff -r 7bddcc4cfc87 -r 20dcc3b31fd4 tools/libxl/libxl_utils.c
--- a/tools/libxl/libxl_utils.c	Tue Apr 12 11:23:27 2011 +0100
+++ b/tools/libxl/libxl_utils.c	Tue Apr 12 12:20:25 2011 +0100
@@ -639,89 +639,6 @@ int libxl_strtomac(const char *mac_s, ui
     return 0;
 }
 
-#define QEMU_VERSION_STR  "QEMU emulator version "
-
-
-int libxl_check_device_model_version(libxl_ctx *ctx, char *path)
-{
-    libxl__gc gc = LIBXL_INIT_GC(ctx);
-    pid_t pid = -1;
-    int pipefd[2];
-    char buf[100];
-    ssize_t i, count = 0;
-    int status;
-    char *abs_path = NULL;
-    int rc = -1;
-
-    abs_path = libxl__abs_path(&gc, path, libxl_private_bindir_path());
-
-    if (pipe(pipefd))
-        goto out;
-
-    pid = fork();
-    if (pid == -1) {
-        goto out;
-    }
-
-    if (!pid) {
-        close(pipefd[0]);
-        if (dup2(pipefd[1], STDOUT_FILENO) == -1)
-            exit(1);
-        execlp(abs_path, abs_path, "-h", NULL);
-
-        close(pipefd[1]);
-        exit(127);
-    }
-
-    close(pipefd[1]);
-
-    /* attempt to get the first line of `qemu -h` */
-    while ((i = read(pipefd[0], buf + count, 99 - count)) > 0) {
-        if (i + count > 90)
-            break;
-        for (int j = 0; j <  i; j++) {
-            if (buf[j + count] == '\n')
-                break;
-        }
-        count += i;
-    }
-    count += i;
-    close(pipefd[0]);
-    waitpid(pid, &status, 0);
-    if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) {
-        goto out;
-    }
-
-    /* Check if we have the forked qemu-xen. */
-    /* QEMU-DM emulator version 0.10.2, ... */
-    if (strncmp("QEMU-DM ", buf, 7) == 0) {
-        rc = 0;
-        goto out;
-    }
-
-    /* Check if the version is above 12.0 */
-    /* The first line is : QEMU emulator version 0.12.50, ... */
-    if (strncmp(QEMU_VERSION_STR, buf, strlen(QEMU_VERSION_STR)) == 0) {
-        int major, minor;
-        char *endptr = NULL;
-        char *v = buf + strlen(QEMU_VERSION_STR);
-
-        major = strtol(v, &endptr, 10);
-        if (major == 0 && endptr && *endptr == '.') {
-            v = endptr + 1;
-            minor = strtol(v, &endptr, 10);
-            if (minor >= 12) {
-                rc = 1;
-                goto out;
-            }
-        }
-    }
-    rc = 0;
-out:
-    libxl__free_all(&gc);
-    return rc;
-}
-
 int libxl_cpumap_alloc(libxl_ctx *ctx, libxl_cpumap *cpumap)
 {
     int max_cpus;
diff -r 7bddcc4cfc87 -r 20dcc3b31fd4 tools/libxl/libxl_utils.h
--- a/tools/libxl/libxl_utils.h	Tue Apr 12 11:23:27 2011 +0100
+++ b/tools/libxl/libxl_utils.h	Tue Apr 12 12:20:25 2011 +0100
@@ -71,12 +71,6 @@ int libxl_strtomac(const char *mac_s, ui
 int libxl_devid_to_device_net2(libxl_ctx *ctx, uint32_t domid,
                                const char *devid, libxl_device_net2 *net2);
 
-/* check the version of qemu
- * return 1 if is the new one
- * return 0 if is the old one
- * return -1 if there are an error */
-int libxl_check_device_model_version(libxl_ctx *ctx, char *path);
-
 int libxl_cpumap_alloc(libxl_ctx *ctx, libxl_cpumap *cpumap);
 int libxl_cpumap_test(libxl_cpumap *cpumap, int cpu);
 void libxl_cpumap_set(libxl_cpumap *cpumap, int cpu);
diff -r 7bddcc4cfc87 -r 20dcc3b31fd4 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Tue Apr 12 11:23:27 2011 +0100
+++ b/tools/libxl/xl_cmdimpl.c	Tue Apr 12 12:20:25 2011 +0100
@@ -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);
@@ -350,7 +350,7 @@ static void printf_info(int domid,
         printf("\t\t\t(timer_mode %d)\n", b_info->u.hvm.timer_mode);
         printf("\t\t\t(nestedhvm %d)\n", b_info->u.hvm.nested_hvm);
 
-        printf("\t\t\t(device_model %s)\n", dm_info->device_model);
+        printf("\t\t\t(device_model %s)\n", dm_info->device_model ? : "default");
         printf("\t\t\t(videoram %d)\n", dm_info->videoram);
         printf("\t\t\t(stdvga %d)\n", dm_info->stdvga);
         printf("\t\t\t(vnc %d)\n", dm_info->vnc);
@@ -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
     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
         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
         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);
         }
@@ -1118,7 +1124,17 @@ skip_vfb:
         libxl_init_dm_info(dm_info, c_info, b_info);
 
         /* then process config related to dm */
-        xlu_cfg_replace_string (config, "device_model", &dm_info->device_model);
+        if (!xlu_cfg_get_string (config, "device_model", &buf))
+            fprintf(stderr, "WARNING: ignoring device_model directive. "
+                    "Use \"device_model_override\" instead if you really want a non-default device_model\n");
+
+        xlu_cfg_replace_string (config, "device_model_override",
+                                &dm_info->device_model);
+        if (!xlu_cfg_get_long (config, "device_model_version", &l))
+            dm_info->device_model_version = l;
+        else if (dm_info->device_model)
+            fprintf(stderr, "WARNING: device model override given without specific DM version, assuming latest\n");
+
         if (!xlu_cfg_get_long (config, "stdvga", &l))
             dm_info->stdvga = l;
         if (!xlu_cfg_get_long (config, "vnc", &l))