Re: [libvirt] [PATCH] fix numa-related (and kernel-dependent) test failures

On Fri, Dec 19, 2008 at 07:57:58PM +0100, Jim Meyering wrote: Most of those Makefile.am changes are unneccessary. Only targets which compile source files using numactl needed the NUMA_CFLAGS/LIBS adding. Specifically libvirt_driver.la and libvirt_lxc. The libnuma ABI doesn't leak out into other things linking to libvirt.so or the drivers, so no need to add link flags for libvirtd, virsh or the tests / examples. All the source files wre missing #include "nodeinfo.h" to actually get the definition of virCapsInitNUMA, so not entirely sure how it managed to compile for you ? Here's a cut down patch removing the unneccessary makefile.am changes and adding the missing includes, which compiles & links for me. Daniel Index: src/Makefile.am =================================================================== RCS file: /data/cvs/libvirt/src/Makefile.am,v retrieving revision 1.118 diff -u -p -r1.118 Makefile.am --- src/Makefile.am 17 Dec 2008 21:39:41 -0000 1.118 +++ src/Makefile.am 21 Dec 2008 17:45:55 -0000 @@ -55,7 +55,7 @@ UTIL_SOURCES = \ xml.c xml.h # Internal generic driver infrastructure -DRIVER_SOURCES = \ +DRIVER_SOURCES = \ driver.c driver.h \ internal.h \ datatypes.c datatypes.h \ @@ -147,7 +147,7 @@ STORAGE_DRIVER_FS_SOURCES = \ storage_backend_fs.h storage_backend_fs.c STORAGE_DRIVER_LVM_SOURCES = \ - storage_backend_logical.h \ + storage_backend_logical.h \ storage_backend_logical.c STORAGE_DRIVER_ISCSI_SOURCES = \ @@ -189,8 +189,8 @@ libvirt_driver_la_SOURCES = \ $(STORAGE_CONF_SOURCES) \ $(NODE_DEVICE_CONF_SOURCES) -libvirt_driver_la_CFLAGS = $(XEN_CFLAGS) -libvirt_driver_la_LDFLAGS = $(XEN_LIBS) +libvirt_driver_la_CFLAGS = $(XEN_CFLAGS) $(NUMACTL_CFLAGS) +libvirt_driver_la_LDFLAGS = $(XEN_LIBS) $(NUMACTL_LIBS) if WITH_TEST if WITH_DRIVER_MODULES @@ -430,8 +430,8 @@ virsh_SOURCES = \ virsh.c virsh_LDFLAGS = $(WARN_CFLAGS) $(COVERAGE_LDFLAGS) -virsh_LDADD = \ - $(STATIC_BINARIES) \ +virsh_LDADD = \ + $(STATIC_BINARIES) \ $(WARN_CFLAGS) \ libvirt.la \ ../gnulib/lib/libgnu.la \ @@ -518,11 +518,11 @@ libexec_PROGRAMS += libvirt_lxc libvirt_lxc_SOURCES = \ $(LXC_CONTROLLER_SOURCES) \ - $(UTIL_SOURCES) \ + $(UTIL_SOURCES) \ $(DOMAIN_CONF_SOURCES) libvirt_lxc_LDFLAGS = $(WARN_CFLAGS) $(COVERAGE_LDCFLAGS) -libvirt_lxc_LDADD = $(LIBXML_LIBS) ../gnulib/lib/libgnu.la -libvirt_lxc_CFLAGS = $(LIBPARTED_CFLAGS) +libvirt_lxc_LDADD = $(LIBXML_LIBS) $(NUMACTL_LIBS) ../gnulib/lib/libgnu.la +libvirt_lxc_CFLAGS = $(LIBPARTED_CFLAGS) $(NUMACTL_CFLAGS) endif endif EXTRA_DIST += $(LXC_CONTROLLER_SOURCES) Index: src/libvirt_sym.version.in =================================================================== RCS file: /data/cvs/libvirt/src/libvirt_sym.version.in,v retrieving revision 1.15 diff -u -p -r1.15 libvirt_sym.version.in --- src/libvirt_sym.version.in 20 Dec 2008 13:09:45 -0000 1.15 +++ src/libvirt_sym.version.in 21 Dec 2008 17:45:55 -0000 @@ -491,6 +491,7 @@ LIBVIRT_PRIVATE_@VERSION@ { # nodeinfo.h virNodeInfoPopulate; + virCapsInitNUMA; # node_device_conf.h Index: src/lxc_conf.c =================================================================== RCS file: /data/cvs/libvirt/src/lxc_conf.c,v retrieving revision 1.23 diff -u -p -r1.23 lxc_conf.c --- src/lxc_conf.c 7 Nov 2008 16:43:58 -0000 1.23 +++ src/lxc_conf.c 21 Dec 2008 17:45:55 -0000 @@ -29,6 +29,7 @@ #include "virterror_internal.h" #include "lxc_conf.h" +#include "nodeinfo.h" /* Functions */ virCapsPtr lxcCapsInit(void) @@ -43,6 +44,9 @@ virCapsPtr lxcCapsInit(void) 0, 0)) == NULL) goto no_memory; + if (virCapsInitNUMA(caps) < 0) + goto no_memory; + /* XXX shouldn't 'borrow' KVM's prefix */ virCapabilitiesSetMacPrefix(caps, (unsigned char []){ 0x52, 0x54, 0x00 }); Index: src/nodeinfo.c =================================================================== RCS file: /data/cvs/libvirt/src/nodeinfo.c,v retrieving revision 1.14 diff -u -p -r1.14 nodeinfo.c --- src/nodeinfo.c 4 Nov 2008 22:30:33 -0000 1.14 +++ src/nodeinfo.c 21 Dec 2008 17:45:55 -0000 @@ -26,13 +26,22 @@ #include <stdio.h> #include <string.h> #include <stdlib.h> +#include <stdint.h> #include <errno.h> -#include "c-ctype.h" + +#if HAVE_NUMACTL +# define NUMA_VERSION1_COMPATIBILITY 1 +# include <numa.h> +#endif #ifdef HAVE_SYS_UTSNAME_H #include <sys/utsname.h> #endif +#include "memory.h" + +#include "c-ctype.h" + #include "virterror_internal.h" #include "nodeinfo.h" #include "physmem.h" @@ -171,3 +180,67 @@ int virNodeInfoPopulate(virConnectPtr co return -1; #endif } + +#if HAVE_NUMACTL +# if LIBNUMA_API_VERSION <= 1 +# define NUMA_MAX_N_CPUS 4096 +# else +# define NUMA_MAX_N_CPUS (numa_all_cpus_ptr->size) +# endif + +# define n_bits(var) (8 * sizeof(var)) +# define MASK_CPU_ISSET(mask, cpu) \ + (((mask)[((cpu) / n_bits(*(mask)))] >> ((cpu) % n_bits(*(mask)))) & 1) + +int +virCapsInitNUMA(virCapsPtr caps) +{ + int n; + uint64_t *mask = NULL; + int *cpus = NULL; + int ret = -1; + int max_n_cpus = NUMA_MAX_N_CPUS; + + if (numa_available() < 0) + return 0; + + int mask_n_bytes = max_n_cpus / 8; + if (VIR_ALLOC_N(mask, mask_n_bytes / sizeof *mask) < 0) + goto cleanup; + + for (n = 0 ; n <= numa_max_node() ; n++) { + int i; + int ncpus; + if (numa_node_to_cpus(n, mask, mask_n_bytes) < 0) + goto cleanup; + + for (ncpus = 0, i = 0 ; i < max_n_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_n_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 +int virCapsInitNUMA(virCapsPtr caps ATTRIBUTE_UNUSED) { return 0; } +#endif Index: src/nodeinfo.h =================================================================== RCS file: /data/cvs/libvirt/src/nodeinfo.h,v retrieving revision 1.3 diff -u -p -r1.3 nodeinfo.h --- src/nodeinfo.h 20 Aug 2008 20:48:36 -0000 1.3 +++ src/nodeinfo.h 21 Dec 2008 17:45:55 -0000 @@ -1,7 +1,7 @@ /* * nodeinfo.c: Helper routines for OS specific node information * - * Copyright (C) 2006, 2007 Red Hat, Inc. + * Copyright (C) 2006-2008 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -25,7 +25,9 @@ #define __VIR_NODEINFO_H__ #include "libvirt/libvirt.h" +#include "capabilities.h" int virNodeInfoPopulate(virConnectPtr conn, virNodeInfoPtr nodeinfo); +int virCapsInitNUMA(virCapsPtr caps); #endif /* __VIR_NODEINFO_H__*/ Index: src/openvz_conf.c =================================================================== RCS file: /data/cvs/libvirt/src/openvz_conf.c,v retrieving revision 1.53 diff -u -p -r1.53 openvz_conf.c --- src/openvz_conf.c 17 Dec 2008 21:13:19 -0000 1.53 +++ src/openvz_conf.c 21 Dec 2008 17:45:55 -0000 @@ -146,6 +146,9 @@ virCapsPtr openvzCapsInit(void) 0, 0)) == NULL) goto no_memory; + if (virCapsInitNUMA(caps) < 0) + goto no_memory; + virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x52, 0x54, 0x00 }); if ((guest = virCapabilitiesAddGuest(caps, Index: src/qemu_conf.c =================================================================== RCS file: /data/cvs/libvirt/src/qemu_conf.c,v retrieving revision 1.117 diff -u -p -r1.117 qemu_conf.c --- src/qemu_conf.c 20 Dec 2008 13:09:45 -0000 1.117 +++ src/qemu_conf.c 21 Dec 2008 17:45:55 -0000 @@ -36,11 +36,6 @@ #include <arpa/inet.h> #include <sys/utsname.h> -#if HAVE_NUMACTL -#define NUMA_VERSION1_COMPATIBILITY 1 -#include <numa.h> -#endif - #include "virterror_internal.h" #include "qemu_conf.h" #include "uuid.h" @@ -51,6 +46,7 @@ #include "verify.h" #include "datatypes.h" #include "xml.h" +#include "nodeinfo.h" VIR_ENUM_DECL(virDomainDiskQEMUBus) VIR_ENUM_IMPL(virDomainDiskQEMUBus, VIR_DOMAIN_DISK_BUS_LAST, @@ -300,66 +296,6 @@ qemudCapsInitGuest(virCapsPtr caps, return 0; } -#if HAVE_NUMACTL -#define MAX_CPUS 4096 -#define MAX_CPUS_MASK_SIZE (sizeof(unsigned long)) -#define MAX_CPUS_MASK_BITS (MAX_CPUS_MASK_SIZE * 8) -#define MAX_CPUS_MASK_LEN (MAX_CPUS / (MAX_CPUS_MASK_BITS)) - -#define MASK_CPU_ISSET(mask, cpu) \ - (((mask)[((cpu) / MAX_CPUS_MASK_BITS)] >> ((cpu) % MAX_CPUS_MASK_BITS)) & 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++) { - int mask_n_bytes = numa_all_cpus_ptr->size / 8; - if (numa_node_to_cpus(n, mask, mask_n_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; @@ -375,7 +311,7 @@ virCapsPtr qemudCapsInit(void) { /* Using KVM's mac prefix for QEMU too */ virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x52, 0x54, 0x00 }); - if (qemudCapsInitNUMA(caps) < 0) + if (virCapsInitNUMA(caps) < 0) goto no_memory; for (i = 0 ; i < ARRAY_CARDINALITY(arch_info_hvm) ; i++) Index: src/uml_conf.c =================================================================== RCS file: /data/cvs/libvirt/src/uml_conf.c,v retrieving revision 1.4 diff -u -p -r1.4 uml_conf.c --- src/uml_conf.c 17 Dec 2008 07:05:46 -0000 1.4 +++ src/uml_conf.c 21 Dec 2008 17:45:55 -0000 @@ -36,11 +36,6 @@ #include <arpa/inet.h> #include <sys/utsname.h> -#if HAVE_NUMACTL -#define NUMA_VERSION1_COMPATIBILITY 1 -#include <numa.h> -#endif - #include "uml_conf.h" #include "uuid.h" #include "buf.h" @@ -48,72 +43,10 @@ #include "util.h" #include "memory.h" #include "verify.h" - +#include "nodeinfo.h" #define umlLog(level, msg...) fprintf(stderr, msg) - - -#if HAVE_NUMACTL -#define MAX_CPUS 4096 -#define MAX_CPUS_MASK_SIZE (sizeof(unsigned long)) -#define MAX_CPUS_MASK_BITS (MAX_CPUS_MASK_SIZE * 8) -#define MAX_CPUS_MASK_LEN (MAX_CPUS / (MAX_CPUS_MASK_BITS)) - -#define MASK_CPU_ISSET(mask, cpu) \ - (((mask)[((cpu) / MAX_CPUS_MASK_BITS)] >> ((cpu) % MAX_CPUS_MASK_BITS)) & 1) - -static int -umlCapsInitNUMA(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++) { - int mask_n_bytes = numa_all_cpus_ptr->size / 8; - if (numa_node_to_cpus(n, mask, mask_n_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 umlCapsInitNUMA(virCapsPtr caps ATTRIBUTE_UNUSED) { return 0; } -#endif - virCapsPtr umlCapsInit(void) { struct utsname utsname; virCapsPtr caps; @@ -126,7 +59,7 @@ virCapsPtr umlCapsInit(void) { 0, 0)) == NULL) goto no_memory; - if (umlCapsInitNUMA(caps) < 0) + if (virCapsInitNUMA(caps) < 0) goto no_memory; if ((guest = virCapabilitiesAddGuest(caps, -- |: Red Hat, Engineering, London -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 :|