Re: [Xen-devel] [PATCH 1/2] libxl, Introduce the command line handler for the new qemu.

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Wed, 4 Aug 2010, anthony.perard@xxxxxxxxxx wrote:
> From: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> 
> This patch adds a function to check the version of the device model.
> Depending on the version of the DM, the command line arguments will be
> built differently.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> ---
>  tools/libxl/libxl.c       |  163 
> ++++++++++++++++++++++++++++++++++++++++++++-
>  tools/libxl/libxl_utils.c |   73 ++++++++++++++++++++
>  tools/libxl/libxl_utils.h |    5 ++
>  3 files changed, 240 insertions(+), 1 deletions(-)
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 02faa0b..19dba55 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -964,7 +964,7 @@ skip_autopass:
>      return 0;
>  }
> 
> -static char ** libxl_build_device_model_args(libxl_ctx *ctx,
> +static char ** libxl_build_device_model_args_old(libxl_ctx *ctx,
>                                               libxl_device_model_info *info,
>                                               libxl_device_nic *vifs,
>                                               int num_vifs)
> @@ -1098,10 +1098,171 @@ static char ** 
> libxl_build_device_model_args(libxl_ctx *ctx,
>      else
>          flexarray_set(dm_args, num++, "xenfv");
>      flexarray_set(dm_args, num++, NULL);
> +    return (char **) flexarray_contents(dm_args);
> +}
> +
> +static char ** libxl_build_device_model_args_new(libxl_ctx *ctx,
> +                                             libxl_device_model_info *info,
> +                                             libxl_device_nic *vifs,
> +                                             int num_vifs)
> +{
> +    int num = 0, i;
> +    int new_qemu = 0;
> +    flexarray_t *dm_args;
> +    int nb;
> +    libxl_device_disk *disks;
> +
> +    dm_args = flexarray_make(16, 1);
> +    if (!dm_args)
> +        return NULL;
> +
> +    new_qemu = libxl_check_device_model_version(ctx, info->device_model);
> +

do we actually need to call libxl_check_device_model_version again here?


> +    flexarray_set(dm_args, num++, "qemu-system-xen");
> +    flexarray_set(dm_args, num++, "-xen-domid");
> +
> +    flexarray_set(dm_args, num++, libxl_sprintf(ctx, "%d", info->domid));
> +
> +    if (info->dom_name) {
> +        flexarray_set(dm_args, num++, "-name");
> +        flexarray_set(dm_args, num++, info->dom_name);
> +    }
> +    if (info->vnc || info->vncdisplay || info->vnclisten || info->vncunused) 
> {
> +        int display = 0;
> +        const char *listen = "127.0.0.1";
> +
> +        flexarray_set(dm_args, num++, "-vnc");
> +
> +        if (info->vncdisplay) {
> +            display = info->vncdisplay;
> +            if (info->vnclisten && strchr(info->vnclisten, ':') == NULL) {
> +                listen = info->vnclisten;
> +            }
> +        } else if (info->vnclisten) {
> +            listen = info->vnclisten;
> +        }
> +
> +        if (strchr(listen, ':') != NULL)
> +            flexarray_set(dm_args, num++,
> +                    libxl_sprintf(ctx, "%s%s", listen,
> +                        info->vncunused ? ",to=99" : ""));
> +        else
> +            flexarray_set(dm_args, num++,
> +                    libxl_sprintf(ctx, "%s:%d%s", listen, display,
> +                        info->vncunused ? ",to=99" : ""));
> +    }
> +    if (info->sdl) {
> +        flexarray_set(dm_args, num++, "-sdl");
> +    }
> +    if (info->keymap) {
> +        flexarray_set(dm_args, num++, "-k");
> +        flexarray_set(dm_args, num++, info->keymap);
> +    }
> +    if (info->nographic && (!info->sdl && !info->vnc)) {
> +        flexarray_set(dm_args, num++, "-nographic");
> +    }
> +    if (info->serial) {
> +        flexarray_set(dm_args, num++, "-serial");
> +        flexarray_set(dm_args, num++, info->serial);
> +    }
> +    if (info->type == XENFV) {
> +        int ioemu_vifs = 0;
> 
> +        if (info->stdvga) {
> +                flexarray_set(dm_args, num++, "-vga");
> +                flexarray_set(dm_args, num++, "std");
> +        }
> +
> +        if (info->boot) {
> +            flexarray_set(dm_args, num++, "-boot");
> +            flexarray_set(dm_args, num++, libxl_sprintf(ctx, "order=%s", 
> info->boot));
> +        }
> +        if (info->usb || info->usbdevice) {
> +            flexarray_set(dm_args, num++, "-usb");
> +            if (info->usbdevice) {
> +                flexarray_set(dm_args, num++, "-usbdevice");
> +                flexarray_set(dm_args, num++, info->usbdevice);
> +            }
> +        }
> +        if (info->soundhw) {
> +            flexarray_set(dm_args, num++, "-soundhw");
> +            flexarray_set(dm_args, num++, info->soundhw);
> +        }
> +        if (!info->apic) {
> +            flexarray_set(dm_args, num++, "-no-acpi");
> +        }
> +        if (info->vcpus > 1) {
> +            flexarray_set(dm_args, num++, "-smp");
> +            if (info->vcpu_avail)
> +                flexarray_set(dm_args, num++, libxl_sprintf(ctx, 
> "%d,maxcpus=%d", info->vcpus, info->vcpu_avail));
> +            else
> +                flexarray_set(dm_args, num++, libxl_sprintf(ctx, "%d", 
> info->vcpus));
> +        }
> +        for (i = 0; i < num_vifs; i++) {
> +            if (vifs[i].nictype == NICTYPE_IOEMU) {
> +                char *smac = libxl_sprintf(ctx, 
> "%02x:%02x:%02x:%02x:%02x:%02x",
> +                                           vifs[i].mac[0], vifs[i].mac[1], 
> vifs[i].mac[2],
> +                                           vifs[i].mac[3], vifs[i].mac[4], 
> vifs[i].mac[5]);
> +                if (!vifs[i].ifname)
> +                    vifs[i].ifname = libxl_sprintf(ctx, "tap%d.%d", 
> info->domid, vifs[i].devid - 1);
> +                flexarray_set(dm_args, num++, "-net");
> +                flexarray_set(dm_args, num++, libxl_sprintf(ctx, 
> "nic,vlan=%d,macaddr=%s,model=%s",
> +                            vifs[i].devid, smac, vifs[i].model));
> +                flexarray_set(dm_args, num++, "-net");
> +                flexarray_set(dm_args, num++, libxl_sprintf(ctx, 
> "tap,vlan=%d,ifname=%s,script=%s",
> +                            vifs[i].devid, vifs[i].ifname, 
> "/etc/xen/scripts/qemu-ifup"));
> +                ioemu_vifs++;
> +            }
> +        }
> +        /* If we have no emulated nics, tell qemu not to create any */
> +        if ( ioemu_vifs == 0 ) {
> +            flexarray_set(dm_args, num++, "-net");
> +            flexarray_set(dm_args, num++, "none");
> +        }
> +    }
> +    if (info->saved_state) {
> +        flexarray_set(dm_args, num++, "-loadvm");
> +        flexarray_set(dm_args, num++, info->saved_state);
> +    }
> +    for (i = 0; info->extra && info->extra[i] != NULL; i++)
> +        flexarray_set(dm_args, num++, info->extra[i]);
> +    flexarray_set(dm_args, num++, "-M");
> +    if (info->type == XENPV)
> +        flexarray_set(dm_args, num++, "xenpv");
> +    else
> +        flexarray_set(dm_args, num++, "xenfv");
> +
> +    disks = libxl_device_disk_list(ctx, info->domid, &nb);
> +    for (; nb > 0; --nb, ++disks) {
> +        if ( disks->is_cdrom ) {
> +            flexarray_set(dm_args, num++, "-cdrom");
> +            flexarray_set(dm_args, num++, disks->physpath);
> +        }else{
> +            flexarray_set(dm_args, num++, libxl_sprintf(ctx, "-%s", 
> disks->virtpath));
> +            flexarray_set(dm_args, num++, disks->physpath);
> +        }
> +    }
> +
> +    flexarray_set(dm_args, num++, NULL);
>      return (char **) flexarray_contents(dm_args);
>  }
> 
> +static char ** libxl_build_device_model_args(libxl_ctx *ctx,
> +                                             libxl_device_model_info *info,
> +                                             libxl_device_nic *vifs,
> +                                             int num_vifs)
> +{
> +    int new_qemu;
> +
> +    new_qemu = libxl_check_device_model_version(ctx, info->device_model);
> +
> +    if (new_qemu) {
> +        return libxl_build_device_model_args_new(ctx, info, vifs, num_vifs);
> +    } else {
> +        return libxl_build_device_model_args_old(ctx, info, vifs, num_vifs);
> +    }
> +}
> +
>  void dm_xenstore_record_pid(void *for_spawn, pid_t innerchild)
>  {
>      libxl_device_model_starting *starting = for_spawn;
> diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
> index 0b330b2..5fff2cc 100644
> --- a/tools/libxl/libxl_utils.c
> +++ b/tools/libxl/libxl_utils.c
> @@ -532,3 +532,76 @@ int libxl_strtomac(const char *mac_s, uint8_t *mac)
>      }
>      return 0;
>  }
> +
> +int libxl_check_device_model_version(libxl_ctx *ctx, char *path)
> +{
> +    pid_t pid = -1;
> +    int pipefd[2];
> +    char buf[100];
> +    ssize_t i, count = 0;
> +    int status;
> +    char *abs_path = NULL;
> +
> +    abs_path = libxl_abs_path(ctx, path, libxl_private_bindir_path());
> +
> +    if (pipe(pipefd))
> +        return 0;
> +
> +    pid = fork();
> +    if (pid == -1) {
> +        return 0;
> +    }
> +
> +    if (!pid) {
> +        close(pipefd[0]);
> +        if (dup2(pipefd[1], STDOUT_FILENO) == -1)
> +            exit(1);
> +        execlp(abs_path, abs_path, "-h", NULL);
> +
> +        close(pipefd[1]);
> +        exit(127);
> +    }
> +
> +    close(pipefd[1]);
> +    if (abs_path != path)
> +        libxl_free(ctx, abs_path);
> +
> +    // attempt to get the first line of `qemu -h`
> +    while ((i = read(pipefd[0], buf + count, 99 - count)) > 0) {
> +        if (i + count > 90)
> +            break;
> +        for (int j = 0; j <  i; j++) {
> +            if (buf[j + count] == '\n')
> +                break;
> +        }
> +        count += i;
> +    }
> +    count += i;
> +    close(pipefd[0]);
> +    waitpid(pid, &status, 0);
> +    if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) {
> +        return 0;
> +    }
> +
> +    // Search for the new version or the old version:
> +    // QEMU emulator version 0.12.50, ...
> +    // QEMU PC emulator version 0.10.2, ...
> +    if (strncmp("QEMU", buf, 4) == 0) {
> +        char *v = strstr(buf, "version ");
> +        if (v) {
> +            int major, minor;
> +            char *endptr = NULL;
> +
> +            v += strlen("version ");
> +            major = strtol(v, &endptr, 10);
> +            if (major == 0 && endptr && *endptr == '.') {
> +                v = endptr + 1;
> +                minor = strtol(v, &endptr, 10);
> +                if (minor >= 12)
> +                    return 1;
> +            }
> +            return 0;
> +        }
> +    }
> +    return 0;
> +}

Please use C style comments.
I think Gianni is right about the string to search: it is probably
better to add a Xen specific string to qemu-dm, like "QEMU-DM", and rely
on that and the version (0.10.2 or older) to distinguish between the
two.

> diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
> index 98a912c..bf8b361 100644
> --- a/tools/libxl/libxl_utils.h
> +++ b/tools/libxl/libxl_utils.h
> @@ -68,5 +68,10 @@ int libxl_strtomac(const char *mac_s, uint8_t *mac);
>  int libxl_devid_to_device_net2(libxl_ctx *ctx, uint32_t domid,
>                                 const char *devid, libxl_device_net2 *net2);
> 
> +/* check the version of qemu
> + * return 1 if is the new one
> + * return 0 if is the old one or if there are an error */
> +int libxl_check_device_model_version(libxl_ctx *ctx, char *path);
> +
>  #endif
> 
> --
> 1.6.5
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel
> 

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