Re: [libvirt] PATCH: Add NUMA info to QEMU driver

20 May 2008

On Tue, May 20, 2008 at 02:15:59PM +0200, Jim Meyering wrote:
...
"Daniel P. Berrange" <berrange@redhat.com> wrote:
...
This patch includes NUMA topology info in the QEMU driver capabilities
XML output. It also implements the free memory driver APIs. This is done
with the LGPL'd  numactl library. The configure script probes for it and
only enables this functionality if it is found. The numactl library has
been around for quite a while - RHEL-3 vintage at least
Looks fine, modulo a few nits...
...
...
+        for (ncpus = 0, i = 0 ; i < MAX_CPUS ; i++)
+            if ((mask[(i / MAX_CPUS_MASK_SIZE)] >> (i % MAX_CPUS_MASK_SIZE)) & 1)
...
+        for (ncpus = 0, i = 0 ; i < MAX_CPUS ; i++)
+            if ((mask[(i / MAX_CPUS_MASK_SIZE)] >> (i % MAX_CPUS_MASK_SIZE)) & 1)
The above bit-is-set query deserves a macro.
Here's a update with that macro-ized, and the stupid fprintf()'s that rich
noticed removed.
...
If the above diagnostics needn't be different,
you could do this:
fail=0
   AC_CHECK_HEADER([numa.h], [], [fail=1])
   AC_CHECK_LIB([numa], [numa_available], [], [fail=1])
   test $fail = 1 &&
     AC_MSG_ERROR([You must install the numactl development package in order to compile libvirt])
[snip]
...
No big deal, but these are underquoted, and technically will cause trouble
if there's ever an m4 macro with a conflicting name, so this syntax is
preferred:
These 2 configure changes are candidates for global cleanup - the same duplicate
diagnostics & underquoting are present through-out


 configure.in      |   39 +++++++++++++++++++++++++++++++
 libvirt.spec.in   |    3 ++
 src/Makefile.am   |    2 +
 src/qemu_conf.c   |   67 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/qemu_driver.c |   64 +++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 175 insertions(+)

Dan

diff -r f912d3498d1b configure.in

--- a/configure.in	Tue May 20 10:49:20 2008 -0400
+++ b/configure.in	Tue May 20 11:54:16 2008 -0400
@@ -533,6 +533,40 @@
 AM_CONDITIONAL(HAVE_SELINUX, [test "$with_selinux" != "no"])
 AC_SUBST(SELINUX_CFLAGS)
 AC_SUBST(SELINUX_LIBS)
+
+dnl NUMA lib
+AC_ARG_WITH(numactl,
+  [  --with-numactl         use numactl for host topology info],
+  [],
+  [with_numactl=check])
+
+NUMACTL_CFLAGS=
+NUMACTL_LIBS=
+if test "$with_qemu" = "yes" -a "$with_numactl" != "no"; then
+  old_cflags="$CFLAGS"
+  old_libs="$LIBS"
+  if test "$with_numactl" = "check"; then
+    AC_CHECK_HEADER([numa.h],[],[with_numactl=no])
+    AC_CHECK_LIB(numa, numa_available,[],[with_numactl=no])
+    if test "$with_numactl" != "no"; then
+      with_numactl="yes"
+    fi
+  else
+    AC_CHECK_HEADER([numa.h],[],
+       [AC_MSG_ERROR([You must install the numactl development package in order to compile libvirt])])
+    AC_CHECK_LIB(numa, numa_available,[],
+       [AC_MSG_ERROR([You must install the numactl development package in order to compile and run libvirt])])
+  fi
+  CFLAGS="$old_cflags"
+  LIBS="$old_libs"
+fi
+if test "$with_numactl" = "yes"; then
+  NUMACTL_LIBS="-lnuma"
+  AC_DEFINE_UNQUOTED(HAVE_NUMACTL, 1, [whether Numactl is available for security])
+fi
+AM_CONDITIONAL(HAVE_NUMACTL, [test "$with_numactl" != "no"])
+AC_SUBST(NUMACTL_CFLAGS)
+AC_SUBST(NUMACTL_LIBS)
 
 dnl virsh libraries
 AC_CHECK_HEADERS([readline/readline.h])
@@ -1001,6 +1035,11 @@
 else
 AC_MSG_NOTICE([  selinux: no])
 fi
