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

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


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