Allow xl to build a domain with multiple USB devices

PATCH New George Dunlap 2 participants 10 messages Nov 30, 2012 – Dec 5, 2012

Share & tools

George Dunlap

author

2012-11-30 16:24 UTC #0 lore raw

PATCH v2 1/1 Allow xl to build a domain with multiple USB devices

v2 adds two patches. The first consoldates the device model argument builder for qemu-traditional and qemu-upstream into a single function, to make it easier to keep up to date.

Along the way, the first patch checks to make sure that the vnc display is not set in both "vnclisten" and "vncdisplay"; if so, it throws an error.

The second patch enforces this in libxl, so the user gets a friendly error message rather than an ugly libxl internal error.

George Dunlap

author

2012-11-30 16:24 UTC #1 lore raw

PATCH v2 1/1 libxl: Combine device model arg build functions

# HG changeset patch
# User George Dunlap <[email protected]> # Date 1354287957 0
# Node ID 722da032ac90c0e1a78b1154fa588bf295d1f009 # Parent ae6fb202b233af815466055d9f1a635802a50855
libxl: Combine device model arg build functions

qemu-traditional and qemu-upstream have some differences in the way the arguments need to be passed. At the moment, this is dealt with by having two entirely separate functions, libxl__build_device_model_args_new and libxl__build_device_model_args_old.

However, at least 80% of these are the same; this means that fixes or additions to one may not make it into the other. Furthermore, there are some unaccounable differences in implementation.

This patch combines the two functions into one, choosing between old and new implementations as necessary.

It also makes the following changes to the vnc generation code:
* Simplifies and comments it, making it easier to read and grok
* Throws an error if duplicate values of display are set, rather than the current very un-intuitive behavior.

Signed-off-by George Dunlap <[email protected]>

tools/libxl/libxl_dm.c +168 ~~-260~~

@@ -95,181 +95,6 @@ static const char *dm_keymap(const libxl
         return NULL;
 }
 