+if test "$with_numactl" = "yes" ; then
+AC_MSG_NOTICE([  numactl: $NUMACTL_CFLAGS $NUMACTL_LIBS])
+else
+AC_MSG_NOTICE([  numactl: no])
+fi
 AC_MSG_NOTICE([])
 AC_MSG_NOTICE([Miscellaneous])
 AC_MSG_NOTICE([])
diff -r f912d3498d1b libvirt.spec.in
--- a/libvirt.spec.in	Tue May 20 10:49:20 2008 -0400
+++ b/libvirt.spec.in	Tue May 20 11:54:16 2008 -0400
@@ -67,6 +67,9 @@
 BuildRequires: bridge-utils
 BuildRequires: qemu
 BuildRequires: cyrus-sasl-devel
+%if %{with_qemu}
+BuildRequires: numactl-devel
+%endif
 %if %{with_polkit}
 BuildRequires: PolicyKit-devel >= 0.6
 %endif
diff -r f912d3498d1b src/Makefile.am
--- a/src/Makefile.am	Tue May 20 10:49:20 2008 -0400
+++ b/src/Makefile.am	Tue May 20 11:54:16 2008 -0400
@@ -9,6 +9,7 @@
 	   $(GNUTLS_CFLAGS) \
 	   $(SASL_CFLAGS) \
 	   $(SELINUX_CFLAGS) \
+	   $(NUMACTL_CFLAGS) \
 	   -DBINDIR=\""$(libexecdir)"\" \
 	   -DSBINDIR=\""$(sbindir)"\" \
 	   -DSYSCONF_DIR="\"$(sysconfdir)\"" \
@@ -100,6 +101,7 @@
 
 libvirt_la_SOURCES = $(CLIENT_SOURCES) $(SERVER_SOURCES)
 libvirt_la_LIBADD = $(LIBXML_LIBS) $(GNUTLS_LIBS) $(SASL_LIBS) $(SELINUX_LIBS) \
+                    $(NUMACTL_LIBS) \
 		    @CYGWIN_EXTRA_LIBADD@ ../gnulib/lib/libgnu.la
 libvirt_la_LDFLAGS = -Wl,--version-script=$(srcdir)/libvirt_sym.version \
                      -version-info @LIBVIRT_VERSION_INFO@ \
diff -r f912d3498d1b src/qemu_conf.c
--- a/src/qemu_conf.c	Tue May 20 10:49:20 2008 -0400
+++ b/src/qemu_conf.c	Tue May 20 11:54:16 2008 -0400
@@ -42,6 +42,10 @@
 #include <libxml/xpath.h>
 #include <libxml/uri.h>
 
+#if HAVE_NUMACTL
+#include <numa.h>
+#endif
+
 #include "libvirt/virterror.h"
 
 #include "qemu_conf.h"
@@ -49,6 +53,7 @@
 #include "buf.h"
 #include "conf.h"
 #include "util.h"
+#include "memory.h"
 #include "verify.h"
 #include "c-ctype.h"
 
@@ -390,6 +395,65 @@
     return 0;
 }
 
