[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

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'>

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,


Yes, that makes perfect sense. Thanks.

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

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]