"Daniel P. Berrange" <berrange@xxxxxxxxxx> wrote: > On Sat, Jan 19, 2008 at 09:21:07PM +0100, Jim Meyering wrote: >> "Daniel P. Berrange" <berrange@xxxxxxxxxx> wrote: >> >> > Xen 3.2.0 removed the sockets_per_node field from the nodeinfo data returned >> > by XenD. I previously add compat for this, but got it in the wrong place >> > so it would most likely end up in a divide-by-zero error. This patch >> > re-arranges it to be correct. >> > >> > diff -rup libvirt-0.4.0.orig/src/xend_internal.c libvirt-0.4.0.new/src/xend_internal.c >> > --- libvirt-0.4.0.orig/src/xend_internal.c 2007-12-17 18:05:27.000000000 -0500 >> > +++ libvirt-0.4.0.new/src/xend_internal.c 2008-01-18 21:13:30.000000000 -0500 >> > @@ -1907,6 +1907,9 @@ sexpr_to_xend_node_info(struct sexpr *ro >> > info->mhz = sexpr_int(root, "node/cpu_mhz"); >> > info->nodes = sexpr_int(root, "node/nr_nodes"); >> > info->sockets = sexpr_int(root, "node/sockets_per_node"); >> > + info->cores = sexpr_int(root, "node/cores_per_socket"); >> > + info->threads = sexpr_int(root, "node/threads_per_core"); >> > + >> > /* Xen 3.2.0 replaces sockets_per_node with 'nr_cpus'. >> > * Old Xen calculated sockets_per_node using its internal >> > * nr_cpus / (nodes*cores*threads), so fake it ourselves >> > @@ -1921,8 +1924,6 @@ sexpr_to_xend_node_info(struct sexpr *ro >> > if (info->sockets == 0) >> > info->sockets = 1; >> > } >> > - info->cores = sexpr_int(root, "node/cores_per_socket"); >> > - info->threads = sexpr_int(root, "node/threads_per_core"); >> > return (0); >> > } >> >> ACK. >> >> Now that you mention divide-by-zero, >> is there something that guarantees none of the >> terms in the denominator there is zero? >> >> info->sockets = nr_cpus / (info->nodes * info->cores * info->threads); > > You always have at least 1 numa node, at least 1 core, and at least 1 > hyperthread, so you'd only get a zero in those if there was a bug in the > Xen hypervisor Yes, I understood that any of those being zero would not make sense. I mean, what if a caller of sexpr_to_xend_node_info were to provide a bogus root sexpr? Is it worth the added comparison to protect against that? int p = info->nodes * info->cores * info->threads; if (p == 0) return -1; info->sockets = nr_cpus / p; -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list