-static char ** libxl__build_device_model_args_old(libxl__gc *gc,
-                                        const char *dm, int domid,
-                                        const libxl_domain_config *guest_config,
-                                        const libxl__domain_build_state *state)
-{
-    const libxl_domain_create_info *c_info = &guest_config->c_info;
-    const libxl_domain_build_info *b_info = &guest_config->b_info;
-    const libxl_device_nic *nics = guest_config->nics;
-    const libxl_vnc_info *vnc = libxl__dm_vnc(guest_config);
-    const libxl_sdl_info *sdl = dm_sdl(guest_config);
-    const int num_nics = guest_config->num_nics;
-    const char *keymap = dm_keymap(guest_config);
-    int i;
-    flexarray_t *dm_args;
-    dm_args = flexarray_make(gc, 16, 1);
-
-    flexarray_vappend(dm_args, dm,
-                      "-d", libxl__sprintf(gc, "%d", domid), NULL);
-
-    if (c_info->name)
-        flexarray_vappend(dm_args, "-domain-name", c_info->name, NULL);
-
-    if (vnc) {
-        char *vncarg;
-        if (vnc->display) {
-            if (vnc->listen && strchr(vnc->listen, ':') == NULL) {
-                vncarg = libxl__sprintf(gc, "%s:%d",
-                                  vnc->listen,
-                                  vnc->display);
-            } else {
-                vncarg = libxl__sprintf(gc, "127.0.0.1:%d", vnc->display);
-            }
-        } else if (vnc->listen) {
-            if (strchr(vnc->listen, ':') != NULL) {
-                vncarg = vnc->listen;
-            } else {
-                vncarg = libxl__sprintf(gc, "%s:0", vnc->listen);
-            }
-        } else {
-            vncarg = "127.0.0.1:0";
-        }
-        if (vnc->passwd && (vnc->passwd[0] != '\0'))
-            vncarg = libxl__sprintf(gc, "%s,password", vncarg);
-        flexarray_append(dm_args, "-vnc");
-        flexarray_append(dm_args, vncarg);
-
-        if (libxl_defbool_val(vnc->findunused)) {
-            flexarray_append(dm_args, "-vncunused");
-        }
-    }
-    if (sdl) {
-        flexarray_append(dm_args, "-sdl");
-        if (!libxl_defbool_val(sdl->opengl)) {
-            flexarray_append(dm_args, "-disable-opengl");
-        }
-        /* XXX sdl->{display,xauthority} into $DISPLAY/$XAUTHORITY */
-    }
-    if (keymap) {
-        flexarray_vappend(dm_args, "-k", keymap, NULL);
-    }
-    if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) {
-        int ioemu_nics = 0;
-        int nr_set_cpus = 0;
-        char *s;
-
-        if (b_info->u.hvm.serial) {
-            flexarray_vappend(dm_args, "-serial", b_info->u.hvm.serial, NULL);
-        }
-
-        if (libxl_defbool_val(b_info->u.hvm.nographic) && (!sdl && !vnc)) {
-            flexarray_append(dm_args, "-nographic");
-        }
-
-        if (b_info->video_memkb) {
-            flexarray_vappend(dm_args, "-videoram",
-                    libxl__sprintf(gc, "%d",
-                                   libxl__sizekb_to_mb(b_info->video_memkb)),
-                    NULL);
-        }
-
-        switch (b_info->u.hvm.vga.kind) {
-        case LIBXL_VGA_INTERFACE_TYPE_STD:
-            flexarray_append(dm_args, "-std-vga");
-            break;
-        case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
-            break;
-        }
-
-        if (b_info->u.hvm.boot) {
-            flexarray_vappend(dm_args, "-boot", b_info->u.hvm.boot, NULL);
-        }
-        if (libxl_defbool_val(b_info->u.hvm.usb) || b_info->u.hvm.usbdevice) {
-            flexarray_append(dm_args, "-usb");
-            if (b_info->u.hvm.usbdevice) {
-                flexarray_vappend(dm_args,
-                                  "-usbdevice", b_info->u.hvm.usbdevice, NULL);
-            }
-        }
-        if (b_info->u.hvm.soundhw) {
-            flexarray_vappend(dm_args, "-soundhw", b_info->u.hvm.soundhw, NULL);
-        }
-        if (libxl_defbool_val(b_info->u.hvm.acpi)) {
-            flexarray_append(dm_args, "-acpi");
-        }
-        if (b_info->max_vcpus > 1) {
-            flexarray_vappend(dm_args, "-vcpus",
-                              libxl__sprintf(gc, "%d", b_info->max_vcpus),
-                              NULL);
-        }
-
-        nr_set_cpus = libxl_bitmap_count_set(&b_info->avail_vcpus);
-        s = libxl_bitmap_to_hex_string(CTX, &b_info->avail_vcpus);
-        flexarray_vappend(dm_args, "-vcpu_avail",
-                              libxl__sprintf(gc, "%s", s), NULL);
-        free(s);
-
-        for (i = 0; i < num_nics; i++) {
-            if (nics[i].nictype == LIBXL_NIC_TYPE_VIF_IOEMU) {
-                char *smac = libxl__sprintf(gc,
-                                   LIBXL_MAC_FMT, LIBXL_MAC_BYTES(nics[i].mac));
-                const char *ifname = libxl__device_nic_devname(gc,
-                                                domid, nics[i].devid,
-                                                LIBXL_NIC_TYPE_VIF_IOEMU);
-                flexarray_vappend(dm_args,
-                                  "-net",
-                                  GCSPRINTF(
-                                      "nic,vlan=%d,macaddr=%s,model=%s",
-                                      nics[i].devid, smac, nics[i].model),
-                                  "-net",
-                                  GCSPRINTF(
-                                      "tap,vlan=%d,ifname=%s,bridge=%s,"
-                                      "script=%s,downscript=%s",
-                                      nics[i].devid, ifname, nics[i].bridge,
-                                      libxl_tapif_script(gc),
-                                      libxl_tapif_script(gc)),
-                                  NULL);
-                ioemu_nics++;
-            }
-        }
-        /* If we have no emulated nics, tell qemu not to create any */
-        if ( ioemu_nics == 0 ) {
-            flexarray_vappend(dm_args, "-net", "none", NULL);
-        }
-        if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) {
-            flexarray_append(dm_args, "-gfx_passthru");
-        }
-    } else {
-        if (!sdl && !vnc)
-            flexarray_append(dm_args, "-nographic");
-    }
-
-    if (state->saved_state) {
-        flexarray_vappend(dm_args, "-loadvm", state->saved_state, NULL);
-    }
-    for (i = 0; b_info->extra && b_info->extra[i] != NULL; i++)
-        flexarray_append(dm_args, b_info->extra[i]);
-    flexarray_append(dm_args, "-M");
-    switch (b_info->type) {
-    case LIBXL_DOMAIN_TYPE_PV:
-        flexarray_append(dm_args, "xenpv");
-        for (i = 0; b_info->extra_pv && b_info->extra_pv[i] != NULL; i++)
-            flexarray_append(dm_args, b_info->extra_pv[i]);
-        break;
-    case LIBXL_DOMAIN_TYPE_HVM:
-        flexarray_append(dm_args, "xenfv");
-        for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++)
-            flexarray_append(dm_args, b_info->extra_hvm[i]);
-        break;
-    default:
-        abort();
-    }
-    flexarray_append(dm_args, NULL);
-    return (char **) flexarray_contents(dm_args);
-}
-
 static const char *qemu_disk_format_string(libxl_disk_format format)
 {
     switch (format) {

@@ -318,7 +143,7 @@ static char *dm_spice_options(libxl__gc 
     return opt;
 }
 
-static char ** libxl__build_device_model_args_new(libxl__gc *gc,
+static char ** libxl__build_device_model_args(libxl__gc *gc,
                                         const char *dm, int guest_domid,
                                         const libxl_domain_config *guest_config,
                                         const libxl__domain_build_state *state)

@@ -336,62 +161,106 @@ static char ** libxl__build_device_model
     flexarray_t *dm_args;
     int i;
     uint64_t ram_size;
+    int dm_new;
+
+    switch (guest_config->b_info.device_model_version) {
+    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
+        dm_new = 0;
+        break;
+    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
+        dm_new = 1;
+        break;
+    default:
+        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unknown device model version %d",
+                         guest_config->b_info.device_model_version);
+        return NULL;
+    }
+
 
     dm_args = flexarray_make(gc, 16, 1);
 
-    flexarray_vappend(dm_args, dm,
-                      "-xen-domid",
-                      libxl__sprintf(gc, "%d", guest_domid), NULL);
+    /* Domain ID */
+    if (dm_new) {
+        flexarray_vappend(dm_args, dm,
+                          "-xen-domid",
+                          libxl__sprintf(gc, "%d", guest_domid), NULL);
+    } else {
+        flexarray_vappend(dm_args, dm,
+                          "-d", libxl__sprintf(gc, "%d", guest_domid), NULL);
+    }
 
-    flexarray_append(dm_args, "-chardev");
-    flexarray_append(dm_args,
-                     libxl__sprintf(gc, "socket,id=libxl-cmd,"
-                                    "path=%s/qmp-libxl-%d,server,nowait",
-                                    libxl__run_dir_path(), guest_domid));
+    if (dm_new) {
+        flexarray_append(dm_args, "-chardev");
+        flexarray_append(dm_args,
+                         libxl__sprintf(gc, "socket,id=libxl-cmd,"
+                                        "path=%s/qmp-libxl-%d,server,nowait",
+                                        libxl__run_dir_path(), guest_domid));
 
-    flexarray_append(dm_args, "-mon");
-    flexarray_append(dm_args, "chardev=libxl-cmd,mode=control");
+        flexarray_append(dm_args, "-mon");
+        flexarray_append(dm_args, "chardev=libxl-cmd,mode=control");
 
-    if (b_info->type == LIBXL_DOMAIN_TYPE_PV) {
-        flexarray_append(dm_args, "-xen-attach");
+        if (b_info->type == LIBXL_DOMAIN_TYPE_PV) {
+            flexarray_append(dm_args, "-xen-attach");
+        }
     }
 
     if (c_info->name) {
-        flexarray_vappend(dm_args, "-name", c_info->name, NULL);
+        if ( dm_new )
+            flexarray_vappend(dm_args, "-name", c_info->name, NULL);
+        else
+            flexarray_vappend(dm_args, "-domain-name", c_info->name, NULL);
     }
+
     if (vnc) {
-        int display = 0;
-        const char *addr = "127.0.0.1";
         char *vncarg = NULL;
 
         flexarray_append(dm_args, "-vnc");
 
-        if (vnc->display) {
-            display = vnc->display;
-            if (vnc->listen && strchr(vnc->listen, ':') == NULL) {
-                addr = vnc->listen;
+        /*
+         * If vnc->listen is present and contains a :, and
+         *  - vnc->display is 0, use vnc->listen
+         *  - vnc->display is non-zero, be confused
+         * If vnc->listen is present but doesn't, use vnc->listen:vnc->display.
+         * If vnc->listen is not present, use 127.0.0.1:vnc->display
+         * (Remembering that vnc->display already defaults to 0.)
+         */
+        if (vnc->listen) {
+            if (strchr(vnc->listen, ':') != NULL) {
+                if (vnc->display) {
+                    LOG(ERROR, "vncdisplay set, vnclisten contains display");
+                    return NULL;
+                }
+                vncarg = vnc->listen;
+            } else {
+                vncarg = libxl__sprintf(gc, "%s:%d", vnc->listen,
+                                        vnc->display);
             }
-        } else if (vnc->listen) {
-            addr = vnc->listen;
-        }
+        } else
+            vncarg = libxl__sprintf(gc, "127.0.0.1:%d", vnc->display);
 
-        if (strchr(addr, ':') != NULL)
-            vncarg = libxl__sprintf(gc, "%s", addr);
-        else
-            vncarg = libxl__sprintf(gc, "%s:%d", addr, display);
         if (vnc->passwd && vnc->passwd[0]) {
             vncarg = libxl__sprintf(gc, "%s,password", vncarg);
         }
-        if (libxl_defbool_val(vnc->findunused)) {
+
+        if (dm_new && libxl_defbool_val(vnc->findunused)) {
             /* This option asks to QEMU to try this number of port before to
              * give up.  So QEMU will try ports between $display and $display +
              * 99.  This option needs to be the last one of the vnc options. */
             vncarg = libxl__sprintf(gc, "%s,to=99", vncarg);
         }
+
         flexarray_append(dm_args, vncarg);
+
+        if (!dm_new && libxl_defbool_val(vnc->findunused)) {
+            flexarray_append(dm_args, "-vncunused");
+        }
     }
     if (sdl) {
         flexarray_append(dm_args, "-sdl");
+        
+        if (!dm_new && !libxl_defbool_val(sdl->opengl)) {
+            flexarray_append(dm_args, "-disable-opengl");
+        }
         /* XXX sdl->{display,xauthority} into $DISPLAY/$XAUTHORITY */
     }

@@ -414,7 +283,7 @@ static char ** libxl__build_device_model
             flexarray_append(dm_args, "-nographic");
         }
 
-        if (libxl_defbool_val(b_info->u.hvm.spice.enable)) {
+        if (dm_new && libxl_defbool_val(b_info->u.hvm.spice.enable)) {
             const libxl_spice_info *spice = &b_info->u.hvm.spice;
             char *spiceoptions = dm_spice_options(gc, spice);
             if (!spiceoptions)

@@ -424,19 +293,35 @@ static char ** libxl__build_device_model
             flexarray_append(dm_args, spiceoptions);
         }
 
-        switch (b_info->u.hvm.vga.kind) {
-        case LIBXL_VGA_INTERFACE_TYPE_STD:
-            flexarray_vappend(dm_args, "-vga", "std", NULL);
-            break;
-        case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
-            flexarray_vappend(dm_args, "-vga", "cirrus", NULL);
-            break;
+        if ( dm_new ) {
+            switch (b_info->u.hvm.vga.kind) {
+            case LIBXL_VGA_INTERFACE_TYPE_STD:
+                flexarray_vappend(dm_args, "-vga", "std", NULL);
+                break;
+            case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
+                flexarray_vappend(dm_args, "-vga", "cirrus", NULL);
+                break;
+            }
+        } else {
+            switch (b_info->u.hvm.vga.kind) {
+            case LIBXL_VGA_INTERFACE_TYPE_STD:
+                flexarray_append(dm_args, "-std-vga");
+                break;
+            case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
+                break;
+            }
         }
 
         if (b_info->u.hvm.boot) {
-            flexarray_vappend(dm_args, "-boot",
-                    libxl__sprintf(gc, "order=%s", b_info->u.hvm.boot), NULL);
+            if (dm_new)
+                flexarray_vappend(dm_args, "-boot",
+                                  libxl__sprintf(gc, "order=%s",
+                                                 b_info->u.hvm.boot), NULL);
+            else
+                flexarray_vappend(dm_args, "-boot", b_info->u.hvm.boot, NULL);
+
         }
+
         if (libxl_defbool_val(b_info->u.hvm.usb) || b_info->u.hvm.usbdevice) {
             flexarray_append(dm_args, "-usb");
             if (b_info->u.hvm.usbdevice) {

@@ -450,19 +335,39 @@ static char ** libxl__build_device_model
         if (!libxl_defbool_val(b_info->u.hvm.acpi)) {
             flexarray_append(dm_args, "-no-acpi");
         }
-        if (b_info->max_vcpus > 1) {
-            flexarray_append(dm_args, "-smp");
-            if (b_info->avail_vcpus.size) {
-                int nr_set_cpus = 0;
-                nr_set_cpus = libxl_bitmap_count_set(&b_info->avail_vcpus);
 
-                flexarray_append(dm_args, libxl__sprintf(gc, "%d,maxcpus=%d",
-                                                         b_info->max_vcpus,
-                                                         nr_set_cpus));
-            } else
-                flexarray_append(dm_args, libxl__sprintf(gc, "%d",
-                                                         b_info->max_vcpus));
+        if (dm_new)
+        {
+            if (b_info->max_vcpus > 1) {
+                flexarray_append(dm_args, "-smp");
+                if (b_info->avail_vcpus.size) {
+                    int nr_set_cpus = 0;
+                    nr_set_cpus = libxl_bitmap_count_set(&b_info->avail_vcpus);
+                    
+                    flexarray_append(dm_args,
+                                     libxl__sprintf(gc, "%d,maxcpus=%d",
+                                                    b_info->max_vcpus,
+                                                    nr_set_cpus));
+                } else
+                    flexarray_append(dm_args,
+                                     libxl__sprintf(gc, "%d",
+                                                    b_info->max_vcpus));
+            }
+        } else {
+            int nr_set_cpus = 0;
+            char *s;
+            if (b_info->max_vcpus > 1) {
+                flexarray_vappend(dm_args, "-vcpus",
+                                  libxl__sprintf(gc, "%d", b_info->max_vcpus),
+                                  NULL);
+            }
+            nr_set_cpus = libxl_bitmap_count_set(&b_info->avail_vcpus);
+            s = libxl_bitmap_to_hex_string(CTX, &b_info->avail_vcpus);
+            flexarray_vappend(dm_args, "-vcpu_avail",
+                              libxl__sprintf(gc, "%s", s), NULL);
+            free(s);
         }
+
         for (i = 0; i < num_nics; i++) {
             if (nics[i].nictype == LIBXL_NIC_TYPE_VIF_IOEMU) {
                 char *smac = libxl__sprintf(gc,

@@ -470,18 +375,35 @@ static char ** libxl__build_device_model
                 const char *ifname = libxl__device_nic_devname(gc,
                                                 guest_domid, nics[i].devid,
                                                 LIBXL_NIC_TYPE_VIF_IOEMU);
-                flexarray_append(dm_args, "-device");
-                flexarray_append(dm_args,
-                   libxl__sprintf(gc, "%s,id=nic%d,netdev=net%d,mac=%s",
-                                                nics[i].model, nics[i].devid,
-                                                nics[i].devid, smac));
-                flexarray_append(dm_args, "-netdev");
-                flexarray_append(dm_args, GCSPRINTF(
-                                          "type=tap,id=net%d,ifname=%s,"
+                if (dm_new) {
+                    flexarray_append(dm_args, "-device");
+                    flexarray_append(dm_args,
+                          libxl__sprintf(gc, "%s,id=nic%d,netdev=net%d,mac=%s",
+                                       nics[i].model, nics[i].devid,
+                                       nics[i].devid, smac));
+                    flexarray_append(dm_args, "-netdev");
+                    flexarray_append(dm_args, GCSPRINTF(
+                                         "type=tap,id=net%d,ifname=%s,"
+                                         "script=%s,downscript=%s",
+                                         nics[i].devid, ifname,
+                                         libxl_tapif_script(gc),
+                                         libxl_tapif_script(gc)));
+                } else {
+                    flexarray_vappend(dm_args,
+                                      "-net",
+                                      GCSPRINTF(
+                                          "nic,vlan=%d,macaddr=%s,model=%s",
+                                          nics[i].devid, smac, nics[i].model),
+                                      "-net",
+                                      GCSPRINTF(
+                                          "tap,vlan=%d,ifname=%s,bridge=%s,"
                                           "script=%s,downscript=%s",
                                           nics[i].devid, ifname,
+                                          nics[i].bridge,
                                           libxl_tapif_script(gc),
-                                          libxl_tapif_script(gc)));
+                                          libxl_tapif_script(gc)),
+                                      NULL);
+                }
                 ioemu_nics++;
             }
         }

@@ -500,10 +422,15 @@ static char ** libxl__build_device_model
     }
 
     if (state->saved_state) {
-        /* This file descriptor is meant to be used by QEMU */
-        int migration_fd = open(state->saved_state, O_RDONLY);
-        flexarray_append(dm_args, "-incoming");
-        flexarray_append(dm_args, libxl__sprintf(gc, "fd:%d", migration_fd));
+        if (dm_new) {
+            /* This file descriptor is meant to be used by QEMU */
+            int migration_fd = open(state->saved_state, O_RDONLY);
+            flexarray_append(dm_args, "-incoming");
+            flexarray_append(dm_args,
+                             libxl__sprintf(gc, "fd:%d", migration_fd));
+        } else {
+            flexarray_vappend(dm_args, "-loadvm", state->saved_state, NULL);
+        }
     }
     for (i = 0; b_info->extra && b_info->extra[i] != NULL; i++)
         flexarray_append(dm_args, b_info->extra[i]);

@@ -523,6 +450,9 @@ static char ** libxl__build_device_model
         abort();
     }
 
+    if (!dm_new)
+        goto finish;
+
     ram_size = libxl__sizekb_to_mb(b_info->max_memkb - b_info->video_memkb);
     flexarray_append(dm_args, "-m");
     flexarray_append(dm_args, libxl__sprintf(gc, "%"PRId64, ram_size));

@@ -585,33 +515,11 @@ static char ** libxl__build_device_model
             flexarray_append(dm_args, drive);
         }
     }
+finish:
     flexarray_append(dm_args, NULL);
     return (char **) flexarray_contents(dm_args);
 }
 
-static char ** libxl__build_device_model_args(libxl__gc *gc,
-                                        const char *dm, int guest_domid,
-                                        const libxl_domain_config *guest_config,
-                                        const libxl__domain_build_state *state)
-{
-    libxl_ctx *ctx = libxl__gc_owner(gc);
-
-    switch (guest_config->b_info.device_model_version) {
-    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
-        return libxl__build_device_model_args_old(gc, dm,
-                                                  guest_domid, guest_config,
-                                                  state);
-    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
-        return libxl__build_device_model_args_new(gc, dm,
-                                                  guest_domid, guest_config,
-                                                  state);
-    default:
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unknown device model version %d",
-                         guest_config->b_info.device_model_version);
-        return NULL;
-    }
-}
-
 static void libxl__dm_vifs_from_hvm_guest_config(libxl__gc *gc,
                                     libxl_domain_config * const guest_config,
                                     libxl_domain_config *dm_config)

George Dunlap

author

2012-11-30 16:24 UTC #4 lore raw

PATCH v2 1/1 xl: Check for duplicate vncdisplay options, and return an error

In reply to George Dunlap

# HG changeset patch
# User George Dunlap <[email protected]>
# Date 1354291984 0
# Node ID fa37bd276212a01e3c898b54a7f2385454c406a7
# Parent 722da032ac90c0e1a78b1154fa588bf295d1f009
xl: Check for duplicate vncdisplay options, and return an error

If the user has set a vnc display number both in vnclisten (with "xxxx:yy"), and with vncdisplay, throw an error.

Update man pages to match.

Signed-off-by George Dunlap <[email protected]>

docs/man/xl.cfg.pod.5 +6 -0

@@ -350,11 +350,17 @@ other VNC-related settings.  The default
 
 Specifies the IP address, and optionally VNC display number, to use.
 
+NB that if you specify the display number here, you should not use
+vncdisplay.
+
 =item C<vncdisplay=DISPLAYNUM>
 
 Specifies the VNC display number to use.  The actual TCP port number
 will be DISPLAYNUM+5900.
 
+NB that you should not use this option if you set the displaynum in the 
+vnclisten string.
+
 =item C<vncunused=BOOLEAN>
 
 Requests that the VNC display setup search for a free TCP port to use.

tools/libxl/xl_cmdimpl.c +11 -0

@@ -1257,6 +1257,7 @@ skip_nic:
                     vfb->sdl.xauthority = strdup(p2 + 1);
                 }
             } while ((p = strtok(NULL, ",")) != NULL);
+
 skip_vfb:
             free(buf2);
             d_config->num_vfbs++;

@@ -1490,6 +1491,16 @@ skip_vfb:
         xlu_cfg_replace_string (config, "soundhw", &b_info->u.hvm.soundhw, 0);
         xlu_cfg_get_defbool(config, "xen_platform_pci",
                             &b_info->u.hvm.xen_platform_pci, 0);
+
+        if(b_info->u.hvm.vnc.listen
+           && b_info->u.hvm.vnc.display
+           && strchr(b_info->u.hvm.vnc.listen, ':') != NULL) {
+            fprintf(stderr,
+                    "ERROR: Display specified both in vnclisten"
+                    " and vncdisplay!\n");
+            exit (1);
+            
+        }
     }
 
     xlu_cfg_destroy(config);

George Dunlap

author

2012-11-30 16:24 UTC #6 lore raw

PATCH v2 1/1 libxl: Allow multiple USB devices on HVM domain creation

In reply to George Dunlap

# HG changeset patch
# User George Dunlap <[email protected]> # Date 1354291988 0
# Node ID af47b01d2dbe0e916e1014a264845c48d6ef8108 # Parent fa37bd276212a01e3c898b54a7f2385454c406a7
libxl: Allow multiple USB devices on HVM domain creation

This patch allows an HVM domain to be created with multiple USB devices.

Since the previous interface only allowed the passing of a single device, this requires us to add a new element to the hvm struct of libxl_domain_build_info -- usbdevice_list. For API compatibility, the old element, usbdevice, remains.

If hvm.usbdevice_list is set, each device listed will cause an extra "-usbdevice [foo]" to be appended to the qemu command line.

Callers may set either hvm.usbdevice or hvm.usbdevice_list, but not both; libxl will throw an error if both are set.

In order to allow users of libxl to write software compatible with older versions of libxl, also define LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST. If this is defined, callers may use either hvm.usbdevice or hvm.usbdevice_list; otherwise, only hvm.usbdevice will be available.

v2:
- Throw an error if both usbdevice and usbdevice_list are set
- Update and clarify definition based on feedback
- Previous patches means this works for both traditional and upstream

Signed-off-by George Dunlap <[email protected]>

tools/libxl/libxl.h +16 -0

@@ -266,6 +266,22 @@
 #endif
 #endif
 
+/* 
+ * LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST
+ * 
+ * If this is defined, then the libxl_domain_build_info structure will
+ * contain hvm.usbdevice_list, a libxl_string_list type that contains
+ * a list of USB devices to specify on the qemu command-line.
+ *
+ * If it is set, callers may use either hvm.usbdevice or
+ * hvm.usbdevice_list, but not both; if both are set, libxl will
+ * throw an error.
+ *
+ * If this is not defined, callers can only use hvm.usbdevice.  Note
+ * that this means only one device can be added at domain build time.
+ */
+#define LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST 1
+
 /* Functions annotated with LIBXL_EXTERNAL_CALLERS_ONLY may not be
  * called from within libxl itself. Callers outside libxl, who
  * do not #include libxl_internal.h, are fine. */

tools/libxl/libxl_dm.c +18 -1

@@ -322,11 +322,28 @@ static char ** libxl__build_device_model
 
         }
 
