]> xenbits.xensource.com Git - xen.git/commitdiff
tools: libxl: hide selection of device-model by default.
authorIan Campbell <ian.campbell@citrix.com>
Wed, 20 Apr 2011 16:13:00 +0000 (17:13 +0100)
committerIan Campbell <ian.campbell@citrix.com>
Wed, 20 Apr 2011 16:13:00 +0000 (17:13 +0100)
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:
      * 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.
      * Add libxl_device_model_info.device_model_stubdomain allowing
        the user to select stub or non-stub device model
      * Default both the device_model field 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:
      * 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.
      * Support a new "device_model_stubdomain_override" option which
        allows the user to request stubdomain if desired.
      * WARN if an HVM guest cfg uses the "device_model" config
        option, and direct users to the "device_model_override" option
        if they really do not want the default. If the "device_model"
        directive contains "stubdom-db" then direct users to the
        "device_model_stubdomain_override" directive.

The default qemu remains the existing qemu-xen based qemu-dm and
stubdomain defaults to off. I chose the name "qemu-xen traditional" to
refer to the existing Xen fork of qemu and simply "qemu-xen" to refer to
the new device model based on qemu upstream.

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 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>
Acked-by: Ian Jackson <ian.jackson.citrix.com>
Committed-by: Ian Jackson <ian.jackson.citrix.com>
tools/libxl/libxl.c
tools/libxl/libxl.h
tools/libxl/libxl.idl
tools/libxl/libxl_create.c
tools/libxl/libxl_dm.c
tools/libxl/libxl_internal.h
tools/libxl/libxl_utils.c
tools/libxl/libxl_utils.h
tools/libxl/xl_cmdimpl.c

index b7c0bb5999ce664f9109083895f1538b357a9883..73dafcfb04b9c1f33f4693f21f233541f97349d8 100644 (file)
@@ -2070,15 +2070,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 (dm_info->device_model_stubdomain)
             *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;
 }
 
index a7c52dc5eeb48023fd4547b691e87e7297c8f265..8a34a92ef81b8e6e2bc1577b41484ad4818df16a 100644 (file)
@@ -166,6 +166,13 @@ typedef enum {
     XENPV,
 } libxl_qemu_machine_type;
 
