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

[beth kon]

Daniel Veillard wrote:

>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 at 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
>
>  
>
Yes, that makes perfect sense. Thanks.

-- 
Elizabeth Kon (Beth)
IBM Linux Technology Center
Open Hypervisor Team
email: eak at us.ibm.com