Hi Stefan, Thanks for reviewing the patch. On 11/29/2011 10:25 PM, Stefan Berger wrote:
On 11/29/2011 09:59 AM, Prerna Saxena wrote:From: Prerna Saxena<prerna@xxxxxxxxxxxxxxxxxx> Date: Mon, 3 Oct 2011 06:01:33 -0700 Subject: [PATCH 3/5] Add support for ppc64 qemu This enables libvirt to select the correct qemu binary (qemu-system-ppc64) for a guest vm based on arch 'ppc64'. Also, libvirt is enabled to correctly parse the list of supported PowerPC CPUs, generated by running 'qemu-system-ppc64 -cpu ?' Signed-off-by: Prerna Saxena<prerna@xxxxxxxxxxxxxxxxxx> --- src/qemu/qemu_capabilities.c | 64 ++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 64 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index c5fe41d..c2d3d93 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -185,6 +185,7 @@ static const struct qemu_arch_info const arch_info_hvm[] = { { "mipsel", 32, NULL, "qemu-system-mipsel", NULL, NULL, 0 }, { "sparc", 32, NULL, "qemu-system-sparc", NULL, NULL, 0 }, { "ppc", 32, NULL, "qemu-system-ppc", NULL, NULL, 0 }, + { "ppc64", 64, NULL, "qemu-system-ppc64", NULL, NULL, 0 }, { "itanium", 64, NULL, "qemu-system-ia64", NULL, NULL, 0 }, { "s390x", 64, NULL, "qemu-system-s390x", NULL, NULL, 0 }, }; @@ -477,6 +478,67 @@ error: return -1; } +/* ppc64 parser. + * Format : PowerPC<machine> <description> + */ +static int +qemuCapsParsePPCModels(const char *output, + unsigned int *retcount, + const char ***retcpus) +{ + const char *p = output; + const char *next; + unsigned int count = 0; + const char **cpus = NULL; + int i; + do { + const char *t; + + if ((next = strchr(p, '\n'))) + next++; + + if (!STRPREFIX(p, "PowerPC ")) + continue;If what you are parsing here is indeed corrupted, you may end up in an endless loop with p not moving anymore. You have to advance 'p' at least by 1 and have a check before whether *p == '\0' to get out of it. You may also want to put this check before the calculation of 'next'.
When the execution gets to a 'continue', the do..while loop will resume execution from next iteration after processing the conditional check. In this case, whe condition in 'while' loop is :
while ((p =next)) which automatically advances 'p'.
+ + /* Skip the preceding sub-string "PowerPC " */ + p += 8; + + /*Malformed string, does not obey the format 'PowerPC<model> <desc>'*/ + if (!(t = strchr(p, ' ')) || (next&& t>= next)) + continue; + + if (*p == '\0' || *p == '\n') + continue;*p = '\0' presumably is end-of-string -- shouldn't we 'break' here ?
Agree. The reason it works fine in its present form is 'cos when p becomes empty, 'next' is empty too, and the loop condition fails which causes the do..while loop to terminate.
+ + if (retcpus) {if retcpus is not provided you may want to return -1 earlier+ unsigned int len; + + if (VIR_REALLOC_N(cpus, count + 1)< 0)virReportOOMError();+ goto error; + + if (t) + len = t - p - 1;len would remain uninitialized if t is not set, but t at this point must be set due to the check above. Remove 'if(t)'.
Will do.
+ + if (!(cpus[count] = strndup(p, len)))virReportOOMError();
I need to explicitly free cpus[] array, hence hadnt invoked this routine. I'll call this just before return '-1'.
+ goto error; + } + count++; + } while ((p = next)); + + if (retcount) + *retcount = count; + if (retcpus) + *retcpus = cpus; + return 0; + +error: + if (cpus) { + for (i = 0; i< count; i++) + VIR_FREE(cpus[i]); + } + VIR_FREE(cpus); + return -1; +} int qemuCapsProbeCPUModels(const char *qemu, @@ -497,6 +559,8 @@ qemuCapsProbeCPUModels(const char *qemu, if (STREQ(arch, "i686") || STREQ(arch, "x86_64")) parse = qemuCapsParseX86Models; + else if (STREQ(arch, "ppc64")) + parse = qemuCapsParsePPCModels; else { VIR_DEBUG("don't know how to parse %s CPU models", arch); return 0;
-- Prerna Saxena Linux Technology Centre, IBM Systems and Technology Lab, Bangalore, India -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list