+typedef enum libxl_device_model_version {
+    /* Historical qemu-xen device model (qemu-dm) */
+    LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL = 1,
+    /* Upstream based qemu-xen device model */
+    LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN = 2,
+} libxl_device_model_version;
+
 typedef enum {
     LIBXL_CONSTYPE_SERIAL = 1,
     LIBXL_CONSTYPE_PV,
index 9450ee72c3b5982e99393541c075fd9f1f84d9ed..0e47f297da1552f2c0bfb7981046f5607987db17 100644 (file)
@@ -9,6 +9,7 @@ libxl_mac = Builtin("mac")
 libxl_cpumap = Builtin("cpumap", destructor_fn="libxl_cpumap_destroy", passby=PASS_BY_REFERENCE)
 libxl_cpuarray = Builtin("cpuarray", destructor_fn="libxl_cpuarray_destroy", passby=PASS_BY_REFERENCE)
 libxl_qemu_machine_type = Number("qemu_machine_type", namespace="libxl_")
+libxl_device_model_version = Number("device_model_version", namespace="libxl_")
 libxl_console_consback = Number("console_consback", namespace="libxl_")
 libxl_console_constype = Number("console_constype", namespace="libxl_")
 libxl_disk_format = Number("disk_format", namespace="libxl_")
@@ -140,7 +141,9 @@ libxl_device_model_info = Struct("device_model_info",[
     ("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", libxl_device_model_version),
+    ("device_model_stubdomain", bool),
+    ("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),
index 6dd75bb0fba4bb79d1e22af282c414da73b22476..51dbcce41038adeb82ea59efdca02f4ac3b8fc0a 100644 (file)
@@ -108,7 +108,9 @@ void libxl_init_dm_info(libxl_device_model_info *dm_info,
     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_QEMU_XEN_TRADITIONAL;
+    dm_info->device_model_stubdomain = false;
+    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;
index 07b5756eb14718f990d974b58436375425d09b8c..c3c0312e4eb166d1f5b1b2d0341f4cd4b87d163c 100644 (file)
@@ -38,7 +38,39 @@ static const char *libxl_tapif_script(libxl__gc *gc)
 #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_stubdomain)
+        return NULL;
+
+    if (info->device_model) {
+        dm = libxl__strdup(gc, info->device_model);
+    } else {
+        switch (info->device_model_version) {
+        case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
+            dm = libxl__abs_path(gc, "qemu-dm", libxl_libexec_path());
+            break;
+        case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
+            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 +82,8 @@ static char ** libxl__build_device_model_args_old(libxl__gc *gc,
     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 +216,7 @@ static const char *qemu_disk_format_string(libxl_disk_format format)
 }
 
 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 +229,8 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
     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 +419,26 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
 }
 
 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 LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
+        return libxl__build_device_model_args_old(gc, dm, info,
+                                                  disks, num_disks,
+                                                  vifs, num_vifs);
+    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
+        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;
     }
 }
 
@@ -518,7 +559,13 @@ static int libxl__create_stubdom(libxl__gc *gc,
     xs_transaction_t t;
     libxl__device_model_starting *dm_starting = 0;
 
-    args = libxl__build_device_model_args(gc, info, disks, num_disks,
+    if (info->device_model_version != LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL) {
+        ret = ERROR_INVAL;
+        goto out;
+    }
+
+    args = libxl__build_device_model_args(gc, "stubdom-dm", info,
+                                          disks, num_disks,
                                           vifs, num_vifs);
     if (!args) {
         ret = ERROR_FAIL;
@@ -676,20 +723,30 @@ int libxl__create_device_model(libxl__gc *gc,
     int rc;
     char **args;
     libxl__device_model_starting buf_starting, *p;
-    xs_transaction_t t; 
+    xs_transaction_t t;
     char *vm_path;
     char **pass_stuff;
+    const char *dm;
 
-    if (strstr(info->device_model, "stubdom-dm")) {
+    if (info->device_model_stubdomain) {
         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, info,
+                                   disks, num_disks,
+                                   vifs, num_vifs,
+                                   &vfb, &vkb, starting_r);
+        goto out;
+    }
+
+    dm = libxl__domain_device_model(gc, info);
+    if (!dm) {
+        rc = ERROR_FAIL;
         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;
@@ -747,8 +804,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;
@@ -847,7 +904,8 @@ static int libxl__build_xenpv_qemu_args(libxl__gc *gc,
         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_QEMU_XEN_TRADITIONAL;
+    info->device_model = NULL;
     info->type = XENPV;
     return 0;
 }
index 256a98e831649b8b86fe809e84b67a8764358d85..6f109c1bdb097ad52ea01831586758b387bff206 100644 (file)
@@ -233,6 +233,8 @@ _hidden int libxl__domain_make(libxl__gc *gc, libxl_domain_create_info *info, ui
 _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,
index 4e59fe6365e5d875f922901a44fb402cb4b8c33e..38e9157f7377c5554eb7ec17423d7a1a5de5fd6d 100644 (file)
@@ -568,89 +568,6 @@ out:
     return rc;
 }
 
-#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;
index a1821f3e4f1d3319e92c0ec3948e462c05dc3e1f..a12c4c02e900fdcedc569199515f7198285a095c 100644 (file)
@@ -66,12 +66,6 @@ int libxl_devid_to_device_nic(libxl_ctx *ctx, uint32_t domid,
 int libxl_devid_to_device_disk(libxl_ctx *ctx, uint32_t domid,
                                const char *devid, libxl_device_disk *disk);
 
-/* 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);
index 916801e3f4783559ff326416705e36eb25f19cc1..00f53ead8e4cf619798a3fd432a93c1aa33089df 100644 (file)
@@ -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);
@@ -774,6 +774,8 @@ static void parse_config_data(const char *configfile_filename_report,
 
         xlu_cfg_replace_string (config, "kernel", &b_info->u.pv.kernel.path);
 
+        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);
 
@@ -1079,7 +1081,32 @@ 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.\n"
+                    "WARNING: Use \"device_model_override\" instead if you really want a non-default device_model\n");
+            if (strstr(buf, "stubdom-dm"))
+                fprintf(stderr, "WARNING: Or use \"device_model_stubdomain_override\" if you want to enable stubdomains\n");
+        }
+
+        xlu_cfg_replace_string (config, "device_model_override",
+                                &dm_info->device_model);
+        if (!xlu_cfg_get_string (config, "device_model_version", &buf)) {
+            if (!strcmp(buf, "qemu-xen-traditional")) {
+                dm_info->device_model_version
+                    = LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL;
+            } else if (!strcmp(buf, "qemu-xen")) {
+                dm_info->device_model_version
+                    = LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
+            } else {
+                fprintf(stderr,
+                        "Unknown device_model_version \"%s\" specified\n", buf);
+                exit(1);
+            }
+        } else if (dm_info->device_model)
+            fprintf(stderr, "WARNING: device model override given without specific DM version\n");
+        if (!xlu_cfg_get_long (config, "device_model_stubdomain_override", &l))
+            dm_info->device_model_stubdomain = l;
         if (!xlu_cfg_get_long (config, "stdvga", &l))
             dm_info->stdvga = l;
         if (!xlu_cfg_get_long (config, "vnc", &l))