+#if HAVE_NUMACTL
+#define MAX_CPUS 4096
+#define MAX_CPUS_MASK_SIZE (sizeof(unsigned long))
+#define MAX_CPUS_MASK_LEN (MAX_CPUS / MAX_CPUS_MASK_SIZE)
+#define MAX_CPUS_MASK_BYTES (MAX_CPUS / 8)
+
+#define MASK_CPU_ISSET(mask, cpu) \
+    (((mask)[((cpu) / MAX_CPUS_MASK_SIZE)] >> ((cpu) % MAX_CPUS_MASK_SIZE)) & 1)
+
+static int
+qemudCapsInitNUMA(virCapsPtr caps)
+{
+    int n, i;
+    unsigned long *mask = NULL;
+    int ncpus;
+    int *cpus = NULL;
+    int ret = -1;
+
+    if (numa_available() < 0)
+        return 0;
+
+    if (VIR_ALLOC_N(mask, MAX_CPUS_MASK_LEN) < 0)
+        goto cleanup;
+
+    for (n = 0 ; n <= numa_max_node() ; n++) {
+        if (numa_node_to_cpus(n, mask, MAX_CPUS_MASK_BYTES) < 0)
+            goto cleanup;
+
+        for (ncpus = 0, i = 0 ; i < MAX_CPUS ; i++)
+            if (MASK_CPU_ISSET(mask, i))
+                ncpus++;
+
+        if (VIR_ALLOC_N(cpus, ncpus) < 0)
+            goto cleanup;
+
+        for (ncpus = 0, i = 0 ; i < MAX_CPUS ; i++)
+            if (MASK_CPU_ISSET(mask, i))
+                cpus[ncpus++] = i;
+
+        if (virCapabilitiesAddHostNUMACell(caps,
+                                           n,
+                                           ncpus,
+                                           cpus) < 0)
+            goto cleanup;
+
+        VIR_FREE(cpus);
+    }
+
+    ret = 0;
+
+cleanup:
+    VIR_FREE(cpus);
+    VIR_FREE(mask);
+    return ret;
+}
+#else
+static int qemudCapsInitNUMA(virCapsPtr caps ATTRIBUTE_UNUSED) { return 0; }
+#endif
+
 virCapsPtr qemudCapsInit(void) {
     struct utsname utsname;
     virCapsPtr caps;
@@ -400,6 +464,9 @@
 
     if ((caps = virCapabilitiesNew(utsname.machine,
                                    0, 0)) == NULL)
+        goto no_memory;
+
+    if (qemudCapsInitNUMA(caps) < 0)
         goto no_memory;
 
     for (i = 0 ; i < (sizeof(arch_info_hvm)/sizeof(arch_info_hvm[0])) ; i++)
diff -r f912d3498d1b src/qemu_driver.c
--- a/src/qemu_driver.c	Tue May 20 10:49:20 2008 -0400
+++ b/src/qemu_driver.c	Tue May 20 11:54:16 2008 -0400
@@ -45,6 +45,10 @@
 #include <stdio.h>
 #include <sys/wait.h>
 #include <libxml/uri.h>
+
+#if HAVE_NUMACTL
+#include <numa.h>
+#endif
 
 #include "libvirt/virterror.h"
 
@@ -1619,6 +1623,61 @@
 }
 
 
+#if HAVE_NUMACTL
+static int
+qemudNodeGetCellsFreeMemory(virConnectPtr conn,
+                            unsigned long long *freeMems,
+                            int startCell,
+                            int maxCells)
+{
+    int n, lastCell, numCells;
+
+    if (numa_available() < 0) {
+        qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT,
+                         "%s", _("NUMA not supported on this host"));
+        return -1;
+    }
+    lastCell = startCell + maxCells - 1;
+    if (lastCell > numa_max_node())
+        lastCell = numa_max_node();
+
+    for (numCells = 0, n = startCell ; n <= lastCell ; n++) {
+        long long mem;
+        if (numa_node_size64(n, &mem) < 0) {
+            qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+                             "%s", _("Failed to query NUMA free memory"));
+            return -1;
+        }
+        freeMems[numCells++] = mem;
+    }
+    return numCells;
+}
+
+static unsigned long long
+qemudNodeGetFreeMemory (virConnectPtr conn)
+{
+    unsigned long long freeMem = 0;
+    int n;
+    if (numa_available() < 0) {
+        qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT,
+                         "%s", _("NUMA not supported on this host"));
+        return -1;
+    }
+
+    for (n = 0 ; n <= numa_max_node() ; n++) {
+        long long mem;
+        if (numa_node_size64(n, &mem) < 0) {
+            qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+                             "%s", _("Failed to query NUMA free memory"));
+            return -1;
+        }
+        freeMem += mem;
+    }
+
+    return freeMem;
+}
+
+#endif
 
 static int qemudGetProcessInfo(unsigned long long *cpuTime, int pid) {
     char proc[PATH_MAX];
@@ -3182,8 +3241,13 @@
     NULL, /* domainMigrateFinish */
     qemudDomainBlockStats, /* domainBlockStats */
     qemudDomainInterfaceStats, /* domainInterfaceStats */
+#if HAVE_NUMACTL
+    qemudNodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */
+    qemudNodeGetFreeMemory,  /* getFreeMemory */
+#else
     NULL, /* nodeGetCellsFreeMemory */
     NULL, /* getFreeMemory */
+#endif
 };
 
 static virNetworkDriver qemuNetworkDriver = {



-- 
|: Red Hat, Engineering, Boston   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

    