[libvirt] PATCH: Add NUMA info to QEMU driver

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 configure.in | 39 ++++++++++++++++++++++++++++++ libvirt.spec.in | 3 ++ src/Makefile.am | 2 + src/qemu_conf.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu_driver.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 178 insertions(+) Regards, Daniel Index: src/qemu_conf.c =================================================================== RCS file: /data/cvs/libvirt/src/qemu_conf.c,v retrieving revision 1.64 diff -u -p -r1.64 qemu_conf.c --- src/qemu_conf.c 15 May 2008 20:07:34 -0000 1.64 +++ src/qemu_conf.c 15 May 2008 21:07:42 -0000 @@ -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> #define qemudLog(level, msg...) fprintf(stderr, msg) @@ -389,6 +394,67 @@ qemudCapsInitGuest(virCapsPtr caps, 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) +static int +qemudCapsInitNUMA(virCapsPtr caps) +{ + int n, i; + unsigned long *mask = NULL; + int ncpus; + int *cpus = NULL; + int ret = -1; + + fprintf(stderr, "Add numa\n"); + + if (numa_available() < 0) + return 0; + + fprintf(stderr, "Start\n"); + if (VIR_ALLOC_N(mask, MAX_CPUS_MASK_LEN) < 0) + goto cleanup; + + for (n = 0 ; n <= numa_max_node() ; n++) { + fprintf(stderr, "Do node %d\n", 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[(i / MAX_CPUS_MASK_SIZE)] >> (i % MAX_CPUS_MASK_SIZE)) & 1) + ncpus++; + + if (VIR_ALLOC_N(cpus, ncpus) < 0) + goto cleanup; + + for (ncpus = 0, i = 0 ; i < MAX_CPUS ; i++) + if ((mask[(i / MAX_CPUS_MASK_SIZE)] >> (i % MAX_CPUS_MASK_SIZE)) & 1) + cpus[ncpus++] = i; + + fprintf(stderr, "Do node %d %d\n", n, ncpus); + 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; @@ -401,6 +467,9 @@ virCapsPtr qemudCapsInit(void) { 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++) if (qemudCapsInitGuest(caps, utsname.machine, Index: src/qemu_driver.c =================================================================== RCS file: /data/cvs/libvirt/src/qemu_driver.c,v retrieving revision 1.74 diff -u -p -r1.74 qemu_driver.c --- src/qemu_driver.c 15 May 2008 16:11:40 -0000 1.74 +++ src/qemu_driver.c 15 May 2008 21:07:50 -0000 @@ -47,6 +47,10 @@ #include <sys/wait.h> #include <libxml/uri.h> +#if HAVE_NUMACTL +#include <numa.h> +#endif + #include "libvirt/virterror.h" #include "event.h" @@ -1605,6 +1609,62 @@ static char *qemudGetCapabilities(virCon } +#if HAVE_NUMACTL +static int +qemudNodeGetCellsFreeMemory(virConnectPtr conn, + unsigned long long *freeMems, + int startCell, + int maxCells) +{ + int n, lastCell, numCells; + fprintf(stderr, "Foo %d %d\n", startCell, maxCells); + 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; + } + fprintf(stderr, "baro %d %llu\n", n, mem); + 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]; @@ -3168,8 +3228,13 @@ static virDriver qemuDriver = { NULL, /* domainMigrateFinish */ qemudDomainBlockStats, /* domainBlockStats */ qemudDomainInterfaceStats, /* domainInterfaceStats */ +#if HAVE_NUMACTL + qemudNodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ + qemudNodeGetFreeMemory, /* getFreeMemory */ +#else NULL, /* nodeGetCellsFreeMemory */ NULL, /* getFreeMemory */ +#endif }; static virNetworkDriver qemuNetworkDriver = { Index: src/Makefile.am =================================================================== RCS file: /data/cvs/libvirt/src/Makefile.am,v retrieving revision 1.79 diff -u -p -r1.79 Makefile.am --- src/Makefile.am 29 Apr 2008 15:38:13 -0000 1.79 +++ src/Makefile.am 15 May 2008 21:07:57 -0000 @@ -9,6 +9,7 @@ INCLUDES = \ $(GNUTLS_CFLAGS) \ $(SASL_CFLAGS) \ $(SELINUX_CFLAGS) \ + $(NUMACTL_CFLAGS) \ -DBINDIR=\""$(libexecdir)"\" \ -DSBINDIR=\""$(sbindir)"\" \ -DSYSCONF_DIR="\"$(sysconfdir)\"" \ @@ -100,6 +101,7 @@ endif 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@ \ Index: configure.in =================================================================== RCS file: /data/cvs/libvirt/configure.in,v retrieving revision 1.143 diff -u -p -r1.143 configure.in --- configure.in 5 May 2008 19:58:56 -0000 1.143 +++ configure.in 15 May 2008 21:08:05 -0000 @@ -534,6 +534,40 @@ AM_CONDITIONAL(HAVE_SELINUX, [test "$wit 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 @@ AC_MSG_NOTICE([ selinux: $SELINUX_CFLAG 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([]) Index: libvirt.spec.in =================================================================== RCS file: /data/cvs/libvirt/libvirt.spec.in,v retrieving revision 1.83 diff -u -p -r1.83 libvirt.spec.in --- libvirt.spec.in 8 Apr 2008 16:45:57 -0000 1.83 +++ libvirt.spec.in 15 May 2008 21:08:14 -0000 @@ -67,6 +67,9 @@ BuildRequires: dnsmasq 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 -- |: 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 :|

On Thu, May 15, 2008 at 10:10:38PM +0100, Daniel P. Berrange 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
Fine, but do remove the debug messages before committing :-) Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://et.redhat.com/~rjones/virt-top

"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.
Index: configure.in =================================================================== RCS file: /data/cvs/libvirt/configure.in,v retrieving revision 1.143 diff -u -p -r1.143 configure.in --- configure.in 5 May 2008 19:58:56 -0000 1.143 +++ configure.in 15 May 2008 21:08:05 -0000 @@ -534,6 +534,40 @@ AM_CONDITIONAL(HAVE_SELINUX, [test "$wit 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])])
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])
+ 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)
[I hesitate to mention this, since there is so much existing code here that does it the same way, but eventually this may well bite someone, so best to start doing it right. ] 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: 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]) same with AC_CHECK_LIB and AC_ARG_WITH arguments: AC_ARG_WITH([numactl], AC_CHECK_LIB([numa], [numa_available],[],[with_numactl=no])

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

"Daniel P. Berrange" <berrange@redhat.com> wrote: ...
These 2 configure changes are candidates for global cleanup - the same duplicate diagnostics & underquoting are present through-out
Yep. Deferring it is fine, of course. That's why I said this: [I hesitate to mention this, since there is so much existing code here that does it the same way, but eventually this may well bite someone, so best to start doing it right. ]
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
Does that mean you'd prefer to keep the slightly different diagnostics above? Here's an alternative: 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])

On Tue, May 20, 2008 at 06:42:17PM +0200, Jim Meyering wrote:
"Daniel P. Berrange" <berrange@redhat.com> wrote:
+ 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
Does that mean you'd prefer to keep the slightly different diagnostics above?
No, i meant many of the other checks have similar duplication of messages so I wanted to change them all at once using the changed you suggest...
Here's an alternative:
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])
Dan. -- |: 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 :|

On Tue, May 20, 2008 at 05:01:31PM +0100, Daniel P. Berrange wrote:
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.
I've comitted this patch now. Regards, Daniel -- |: 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 :|
participants (3)
-
Daniel P. Berrange
-
Jim Meyering
-
Richard W.M. Jones