[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCH 1 of 4 v2] libxl: Combine device model arg build functions



# HG changeset patch
# User George Dunlap <george.dunlap@xxxxxxxxxxxxx>
# 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 <george.dunlap@xxxxxxxxxxxxx>

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -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)

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.