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

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

 



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@xxxxxxxxxx>

 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@xxxxxxxxxx

--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]