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

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



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


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