-        if (libxl_defbool_val(b_info->u.hvm.usb) || b_info->u.hvm.usbdevice) {
+        if (libxl_defbool_val(b_info->u.hvm.usb)
+            || b_info->u.hvm.usbdevice
+            || b_info->u.hvm.usbdevice_list) {
+            if ( b_info->u.hvm.usbdevice && b_info->u.hvm.usbdevice_list )
+            {
+                LOG(ERROR, "%s: Both usbdevice and usbdevice_list set",
+                    __func__);
+                return NULL;
+            }
             flexarray_append(dm_args, "-usb");
             if (b_info->u.hvm.usbdevice) {
                 flexarray_vappend(dm_args,
                                   "-usbdevice", b_info->u.hvm.usbdevice, NULL);
+            } else if (b_info->u.hvm.usbdevice_list) {
+                char **p;
+                for (p = b_info->u.hvm.usbdevice_list;
+                     *p;
+                     p++) {
+                    flexarray_vappend(dm_args,
+                                      "-usbdevice",
+                                      *p, NULL);
+                }
             }
         }
         if (b_info->u.hvm.soundhw) {

tools/libxl/libxl_types.idl +1 -0

@@ -326,6 +326,7 @@ libxl_domain_build_info = Struct("domain
                                        ("usbdevice",        string),
                                        ("soundhw",          string),
                                        ("xen_platform_pci", libxl_defbool),
+                                       ("usbdevice_list",   libxl_string_list),
                                        ])),
                  ("pv", Struct(None, [("kernel", string),
                                       ("slack_memkb", MemKB),

George Dunlap

author

2012-11-30 16:24 UTC #8 lore raw

PATCH v2 1/1 xl: Accept a list for usbdevice in config file

In reply to George Dunlap

# HG changeset patch
# User George Dunlap <[email protected]> # Date 1354291988 0
# Node ID 043d69be04c91eaead159e8e74999c59eb68f560 # Parent af47b01d2dbe0e916e1014a264845c48d6ef8108
xl: Accept a list for usbdevice in config file

Allow the "usbdevice" key to accept a list of USB devices, and pass them in using the new usbdevice_list domain build element.

For backwards compatibility, still accept singleton values.

Also update the xl.cfg manpage, adding information about how to pass through host devices.

v2:
- Add some verbiage to make it clear that "usb" is for emulated devices
- Reference qemu manual for more usbdevice options

Signed-off-by George Dunlap <[email protected]>

docs/man/xl.cfg.pod.5 +19 -9

@@ -1107,17 +1107,27 @@ device.
 
 =item B<usb=BOOLEAN>
 
-Enables or disables a USB bus in the guest.
+Enables or disables an emulated USB bus in the guest.
 
-=item B<usbdevice=DEVICE>
+=item B<usbdevice=[ "DEVICE", "DEVICE", ...]>
 
-Adds B<DEVICE> to the USB bus. The USB bus must also be enabled using
-B<usb=1>. The most common use for this option is B<usbdevice=tablet>
-which adds pointer device using absolute coordinates. Such devices
-function better than relative coordinate devices (such as a standard
-mouse) since many methods of exporting guest graphics (such as VNC)
-work better in this mode. Note that this is independent of the actual
-pointer device you are using on the host/client side.
+Adds B<DEVICE>s to the emulated USB bus. The USB bus must also be
+enabled using B<usb=1>. The most common use for this option is
+B<usbdevice=['tablet']> which adds pointer device using absolute
+coordinates. Such devices function better than relative coordinate
+devices (such as a standard mouse) since many methods of exporting
+guest graphics (such as VNC) work better in this mode. Note that this
+is independent of the actual pointer device you are using on the
+host/client side.
+
+Host devices can also be passed through in this way, by specifying
+host:USBID, where USBID is of the form xxxx:yyyy.  The USBID can 
+typically be found by using lsusb or usb-devices.
+
+The form usbdevice=DEVICE is also accepted for backwards compatibility.
+
+More valid options can be found in the "usbdevice" section of the qemu
+documentation.
 
 =back

tools/libxl/xl_cmdimpl.c +17 -2

@@ -1486,8 +1486,23 @@ skip_vfb:
         xlu_cfg_replace_string (config, "serial", &b_info->u.hvm.serial, 0);
         xlu_cfg_replace_string (config, "boot", &b_info->u.hvm.boot, 0);
         xlu_cfg_get_defbool(config, "usb", &b_info->u.hvm.usb, 0);
-        xlu_cfg_replace_string (config, "usbdevice",
-                                &b_info->u.hvm.usbdevice, 0);
+        switch (xlu_cfg_get_list_as_string_list(config, "usbdevice",
+                                                &b_info->u.hvm.usbdevice_list,
+                                                1))
+        {
+
+        case 0: break; /* Success */
+        case ESRCH: break; /* Option not present */
+        case EINVAL:
+            /* If it's not a valid list, try reading it as an atom, falling through to
+             * an error if it fails */
+            if (!xlu_cfg_replace_string(config, "usbdevice", &b_info->u.hvm.usbdevice, 0)) 
+                break;
+            /* FALLTHRU */
+        default:
+            fprintf(stderr,"xl: Unable to parse usbdevice.\n");
+            exit(-ERROR_FAIL);
+        }
         xlu_cfg_replace_string (config, "soundhw", &b_info->u.hvm.soundhw, 0);
         xlu_cfg_get_defbool(config, "xen_platform_pci",
                             &b_info->u.hvm.xen_platform_pci, 0);

George Dunlap

author

2012-11-30 16:53 UTC #2 lore raw

Re: [PATCH 1 of 4 v2] libxl: Combine device model arg build functions

In reply to George Dunlap

This patch also provides an easy way to trigger an interesting "failure to handle failure" bug in libxl.

To reproduce:
* Apply *only* this patch, and not patch 2 in this series (which filters it out at the xl level)
* Create a config file where vnclisten has a display number, and vncdisplay is non-zero; e.g.:
vnclisten="0.0.0.0:1"
vncdisplay=1

(vncdisplay=0 is indistinguishable, to xl, from not having it set at all.)

The result is a set of error messages like this:

libxl: error: libxl_dm.c:231:libxl__build_device_model_args: vncdisplay set, but vnclisten also contains adisplay number Too confused to continue.
libxl: error: libxl_dm.c:1113:device_model_spawn_outcome: (null): spawn failed (rc=-3)
libxl: error: libxl_exec.c:118:libxl_report_child_exitstatus:
/etc/xen/scripts/vif-bridge add [21168] exited with error status 1
libxl: error: libxl_create.c:1117:domcreate_attach_vtpms: unable to add nic devices
libxl: error: libxl_dm.c:1140:libxl__destroy_device_model: could not find device-model's pid for dom 25
libxl: error: libxl.c:1414:libxl__destroy_domid:
libxl__destroy_device_model failed for 25

Obviously the failure in libxl__build_device_model_args() failed to abort the domain-building process, and it continued to try to run vif and vtpm setups.

-George

Quoted 619 lines · George Dunlap

However, at least 80% of these are the same; this means that fixes or additions to one may not make it into the other. Furthermore, there are some unaccounable differences in implementation.

This patch combines the two functions into one, choosing between old and new implementations as necessary.

Signed-off-by George Dunlap <[email protected]>

tools/libxl/libxl_dm.c +0 ~~-67~~

@@ -95,181 +95,6 @@ static const char *dm_keymap(const libxl
         return NULL;
 }
 
-static char ** libxl__build_device_model_args_old(libxl__gc *gc,
-                                        const char *dm, int domid,
-                                        const libxl_domain_config
 *guest_config,
-                                        const libxl__domain_build_state
 *state)
-{
-    const libxl_domain_create_info *c_info = &guest_config->c_info;
-    const libxl_domain_build_info *b_info = &guest_config->b_info;
-    const libxl_device_nic *nics = guest_config->nics;
-    const libxl_vnc_info *vnc = libxl__dm_vnc(guest_config);
-    const libxl_sdl_info *sdl = dm_sdl(guest_config);
-    const int num_nics = guest_config->num_nics;
-    const char *keymap = dm_keymap(guest_config);
-    int i;
-    flexarray_t *dm_args;
-    dm_args = flexarray_make(gc, 16, 1);
-
-    flexarray_vappend(dm_args, dm,
-                      "-d", libxl__sprintf(gc, "%d", domid), NULL);
-
-    if (c_info->name)
-        flexarray_vappend(dm_args, "-domain-name", c_info->name, NULL);
-
-    if (vnc) {
-        char *vncarg;
-        if (vnc->display) {
-            if (vnc->listen && strchr(vnc->listen, ':') == NULL) {
-                vncarg = libxl__sprintf(gc, "%s:%d",
-                                  vnc->listen,
-                                  vnc->display);
-            } else {
-                vncarg = libxl__sprintf(gc, "127.0.0.1:%d",
 vnc->display);
-            }
-        } else if (vnc->listen) {
-            if (strchr(vnc->listen, ':') != NULL) {
-                vncarg = vnc->listen;
-            } else {
-                vncarg = libxl__sprintf(gc, "%s:0", vnc->listen);
-            }
-        } else {
-            vncarg = "127.0.0.1:0";
-        }
-        if (vnc->passwd && (vnc->passwd[0] != '\0'))
-            vncarg = libxl__sprintf(gc, "%s,password", vncarg);
-        flexarray_append(dm_args, "-vnc");
-        flexarray_append(dm_args, vncarg);
-
-        if (libxl_defbool_val(vnc->findunused)) {
-            flexarray_append(dm_args, "-vncunused");
-        }
-    }
-    if (sdl) {
-        flexarray_append(dm_args, "-sdl");
-        if (!libxl_defbool_val(sdl->opengl)) {
-            flexarray_append(dm_args, "-disable-opengl");
-        }
-        /* XXX sdl->{display,xauthority} into $DISPLAY/$XAUTHORITY */
-    }
-    if (keymap) {
-        flexarray_vappend(dm_args, "-k", keymap, NULL);
-    }
-    if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) {
-        int ioemu_nics = 0;
-        int nr_set_cpus = 0;
-        char *s;
-
-        if (b_info->u.hvm.serial) {
-            flexarray_vappend(dm_args, "-serial", b_info->u.hvm.serial,

NULL);

tools/libxl/libxl_dm.c +0 -3

-        }
-
-        if (libxl_defbool_val(b_info->u.hvm.nographic) && (!sdl && !vnc))

{

tools/libxl/libxl_dm.c +0 -7

-            flexarray_append(dm_args, "-nographic");
-        }
-
-        if (b_info->video_memkb) {
-            flexarray_vappend(dm_args, "-videoram",
-                    libxl__sprintf(gc, "%d",
-

libxl__sizekb_to_mb(b_info->video_memkb)),

tools/libxl/libxl_dm.c +0 ~~-15~~

-                    NULL);
-        }
-
-        switch (b_info->u.hvm.vga.kind) {
-        case LIBXL_VGA_INTERFACE_TYPE_STD:
-            flexarray_append(dm_args, "-std-vga");
-            break;
-        case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
-            break;
-        }
-
-        if (b_info->u.hvm.boot) {
-            flexarray_vappend(dm_args, "-boot", b_info->u.hvm.boot, NULL);
-        }
-        if (libxl_defbool_val(b_info->u.hvm.usb) ||

b_info->u.hvm.usbdevice) {

tools/libxl/libxl_dm.c +0 -4

-            flexarray_append(dm_args, "-usb");
-            if (b_info->u.hvm.usbdevice) {
-                flexarray_vappend(dm_args,
-                                  "-usbdevice", b_info->u.hvm.usbdevice,

NULL);

tools/libxl/libxl_dm.c +0 -4

-            }
-        }
-        if (b_info->u.hvm.soundhw) {
-            flexarray_vappend(dm_args, "-soundhw", b_info->u.hvm.soundhw,

NULL);

tools/libxl/libxl_dm.c +0 ~~-20~~

-        }
-        if (libxl_defbool_val(b_info->u.hvm.acpi)) {
-            flexarray_append(dm_args, "-acpi");
-        }
-        if (b_info->max_vcpus > 1) {
-            flexarray_vappend(dm_args, "-vcpus",
-                              libxl__sprintf(gc, "%d", b_info->max_vcpus),
-                              NULL);
-        }
-
-        nr_set_cpus = libxl_bitmap_count_set(&b_info->avail_vcpus);
-        s = libxl_bitmap_to_hex_string(CTX, &b_info->avail_vcpus);
-        flexarray_vappend(dm_args, "-vcpu_avail",
-                              libxl__sprintf(gc, "%s", s), NULL);
-        free(s);
-
-        for (i = 0; i < num_nics; i++) {
-            if (nics[i].nictype == LIBXL_NIC_TYPE_VIF_IOEMU) {
-                char *smac = libxl__sprintf(gc,
-                                   LIBXL_MAC_FMT,

LIBXL_MAC_BYTES(nics[i].mac));

tools/libxl/libxl_dm.c +0 ~~-13~~

-                const char *ifname = libxl__device_nic_devname(gc,
-                                                domid, nics[i].devid,
-                                                LIBXL_NIC_TYPE_VIF_IOEMU);
-                flexarray_vappend(dm_args,
-                                  "-net",
-                                  GCSPRINTF(
-                                      "nic,vlan=%d,macaddr=%s,model=%s",
-                                      nics[i].devid, smac, nics[i].model),
-                                  "-net",
-                                  GCSPRINTF(
-                                      "tap,vlan=%d,ifname=%s,bridge=%s,"
-                                      "script=%s,downscript=%s",
-                                      nics[i].devid, ifname,

nics[i].bridge,

tools/libxl/libxl_dm.c +0 ~~-33~~

-                                      libxl_tapif_script(gc),
-                                      libxl_tapif_script(gc)),
-                                  NULL);
-                ioemu_nics++;
-            }
-        }
-        /* If we have no emulated nics, tell qemu not to create any */
-        if ( ioemu_nics == 0 ) {
-            flexarray_vappend(dm_args, "-net", "none", NULL);
-        }
-        if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) {
-            flexarray_append(dm_args, "-gfx_passthru");
-        }
-    } else {
-        if (!sdl && !vnc)
-            flexarray_append(dm_args, "-nographic");
-    }
-
-    if (state->saved_state) {
-        flexarray_vappend(dm_args, "-loadvm", state->saved_state, NULL);
-    }
-    for (i = 0; b_info->extra && b_info->extra[i] != NULL; i++)
-        flexarray_append(dm_args, b_info->extra[i]);
-    flexarray_append(dm_args, "-M");
-    switch (b_info->type) {
-    case LIBXL_DOMAIN_TYPE_PV:
-        flexarray_append(dm_args, "xenpv");
-        for (i = 0; b_info->extra_pv && b_info->extra_pv[i] != NULL; i++)
-            flexarray_append(dm_args, b_info->extra_pv[i]);
-        break;
-    case LIBXL_DOMAIN_TYPE_HVM:
-        flexarray_append(dm_args, "xenfv");
-        for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL;

i++)

tools/libxl/libxl_dm.c +1 ~~-10~~

-            flexarray_append(dm_args, b_info->extra_hvm[i]);
-        break;
-    default:
-        abort();
-    }
-    flexarray_append(dm_args, NULL);
-    return (char **) flexarray_contents(dm_args);
-}
-
 static const char *qemu_disk_format_string(libxl_disk_format format)
 {
     switch (format) {

@@ -318,7 +143,7 @@ static char *dm_spice_options(libxl__gc
     return opt;
 }
 
-static char ** libxl__build_device_model_args_new(libxl__gc *gc,
+static char ** libxl__build_device_model_args(libxl__gc *gc,
                                         const char *dm, int guest_domid,
                                         const libxl_domain_config
 *guest_config,

const libxl__domain_build_state
*state)

tools/libxl/libxl_dm.c +45 ~~-19~~

@@ -336,62 +161,106 @@ static char ** libxl__build_device_model
     flexarray_t *dm_args;
     int i;
     uint64_t ram_size;
+    int dm_new;
+
+    switch (guest_config->b_info.device_model_version) {
+    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
+        dm_new = 0;
+        break;
+    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
+        dm_new = 1;
+        break;
+    default:
+        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unknown device model
 version %d",
+                         guest_config->b_info.device_model_version);
+        return NULL;
+    }
+
 
     dm_args = flexarray_make(gc, 16, 1);
 
-    flexarray_vappend(dm_args, dm,
-                      "-xen-domid",
-                      libxl__sprintf(gc, "%d", guest_domid), NULL);
+    /* Domain ID */
+    if (dm_new) {
+        flexarray_vappend(dm_args, dm,
+                          "-xen-domid",
+                          libxl__sprintf(gc, "%d", guest_domid), NULL);
+    } else {
+        flexarray_vappend(dm_args, dm,
+                          "-d", libxl__sprintf(gc, "%d", guest_domid),
 NULL);
+    }
 
-    flexarray_append(dm_args, "-chardev");
-    flexarray_append(dm_args,
-                     libxl__sprintf(gc, "socket,id=libxl-cmd,"
-                                    "path=%s/qmp-libxl-%d,server,nowait",
-                                    libxl__run_dir_path(), guest_domid));
+    if (dm_new) {
+        flexarray_append(dm_args, "-chardev");
+        flexarray_append(dm_args,
+                         libxl__sprintf(gc, "socket,id=libxl-cmd,"
+
 "path=%s/qmp-libxl-%d,server,nowait",
+                                        libxl__run_dir_path(),
 guest_domid));
 
-    flexarray_append(dm_args, "-mon");
-    flexarray_append(dm_args, "chardev=libxl-cmd,mode=control");
+        flexarray_append(dm_args, "-mon");
+        flexarray_append(dm_args, "chardev=libxl-cmd,mode=control");
 
-    if (b_info->type == LIBXL_DOMAIN_TYPE_PV) {
-        flexarray_append(dm_args, "-xen-attach");
+        if (b_info->type == LIBXL_DOMAIN_TYPE_PV) {
+            flexarray_append(dm_args, "-xen-attach");
+        }
     }
 
     if (c_info->name) {
-        flexarray_vappend(dm_args, "-name", c_info->name, NULL);
+        if ( dm_new )
+            flexarray_vappend(dm_args, "-name", c_info->name, NULL);
+        else
+            flexarray_vappend(dm_args, "-domain-name", c_info->name,
 NULL);
     }
+
     if (vnc) {
-        int display = 0;
-        const char *addr = "127.0.0.1";
         char *vncarg = NULL;
 
         flexarray_append(dm_args, "-vnc");
 
-        if (vnc->display) {
-            display = vnc->display;
-            if (vnc->listen && strchr(vnc->listen, ':') == NULL) {
-                addr = vnc->listen;
+        /*
+         * If vnc->listen is present and contains a :, and
+         *  - vnc->display is 0, use vnc->listen
+         *  - vnc->display is non-zero, be confused
+         * If vnc->listen is present but doesn't, use

vnc->listen:vnc->display.

tools/libxl/libxl_dm.c +7 -0

+         * If vnc->listen is not present, use 127.0.0.1:vnc->display
+         * (Remembering that vnc->display already defaults to 0.)
+         */
+        if (vnc->listen) {
+            if (strchr(vnc->listen, ':') != NULL) {
+                if (vnc->display) {
+                    LOG(ERROR, "vncdisplay set, vnclisten contains

display");

tools/libxl/libxl_dm.c +10 -8

+                    return NULL;
+                }
+                vncarg = vnc->listen;
+            } else {
+                vncarg = libxl__sprintf(gc, "%s:%d", vnc->listen,
+                                        vnc->display);
             }
-        } else if (vnc->listen) {
-            addr = vnc->listen;
-        }
+        } else
+            vncarg = libxl__sprintf(gc, "127.0.0.1:%d", vnc->display);
 
-        if (strchr(addr, ':') != NULL)
-            vncarg = libxl__sprintf(gc, "%s", addr);
-        else
-            vncarg = libxl__sprintf(gc, "%s:%d", addr, display);
         if (vnc->passwd && vnc->passwd[0]) {
             vncarg = libxl__sprintf(gc, "%s,password", vncarg);
         }
-        if (libxl_defbool_val(vnc->findunused)) {
+
+        if (dm_new && libxl_defbool_val(vnc->findunused)) {
             /* This option asks to QEMU to try this number of port before

to
* give up. So QEMU will try ports between $display and
$display +
* 99. This option needs to be the last one of the vnc
options. */

             vncarg = libxl__sprintf(gc, "%s,to=99", vncarg);
         }

tools/libxl/libxl_dm.c +35 ~~-10~~

+
         flexarray_append(dm_args, vncarg);
+
+        if (!dm_new && libxl_defbool_val(vnc->findunused)) {
+            flexarray_append(dm_args, "-vncunused");
+        }
     }
     if (sdl) {
         flexarray_append(dm_args, "-sdl");
+
+        if (!dm_new && !libxl_defbool_val(sdl->opengl)) {
+            flexarray_append(dm_args, "-disable-opengl");
+        }
         /* XXX sdl->{display,xauthority} into $DISPLAY/$XAUTHORITY */
     }

@@ -414,7 +283,7 @@ static char ** libxl__build_device_model
             flexarray_append(dm_args, "-nographic");
         }
 
-        if (libxl_defbool_val(b_info->u.hvm.spice.enable)) {
+        if (dm_new && libxl_defbool_val(b_info->u.hvm.spice.enable)) {
             const libxl_spice_info *spice = &b_info->u.hvm.spice;
             char *spiceoptions = dm_spice_options(gc, spice);
             if (!spiceoptions)

@@ -424,19 +293,35 @@ static char ** libxl__build_device_model
             flexarray_append(dm_args, spiceoptions);
         }
 
-        switch (b_info->u.hvm.vga.kind) {
-        case LIBXL_VGA_INTERFACE_TYPE_STD:
-            flexarray_vappend(dm_args, "-vga", "std", NULL);
-            break;
-        case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
-            flexarray_vappend(dm_args, "-vga", "cirrus", NULL);
-            break;
+        if ( dm_new ) {
+            switch (b_info->u.hvm.vga.kind) {
+            case LIBXL_VGA_INTERFACE_TYPE_STD:
+                flexarray_vappend(dm_args, "-vga", "std", NULL);
+                break;
+            case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
+                flexarray_vappend(dm_args, "-vga", "cirrus", NULL);
+                break;
+            }
+        } else {
+            switch (b_info->u.hvm.vga.kind) {
+            case LIBXL_VGA_INTERFACE_TYPE_STD:
+                flexarray_append(dm_args, "-std-vga");
+                break;
+            case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
+                break;
+            }
         }
 
         if (b_info->u.hvm.boot) {
-            flexarray_vappend(dm_args, "-boot",
-                    libxl__sprintf(gc, "order=%s", b_info->u.hvm.boot),
 NULL);
+            if (dm_new)
+                flexarray_vappend(dm_args, "-boot",
+                                  libxl__sprintf(gc, "order=%s",
+                                                 b_info->u.hvm.boot),
 NULL);
+            else
+                flexarray_vappend(dm_args, "-boot", b_info->u.hvm.boot,
 NULL);
+
         }
+

         if (libxl_defbool_val(b_info->u.hvm.usb) ||
b_info->u.hvm.usbdevice) {
             flexarray_append(dm_args, "-usb");
             if (b_info->u.hvm.usbdevice) {

tools/libxl/libxl_dm.c +7 ~~-11~~

@@ -450,19 +335,39 @@ static char ** libxl__build_device_model
         if (!libxl_defbool_val(b_info->u.hvm.acpi)) {
             flexarray_append(dm_args, "-no-acpi");
         }
-        if (b_info->max_vcpus > 1) {
-            flexarray_append(dm_args, "-smp");
-            if (b_info->avail_vcpus.size) {
-                int nr_set_cpus = 0;
-                nr_set_cpus =
 libxl_bitmap_count_set(&b_info->avail_vcpus);
 
-                flexarray_append(dm_args, libxl__sprintf(gc,
 "%d,maxcpus=%d",
-
 b_info->max_vcpus,
-                                                         nr_set_cpus));
-            } else
-                flexarray_append(dm_args, libxl__sprintf(gc, "%d",
-
 b_info->max_vcpus));
+        if (dm_new)
+        {
+            if (b_info->max_vcpus > 1) {
+                flexarray_append(dm_args, "-smp");
+                if (b_info->avail_vcpus.size) {
+                    int nr_set_cpus = 0;
+                    nr_set_cpus =

libxl_bitmap_count_set(&b_info->avail_vcpus);

tools/libxl/libxl_dm.c +16 -0

+
+                    flexarray_append(dm_args,
+                                     libxl__sprintf(gc, "%d,maxcpus=%d",
+                                                    b_info->max_vcpus,
+                                                    nr_set_cpus));
+                } else
+                    flexarray_append(dm_args,
+                                     libxl__sprintf(gc, "%d",
+                                                    b_info->max_vcpus));
+            }
+        } else {
+            int nr_set_cpus = 0;
+            char *s;
+            if (b_info->max_vcpus > 1) {
+                flexarray_vappend(dm_args, "-vcpus",
+                                  libxl__sprintf(gc, "%d",

b_info->max_vcpus),

tools/libxl/libxl_dm.c +43 ~~-13~~

+                                  NULL);
+            }
+            nr_set_cpus = libxl_bitmap_count_set(&b_info->avail_vcpus);
+            s = libxl_bitmap_to_hex_string(CTX, &b_info->avail_vcpus);
+            flexarray_vappend(dm_args, "-vcpu_avail",
+                              libxl__sprintf(gc, "%s", s), NULL);
+            free(s);
         }
+
         for (i = 0; i < num_nics; i++) {
             if (nics[i].nictype == LIBXL_NIC_TYPE_VIF_IOEMU) {
                 char *smac = libxl__sprintf(gc,

@@ -470,18 +375,35 @@ static char ** libxl__build_device_model
                 const char *ifname = libxl__device_nic_devname(gc,
                                                 guest_domid,
 nics[i].devid,
                                                 LIBXL_NIC_TYPE_VIF_IOEMU);
-                flexarray_append(dm_args, "-device");
-                flexarray_append(dm_args,
-                   libxl__sprintf(gc, "%s,id=nic%d,netdev=net%d,mac=%s",
-                                                nics[i].model,
 nics[i].devid,
-                                                nics[i].devid, smac));
-                flexarray_append(dm_args, "-netdev");
-                flexarray_append(dm_args, GCSPRINTF(
-                                          "type=tap,id=net%d,ifname=%s,"
+                if (dm_new) {
+                    flexarray_append(dm_args, "-device");
+                    flexarray_append(dm_args,
+                          libxl__sprintf(gc,
 "%s,id=nic%d,netdev=net%d,mac=%s",
+                                       nics[i].model, nics[i].devid,
+                                       nics[i].devid, smac));
+                    flexarray_append(dm_args, "-netdev");
+                    flexarray_append(dm_args, GCSPRINTF(
+                                         "type=tap,id=net%d,ifname=%s,"
+                                         "script=%s,downscript=%s",
+                                         nics[i].devid, ifname,
+                                         libxl_tapif_script(gc),
+                                         libxl_tapif_script(gc)));
+                } else {
+                    flexarray_vappend(dm_args,
+                                      "-net",
+                                      GCSPRINTF(
+
 "nic,vlan=%d,macaddr=%s,model=%s",
+                                          nics[i].devid, smac,
 nics[i].model),
+                                      "-net",
+                                      GCSPRINTF(
+
 "tap,vlan=%d,ifname=%s,bridge=%s,"
                                           "script=%s,downscript=%s",
                                           nics[i].devid, ifname,
+                                          nics[i].bridge,
                                           libxl_tapif_script(gc),
-                                          libxl_tapif_script(gc)));
+                                          libxl_tapif_script(gc)),
+                                      NULL);
+                }
                 ioemu_nics++;
             }
         }

@@ -500,10 +422,15 @@ static char ** libxl__build_device_model
     }
 
     if (state->saved_state) {
-        /* This file descriptor is meant to be used by QEMU */
-        int migration_fd = open(state->saved_state, O_RDONLY);
-        flexarray_append(dm_args, "-incoming");
-        flexarray_append(dm_args, libxl__sprintf(gc, "fd:%d",
 migration_fd));
+        if (dm_new) {
+            /* This file descriptor is meant to be used by QEMU */
+            int migration_fd = open(state->saved_state, O_RDONLY);
+            flexarray_append(dm_args, "-incoming");
+            flexarray_append(dm_args,
+                             libxl__sprintf(gc, "fd:%d", migration_fd));
+        } else {
+            flexarray_vappend(dm_args, "-loadvm", state->saved_state,
 NULL);
+        }
     }

     for (i = 0; b_info->extra && b_info->extra[i] != NULL; i++)
         flexarray_append(dm_args, b_info->extra[i]);

tools/libxl/libxl_dm.c +3 -0

@@ -523,6 +450,9 @@ static char ** libxl__build_device_model
         abort();
     }
 
+    if (!dm_new)
+        goto finish;
+
     ram_size = libxl__sizekb_to_mb(b_info->max_memkb -
 b_info->video_memkb);
     flexarray_append(dm_args, "-m");

     flexarray_append(dm_args, libxl__sprintf(gc, "%"PRId64, ram_size));

tools/libxl/libxl_dm.c +1 ~~-15~~

@@ -585,33 +515,11 @@ static char ** libxl__build_device_model
             flexarray_append(dm_args, drive);
         }
     }
+finish:
     flexarray_append(dm_args, NULL);
     return (char **) flexarray_contents(dm_args);
 }
 
-static char ** libxl__build_device_model_args(libxl__gc *gc,
-                                        const char *dm, int guest_domid,
-                                        const libxl_domain_config
 *guest_config,
-                                        const libxl__domain_build_state
 *state)
-{
-    libxl_ctx *ctx = libxl__gc_owner(gc);
-
-    switch (guest_config->b_info.device_model_version) {
-    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
-        return libxl__build_device_model_args_old(gc, dm,
-                                                  guest_domid,
 guest_config,
-                                                  state);
-    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
-        return libxl__build_device_model_args_new(gc, dm,
-                                                  guest_domid,

guest_config,

tools/libxl/libxl_dm.c +0 -3

-                                                  state);
-    default:
-        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unknown device model

version %d",

tools/libxl/libxl_dm.c +0 -5

-                         guest_config->b_info.device_model_version);
-        return NULL;
-    }
-}
-
 static void libxl__dm_vifs_from_hvm_guest_config(libxl__gc *gc,
                                     libxl_domain_config * const

guest_config,
libxl_domain_config *dm_config)

Quoted 5 lines

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Ian Campbell

2012-12-04 15:21 UTC #3 lore raw

Re: [PATCH 1 of 4 v2] libxl: Combine device model arg build functions

In reply to George Dunlap

Quoted 7 lines · George Dunlap

# HG changeset patch
# User George Dunlap <[email protected]> # Date 1354287957 0
# Node ID 722da032ac90c0e1a78b1154fa588bf295d1f009 # Parent ae6fb202b233af815466055d9f1a635802a50855
libxl: Combine device model arg build functions

Quoted 8 lines

qemu-traditional and qemu-upstream have some differences in the way the arguments need to be passed. At the moment, this is dealt with by having two entirely separate functions, libxl__build_device_model_args_new and libxl__build_device_model_args_old.
However, at least 80% of these are the same; this means that fixes or additions to one may not make it into the other. Furthermore, there are some unaccounable differences in implementation.

FWIW:

 1 file changed, 168 insertions(+), 260 deletions(-)

But that doesn't really show how much of the code in the new function is shared and how much is per-qemu-version. Casting my eye over the code with the patch applied (i.e. an unscientific estimate) it seems like most of it is under an if (dm_new) of some sort.

My main concern is that qemu-xen-traditional is frozen and so this code shouldn't be changing, but by bundling it with the qemu-xen code it may be subject to churn as new stuff gets added to the new qemu and plumbed through.

A related concern is that some of the interfaces we use (e.g. "-hda") are deprecated in favour of more expressive forms (-drive -device et al), depending on how things go upstream we may want to switch to using them.

Perhaps a useful compromise would be to pull the common stuff into a common function but call out to version specific function for the bits which differ too.

Perhaps factoring into functional areas might help too? e.g. make_{disk,net,foo}_args, then the decision to combine or not can be made on a per functional area basis?

Quoted 11 lines

tools/libxl/libxl_dm.c +3 -0

@@ -523,6 +450,9 @@ static char ** libxl__build_device_model
         abort();
     }
 
+    if (!dm_new)
+        goto finish;
+
     ram_size = libxl__sizekb_to_mb(b_info->max_memkb - b_info->video_memkb);
     flexarray_append(dm_args, "-m");
     flexarray_append(dm_args, libxl__sprintf(gc, "%"PRId64, ram_size));

tools/libxl/libxl_dm.c static char ** libxl__build_device_model · line 518

Quoted 4 lines

tools/libxl/libxl_dm.c +1 -0

@@ -585,33 +515,11 @@ static char ** libxl__build_device_model
             flexarray_append(dm_args, drive);
         }
     }
+finish:

This feels especially unsatisfying...

Ian Campbell

2012-12-04 15:30 UTC #7 lore raw

Re: [PATCH 3 of 4 v2] libxl: Allow multiple USB devices on HVM domain creation

In reply to George Dunlap

Quoted 26 lines · George Dunlap

# HG changeset patch
# User George Dunlap <[email protected]> # Date 1354291988 0
# Node ID af47b01d2dbe0e916e1014a264845c48d6ef8108 # Parent fa37bd276212a01e3c898b54a7f2385454c406a7
libxl: Allow multiple USB devices on HVM domain creation
This patch allows an HVM domain to be created with multiple USB devices.
Since the previous interface only allowed the passing of a single device, this requires us to add a new element to the hvm struct of libxl_domain_build_info -- usbdevice_list. For API compatibility, the old element, usbdevice, remains.
If hvm.usbdevice_list is set, each device listed will cause an extra "-usbdevice [foo]" to be appended to the qemu command line.
Callers may set either hvm.usbdevice or hvm.usbdevice_list, but not both; libxl will throw an error if both are set.
In order to allow users of libxl to write software compatible with older versions of libxl, also define LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST. If this is defined, callers may use either hvm.usbdevice or hvm.usbdevice_list; otherwise, only hvm.usbdevice will be available.

Quoted 4 lines

v2:
- Throw an error if both usbdevice and usbdevice_list are set
- Update and clarify definition based on feedback
- Previous patches means this works for both traditional and upstream

Is this a requirement? Did this work with qemu-xen-trad with xend?

Unless it did I'm not especially in favour of adding new features to qemu-xen-trad just because it looks easy to do so.

Quoted 44 lines

Signed-off-by George Dunlap <[email protected]>

tools/libxl/libxl.h +16 -0

@@ -266,6 +266,22 @@
 #endif
 #endif
 
+/* 
+ * LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST
+ * 
+ * If this is defined, then the libxl_domain_build_info structure will
+ * contain hvm.usbdevice_list, a libxl_string_list type that contains
+ * a list of USB devices to specify on the qemu command-line.
+ *
+ * If it is set, callers may use either hvm.usbdevice or
+ * hvm.usbdevice_list, but not both; if both are set, libxl will
+ * throw an error.
+ *
+ * If this is not defined, callers can only use hvm.usbdevice.  Note
+ * that this means only one device can be added at domain build time.
+ */
+#define LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST 1
+
 /* Functions annotated with LIBXL_EXTERNAL_CALLERS_ONLY may not be
  * called from within libxl itself. Callers outside libxl, who
  * do not #include libxl_internal.h, are fine. */

tools/libxl/libxl_dm.c +8 -1

@@ -322,11 +322,28 @@ static char ** libxl__build_device_model
 
         }
 
-        if (libxl_defbool_val(b_info->u.hvm.usb) || b_info->u.hvm.usbdevice) {
+        if (libxl_defbool_val(b_info->u.hvm.usb)
+            || b_info->u.hvm.usbdevice
+            || b_info->u.hvm.usbdevice_list) {
+            if ( b_info->u.hvm.usbdevice && b_info->u.hvm.usbdevice_list )
+            {
+                LOG(ERROR, "%s: Both usbdevice and usbdevice_list set",
+                    __func__);
+                return NULL;

tools/libxl/libxl_dm.c static char ** libxl__build_device_model · line 342

Quoted 10 lines

tools/libxl/libxl_dm.c +6 -0

+            }
             flexarray_append(dm_args, "-usb");
             if (b_info->u.hvm.usbdevice) {
                 flexarray_vappend(dm_args,
                                   "-usbdevice", b_info->u.hvm.usbdevice, NULL);
+            } else if (b_info->u.hvm.usbdevice_list) {
+                char **p;
+                for (p = b_info->u.hvm.usbdevice_list;
+                     *p;
+                     p++) {

Is the line too long if you unfold this? It doesn't look to me like it should be.

Quoted 11 lines

tools/libxl/libxl_dm.c +4 -0

+                    flexarray_vappend(dm_args,
+                                      "-usbdevice",
+                                      *p, NULL);
+                }
             }
         }
         if (b_info->u.hvm.soundhw) {

tools/libxl/libxl_types.idl libxl_domain_build_info = Struct("domain · line 329

Quoted 4 lines

tools/libxl/libxl_types.idl +1 -0

@@ -326,6 +326,7 @@ libxl_domain_build_info = Struct("domain
                                        ("usbdevice",        string),
                                        ("soundhw",          string),
                                        ("xen_platform_pci", libxl_defbool),
+                                       ("usbdevice_list",   libxl_string_list),

Is there any chance we might want something more structured that a string in the interface in the future? e.g. libxl_device_usb?

Quoted 8 lines

tools/libxl/libxl_types.idl +0 -0

                                        ])),
                  ("pv", Struct(None, [("kernel", string),
                                       ("slack_memkb", MemKB),

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Ian Campbell

Acked-by on v2 1/1

2012-12-04 15:50 UTC #9 lore raw

Re: [PATCH 4 of 4 v2] xl: Accept a list for usbdevice in config file

In reply to George Dunlap

Quoted 12 lines · George Dunlap

# HG changeset patch
# User George Dunlap <[email protected]> # Date 1354291988 0
# Node ID 043d69be04c91eaead159e8e74999c59eb68f560 # Parent af47b01d2dbe0e916e1014a264845c48d6ef8108
xl: Accept a list for usbdevice in config file
Allow the "usbdevice" key to accept a list of USB devices, and pass them in using the new usbdevice_list domain build element.
For backwards compatibility, still accept singleton values.

Quoted 8 lines

Also update the xl.cfg manpage, adding information about how to pass through host devices.
v2:
- Add some verbiage to make it clear that "usb" is for emulated devices
- Reference qemu manual for more usbdevice options
Signed-off-by George Dunlap <[email protected]>

Acked-by Ian Campbell <[email protected]>

Quoted 75 lines

docs/man/xl.cfg.pod.5 +19 -9

@@ -1107,17 +1107,27 @@ device.
 
 =item B<usb=BOOLEAN>
 
-Enables or disables a USB bus in the guest.
+Enables or disables an emulated USB bus in the guest.
 
-=item B<usbdevice=DEVICE>
+=item B<usbdevice=[ "DEVICE", "DEVICE", ...]>
 
-Adds B<DEVICE> to the USB bus. The USB bus must also be enabled using
-B<usb=1>. The most common use for this option is B<usbdevice=tablet>
-which adds pointer device using absolute coordinates. Such devices
-function better than relative coordinate devices (such as a standard
-mouse) since many methods of exporting guest graphics (such as VNC)
-work better in this mode. Note that this is independent of the actual
-pointer device you are using on the host/client side.
+Adds B<DEVICE>s to the emulated USB bus. The USB bus must also be
+enabled using B<usb=1>. The most common use for this option is
+B<usbdevice=['tablet']> which adds pointer device using absolute
+coordinates. Such devices function better than relative coordinate
+devices (such as a standard mouse) since many methods of exporting
+guest graphics (such as VNC) work better in this mode. Note that this
+is independent of the actual pointer device you are using on the
+host/client side.
+
+Host devices can also be passed through in this way, by specifying
+host:USBID, where USBID is of the form xxxx:yyyy.  The USBID can 
+typically be found by using lsusb or usb-devices.
+
+The form usbdevice=DEVICE is also accepted for backwards compatibility.
+
+More valid options can be found in the "usbdevice" section of the qemu
+documentation.
 
 =back

tools/libxl/xl_cmdimpl.c +17 -2

@@ -1486,8 +1486,23 @@ skip_vfb:
         xlu_cfg_replace_string (config, "serial", &b_info->u.hvm.serial, 0);
         xlu_cfg_replace_string (config, "boot", &b_info->u.hvm.boot, 0);
         xlu_cfg_get_defbool(config, "usb", &b_info->u.hvm.usb, 0);
-        xlu_cfg_replace_string (config, "usbdevice",
-                                &b_info->u.hvm.usbdevice, 0);
+        switch (xlu_cfg_get_list_as_string_list(config, "usbdevice",
+                                                &b_info->u.hvm.usbdevice_list,
+                                                1))
+        {
+
+        case 0: break; /* Success */
+        case ESRCH: break; /* Option not present */
+        case EINVAL:
+            /* If it's not a valid list, try reading it as an atom, falling through to
+             * an error if it fails */
+            if (!xlu_cfg_replace_string(config, "usbdevice", &b_info->u.hvm.usbdevice, 0)) 
+                break;
+            /* FALLTHRU */
+        default:
+            fprintf(stderr,"xl: Unable to parse usbdevice.\n");
+            exit(-ERROR_FAIL);
+        }
         xlu_cfg_replace_string (config, "soundhw", &b_info->u.hvm.soundhw, 0);
         xlu_cfg_get_defbool(config, "xen_platform_pci",
                             &b_info->u.hvm.xen_platform_pci, 0);

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel

Ian Campbell

2012-12-05 11:22 UTC #5 lore raw

Re: [PATCH 2 of 4 v2] xl: Check for duplicate vncdisplay options, and return an error

In reply to George Dunlap

(seems that I forgot to hit send on this yesterday)

Quoted 7 lines · George Dunlap

# HG changeset patch
# User George Dunlap <[email protected]>
# Date 1354291984 0
# Node ID fa37bd276212a01e3c898b54a7f2385454c406a7
# Parent 722da032ac90c0e1a78b1154fa588bf295d1f009
xl: Check for duplicate vncdisplay options, and return an error

Quoted 6 lines

If the user has set a vnc display number both in vnclisten (with "xxxx:yy"), and with vncdisplay, throw an error.
Update man pages to match.
Signed-off-by George Dunlap <[email protected]>

Kind of nasty that we are stuck with both at the libxl interface, but oh well, to late now...

Acked + applied, thanks.

Quoted 55 lines

docs/man/xl.cfg.pod.5 +6 -0

@@ -350,11 +350,17 @@ other VNC-related settings.  The default
 
 Specifies the IP address, and optionally VNC display number, to use.
 
+NB that if you specify the display number here, you should not use
+vncdisplay.
+
 =item C<vncdisplay=DISPLAYNUM>
 
 Specifies the VNC display number to use.  The actual TCP port number
 will be DISPLAYNUM+5900.
 
+NB that you should not use this option if you set the displaynum in the 
+vnclisten string.
+
 =item C<vncunused=BOOLEAN>
 
 Requests that the VNC display setup search for a free TCP port to use.

tools/libxl/xl_cmdimpl.c +11 -0

@@ -1257,6 +1257,7 @@ skip_nic:
                     vfb->sdl.xauthority = strdup(p2 + 1);
                 }
             } while ((p = strtok(NULL, ",")) != NULL);
+
 skip_vfb:
             free(buf2);
             d_config->num_vfbs++;

@@ -1490,6 +1491,16 @@ skip_vfb:
         xlu_cfg_replace_string (config, "soundhw", &b_info->u.hvm.soundhw, 0);
         xlu_cfg_get_defbool(config, "xen_platform_pci",
                             &b_info->u.hvm.xen_platform_pci, 0);
+
+        if(b_info->u.hvm.vnc.listen
+           && b_info->u.hvm.vnc.display
+           && strchr(b_info->u.hvm.vnc.listen, ':') != NULL) {
+            fprintf(stderr,
+                    "ERROR: Display specified both in vnclisten"
+                    " and vncdisplay!\n");
+            exit (1);
+            
+        }
     }
 
     xlu_cfg_destroy(config);

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel