Re: [libvirt] [PATCH 2/6] Probe for QEMU machine types
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(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/