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@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list