On Fri, Oct 05, 2007 at 03:55:14PM -0400, beth kon wrote:

I was able to test on a 128-way NUMA box and found a bug. My code did not handle the case of no cpus being associated with a node. I decided to represent (pretty straightforward decision :-) no cpus as follows in the xml...

<cell id='2'> <cpus num='0'> </cpus> </cell>

Here is the patch...

Signed-off-by: Beth Kon <eak@us.ibm.com>

Hi Beth, the patch makes sense but I think a small improvement is in order:

diff -urpN libvirt.orig/src/xend_internal.c libvirt/src/xend_internal.c --- libvirt.orig/src/xend_internal.c 2007-10-03 19:27:25.000000000 -0700 +++ libvirt/src/xend_internal.c 2007-10-04 05:41:13.000000000 -0700 @@ -1989,6 +1989,15 @@ sexpr_to_xend_topology_xml(virConnectPtr /* get list of cpus associated w/ single cell */ while (1) { if ((len = getNumber(offset, &cpuNum)) < 0) { + if (!strncmp (offset, "no cpus", 7)){ + *(cpuIdsPtr++) = -1; + break; + } else { + virXendError(conn, VIR_ERR_XEN_CALL, "topology string syntax error"); + goto error; + } + } + if ((len = getNumber(offset, &cpuNum)) < 0) {

Seems to me that at this point the test should read if (len < 0) { as getNumber has no side effect and offset or cpuNum are not changed. Actually I would just move the len = getNumber(offset, &cpuNum) as a separate statement out at the beginning of the block of the while loop for clarity of the code, Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/