[libvirt] [PATCH 2/6] Probe for QEMU machine types

[Daniel Veillard]

On Thu, Jul 23, 2009 at 06:34:40PM +0100, Mark McLoughlin wrote:
> Currently we hardcode the QEMU machine types. We should really just
> parse the output of 'qemu -M ?' so the lists don't get out of sync.

  yes that makes sense, maintaining that list in libvirt itself
is a garanteed way to break things, just looking at the current version
our ppc list lists only 3 machine types while the binary reports 7 !

> xenner doesn't support '-M ?', so we still need to hardcode that.
> 
> The horrible (const char *const *) is removed in a subsequent patch.
> 
> * src/qemu_conf.c: kill the arch_info*machines tables, retain the
>   hardcoded xenner machine type, add qemudProbeMachineTypes() to
>   run and parse 'qemu -M ?' and use it in qemudCapsInitGuest()
> ---
[...]
> +static int
> +qemudParseMachineTypesStr(const char *output,
> +                          char ***machines,
> +                          int *nmachines)
> +{
> +    const char *p = output;
> +    const char *next;
> +    char **list = NULL;
> +    int i, nitems = 0;
> +
> +    do {
> +        const char *t;
> +        char *machine;
> +
> +        if ((next = strchr(p, '\n')))
> +            ++next;
> +
> +        if (STRPREFIX(p, "Supported machines are:"))
> +            continue;

  It's supposed to happen only on the first line, maybe suppressing the
first line if it ends with ':' is a safer bet in case the description
changes in the future (might be localized for example). But good enough
for now, if only they had used a structured description ...

> +        if (!(t = strchr(p, ' ')) || (next && t >= next))
> +            continue;

  Are we garanteed it will always be spaces, seems the case in what I
looked but maybe tabs might break this code. Still good enough for now.

> +        if (!(machine = strndup(p, t - p)))
> +            goto error;
> +
> +        if (VIR_REALLOC_N(list, nitems + 1) < 0) {
> +            VIR_FREE(machine);
> +            goto error;
> +        }

  Realloc in a loop, but I don't see how to avoid this except going
twice though the loop ... anyway this should be small and infrequent.

[...]
> +static int
> +qemudProbeMachineTypes(const char *binary,
> +                       char ***machines,
> +                       int *nmachines)
> +{
> +    const char *const qemuarg[] = { binary, "-M", "?", NULL };
> +    const char *const qemuenv[] = { "LC_ALL=C", NULL };

  okay we force the locale ...

ACK, this is really an improvement, just that parsing unstructured data sucks,

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/