Re: [Libvir] [PATCH] Topology fix for "no cpus"

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(a)us.ibm.com&gt; > > 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