[libvirt] PATCH: Centralize some duplicated NUMA code

The QEMU and UML drivers currently duplicate a non-trivial chunk of NUMA code. This patch moves it all into src/nodeinfo.c which already has some other common NUMA code for capabilities initialization. In doing so it changes the functions to have a common 'nodeXXXX' naming scheme, to avoid clash with the public APIs using 'virXXXX'. Finally, these central functions can be directly used in the driver function tables, so we kill off some pointless wrappers and just add these funtions directly to drivers. libvirt_private.syms | 6 +- lxc_conf.c | 2 nodeinfo.c | 127 +++++++++++++++++++++++++++++++++++++++++++-------- nodeinfo.h | 11 +++- openvz_conf.c | 7 -- openvz_driver.c | 7 -- qemu_conf.c | 2 qemu_driver.c | 97 ++------------------------------------ uml_conf.c | 2 uml_driver.c | 84 +-------------------------------- vbox/vbox_tmpl.c | 7 -- 11 files changed, 138 insertions(+), 214 deletions(-) Daniel diff -r 91d1b6ebc112 src/libvirt_private.syms --- a/src/libvirt_private.syms Wed May 27 22:57:21 2009 +0100 +++ b/src/libvirt_private.syms Thu May 28 11:33:16 2009 +0100 @@ -220,8 +220,10 @@ virNetworkObjUnlock; # nodeinfo.h -virNodeInfoPopulate; -virCapsInitNUMA; +nodeGetInfo; +nodeCapsInitNUMA; +nodeGetCellsFreeMemory; +nodeGetFreeMemory; # node_device_conf.h diff -r 91d1b6ebc112 src/lxc_conf.c --- a/src/lxc_conf.c Wed May 27 22:57:21 2009 +0100 +++ b/src/lxc_conf.c Thu May 28 11:33:16 2009 +0100 @@ -46,7 +46,7 @@ virCapsPtr lxcCapsInit(void) 0, 0)) == NULL) goto no_memory; - if (virCapsInitNUMA(caps) < 0) + if (nodeCapsInitNUMA(caps) < 0) goto no_memory; /* XXX shouldn't 'borrow' KVM's prefix */ diff -r 91d1b6ebc112 src/nodeinfo.c --- a/src/nodeinfo.c Wed May 27 22:57:21 2009 +0100 +++ b/src/nodeinfo.c Thu May 28 11:33:16 2009 +0100 @@ -48,6 +48,10 @@ #define VIR_FROM_THIS VIR_FROM_NONE +#define nodeReportError(conn, code, fmt...) \ + virReportErrorHelper(conn, VIR_FROM_NONE, code, __FILE__, \ + __FUNCTION__, __LINE__, fmt) + #ifdef __linux__ #define CPUINFO_PATH "/proc/cpuinfo" @@ -73,9 +77,8 @@ int linuxNodeInfoCPUPopulate(virConnectP while (*buf && c_isspace(*buf)) buf++; if (*buf != ':') { - virRaiseError(conn, NULL, NULL, 0, VIR_ERR_INTERNAL_ERROR, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, - "%s", _("parsing cpuinfo processor")); + nodeReportError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("parsing cpuinfo processor")); return -1; } nodeinfo->cpus++; @@ -86,9 +89,8 @@ int linuxNodeInfoCPUPopulate(virConnectP while (*buf && c_isspace(*buf)) buf++; if (*buf != ':' || !buf[1]) { - virRaiseError(conn, NULL, NULL, 0, VIR_ERR_INTERNAL_ERROR, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, - "%s", _("parsing cpuinfo cpu MHz")); + nodeReportError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("parsing cpuinfo cpu MHz")); return -1; } if (virStrToLong_ui(buf+1, &p, 10, &ui) == 0 @@ -102,9 +104,8 @@ int linuxNodeInfoCPUPopulate(virConnectP while (*buf && c_isspace(*buf)) buf++; if (*buf != ':' || !buf[1]) { - virRaiseError(conn, NULL, NULL, 0, VIR_ERR_INTERNAL_ERROR, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, - "parsing cpuinfo cpu cores %c", *buf); + nodeReportError(conn, VIR_ERR_INTERNAL_ERROR, + "parsing cpuinfo cpu cores %c", *buf); return -1; } if (virStrToLong_ui(buf+1, &p, 10, &id) == 0 @@ -115,9 +116,8 @@ int linuxNodeInfoCPUPopulate(virConnectP } if (!nodeinfo->cpus) { - virRaiseError(conn, NULL, NULL, 0, VIR_ERR_INTERNAL_ERROR, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, - "%s", _("no cpus found")); + nodeReportError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("no cpus found")); return -1; } @@ -133,8 +133,8 @@ int linuxNodeInfoCPUPopulate(virConnectP #endif -int virNodeInfoPopulate(virConnectPtr conn, - virNodeInfoPtr nodeinfo) { +int nodeGetInfo(virConnectPtr conn, + virNodeInfoPtr nodeinfo) { #ifdef HAVE_UNAME struct utsname info; @@ -170,9 +170,8 @@ int virNodeInfoPopulate(virConnectPtr co } #else /* XXX Solaris will need an impl later if they port QEMU driver */ - virRaiseError(conn, NULL, NULL, 0, VIR_ERR_INTERNAL_ERROR, - VIR_ERR_ERROR, NULL, NULL, NULL, 0, 0, - "%s:%s not implemented on this platform\n", __FILE__, __FUNCTION__); + nodeError(conn, VIR_ERR_NO_SUPPORT, "%s" + _("node info not implemented on this platform")); return -1; #endif } @@ -189,7 +188,7 @@ int virNodeInfoPopulate(virConnectPtr co (((mask)[((cpu) / n_bits(*(mask)))] >> ((cpu) % n_bits(*(mask)))) & 1) int -virCapsInitNUMA(virCapsPtr caps) +nodeCapsInitNUMA(virCapsPtr caps) { int n; unsigned long *mask = NULL; @@ -237,6 +236,96 @@ cleanup: VIR_FREE(mask); return ret; } + + +int +nodeGetCellsFreeMemory(virConnectPtr conn, + unsigned long long *freeMems, + int startCell, + int maxCells) +{ + int n, lastCell, numCells; + int ret = -1; + int maxCell; + + if (numa_available() < 0) { + nodeReportError(conn, VIR_ERR_NO_SUPPORT, + "%s", _("NUMA not supported on this host")); + goto cleanup; + } + maxCell = numa_max_node(); + if (startCell > maxCell) { + nodeReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("start cell %d out of range (0-%d)"), + startCell, maxCell); + goto cleanup; + } + lastCell = startCell + maxCells - 1; + if (lastCell > maxCell) + lastCell = maxCell; + + for (numCells = 0, n = startCell ; n <= lastCell ; n++) { + long long mem; + if (numa_node_size64(n, &mem) < 0) { + nodeReportError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("Failed to query NUMA free memory")); + goto cleanup; + } + freeMems[numCells++] = mem; + } + ret = numCells; + +cleanup: + return ret; +} + +unsigned long long +nodeGetFreeMemory(virConnectPtr conn) +{ + unsigned long long freeMem = 0; + int n; + + if (numa_available() < 0) { + nodeReportError(conn, VIR_ERR_NO_SUPPORT, + "%s", _("NUMA not supported on this host")); + goto cleanup; + } + + for (n = 0 ; n <= numa_max_node() ; n++) { + long long mem; + if (numa_node_size64(n, &mem) < 0) { + nodeReportError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("Failed to query NUMA free memory")); + goto cleanup; + } + freeMem += mem; + } + +cleanup: + return freeMem; +} + #else -int virCapsInitNUMA(virCapsPtr caps ATTRIBUTE_UNUSED) { return 0; } +int nodeCapsInitNUMA(virCapsPtr caps ATTRIBUTE_UNUSED) { + return 0; +} + +int nodeGetCellsFreeMemory(virConnectPtr conn, + unsigned long long *freeMems ATTRIBUTE_UNUSED, + int startCell ATTRIBUTE_UNUSED, + int maxCells ATTRIBUTE_UNUSED) +{ + nodeReportError(conn, VIR_ERR_NO_SUPPORT, "%s", + _("NUMA memory information not available on this platform")); + return -1; +} + +unsigned long long nodeGetFreeMemory(virConnectPtr conn) +{ + nodeReportError(conn, VIR_ERR_NO_SUPPORT, "%s", + _("NUMA memory information not available on this platform")); + return 0; +} #endif + + diff -r 91d1b6ebc112 src/nodeinfo.h --- a/src/nodeinfo.h Wed May 27 22:57:21 2009 +0100 +++ b/src/nodeinfo.h Thu May 28 11:33:16 2009 +0100 @@ -27,7 +27,14 @@ #include "libvirt/libvirt.h" #include "capabilities.h" -int virNodeInfoPopulate(virConnectPtr conn, virNodeInfoPtr nodeinfo); -int virCapsInitNUMA(virCapsPtr caps); +int nodeGetInfo(virConnectPtr conn, virNodeInfoPtr nodeinfo); +int nodeCapsInitNUMA(virCapsPtr caps); + + +int nodeGetCellsFreeMemory(virConnectPtr conn, + unsigned long long *freeMems, + int startCell, + int maxCells); +unsigned long long nodeGetFreeMemory(virConnectPtr conn); #endif /* __VIR_NODEINFO_H__*/ diff -r 91d1b6ebc112 src/openvz_conf.c --- a/src/openvz_conf.c Wed May 27 22:57:21 2009 +0100 +++ b/src/openvz_conf.c Thu May 28 11:33:16 2009 +0100 @@ -148,7 +148,7 @@ virCapsPtr openvzCapsInit(void) 0, 0)) == NULL) goto no_memory; - if (virCapsInitNUMA(caps) < 0) + if (nodeCapsInitNUMA(caps) < 0) goto no_memory; virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x52, 0x54, 0x00 }); @@ -527,11 +527,8 @@ openvzGetNodeCPUs(void) { virNodeInfo nodeinfo; - if (virNodeInfoPopulate(NULL, &nodeinfo) < 0) { - openvzError(NULL, VIR_ERR_INTERNAL_ERROR, - "%s", _("Cound not read nodeinfo")); + if (nodeGetInfo(NULL, &nodeinfo) < 0) return 0; - } return nodeinfo.cpus; } diff -r 91d1b6ebc112 src/openvz_driver.c --- a/src/openvz_driver.c Wed May 27 22:57:21 2009 +0100 +++ b/src/openvz_driver.c Thu May 28 11:33:16 2009 +0100 @@ -1136,11 +1136,6 @@ static const char *openvzGetType(virConn return strdup("OpenVZ"); } -static int openvzGetNodeInfo(virConnectPtr conn, - virNodeInfoPtr nodeinfo) { - return virNodeInfoPopulate(conn, nodeinfo); -} - static char *openvzGetCapabilities(virConnectPtr conn) { struct openvz_driver *driver = conn->privateData; char *ret; @@ -1316,7 +1311,7 @@ static virDriver openvzDriver = { openvzGetVersion, /* version */ NULL, /* getHostname */ openvzGetMaxVCPUs, /* getMaxVcpus */ - openvzGetNodeInfo, /* nodeGetInfo */ + nodeGetInfo, /* nodeGetInfo */ openvzGetCapabilities, /* getCapabilities */ openvzListDomains, /* listDomains */ openvzNumDomains, /* numOfDomains */ diff -r 91d1b6ebc112 src/qemu_conf.c --- a/src/qemu_conf.c Wed May 27 22:57:21 2009 +0100 +++ b/src/qemu_conf.c Thu May 28 11:33:16 2009 +0100 @@ -377,7 +377,7 @@ virCapsPtr qemudCapsInit(void) { /* Using KVM's mac prefix for QEMU too */ virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x52, 0x54, 0x00 }); - if (virCapsInitNUMA(caps) < 0) + if (nodeCapsInitNUMA(caps) < 0) goto no_memory; virCapabilitiesAddHostMigrateTransport(caps, diff -r 91d1b6ebc112 src/qemu_driver.c --- a/src/qemu_driver.c Wed May 27 22:57:21 2009 +0100 +++ b/src/qemu_driver.c Thu May 28 11:33:16 2009 +0100 @@ -44,11 +44,6 @@ #include <sys/wait.h> #include <sys/ioctl.h> -#if HAVE_NUMACTL -#define NUMA_VERSION1_COMPATIBILITY 1 -#include <numa.h> -#endif - #if HAVE_SCHED_H #include <sched.h> #endif @@ -1099,7 +1094,7 @@ qemudInitCpus(virConnectPtr conn, int i, maxcpu = QEMUD_CPUMASK_LEN; virNodeInfo nodeinfo; - if (virNodeInfoPopulate(conn, &nodeinfo) < 0) + if (nodeGetInfo(conn, &nodeinfo) < 0) return -1; /* setaffinity fails if you set bits for CPUs which @@ -1869,11 +1864,6 @@ static int qemudGetMaxVCPUs(virConnectPt return -1; } -static int qemudGetNodeInfo(virConnectPtr conn, - virNodeInfoPtr nodeinfo) { - return virNodeInfoPopulate(conn, nodeinfo); -} - static char *qemudGetCapabilities(virConnectPtr conn) { struct qemud_driver *driver = conn->privateData; @@ -1890,76 +1880,6 @@ static char *qemudGetCapabilities(virCon } -#if HAVE_NUMACTL -static int -qemudNodeGetCellsFreeMemory(virConnectPtr conn, - unsigned long long *freeMems, - int startCell, - int maxCells) -{ - int n, lastCell, numCells; - int ret = -1; - int maxCell; - - if (numa_available() < 0) { - qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT, - "%s", _("NUMA not supported on this host")); - goto cleanup; - } - maxCell = numa_max_node(); - if (startCell > maxCell) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("start cell %d out of range (0-%d)"), - startCell, maxCell); - goto cleanup; - } - lastCell = startCell + maxCells - 1; - if (lastCell > maxCell) - lastCell = maxCell; - - 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")); - goto cleanup; - } - freeMems[numCells++] = mem; - } - ret = numCells; - -cleanup: - return ret; -} - -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")); - goto cleanup; - } - - 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")); - goto cleanup; - } - freeMem += mem; - } - -cleanup: - return freeMem; -} - -#endif - static int qemudGetProcessInfo(unsigned long long *cpuTime, int pid) { char proc[PATH_MAX]; FILE *pidinfo; @@ -2984,7 +2904,7 @@ qemudDomainPinVcpu(virDomainPtr dom, goto cleanup; } - if (virNodeInfoPopulate(dom->conn, &nodeinfo) < 0) + if (nodeGetInfo(dom->conn, &nodeinfo) < 0) goto cleanup; maxcpu = maplen * 8; @@ -3046,7 +2966,7 @@ qemudDomainGetVcpus(virDomainPtr dom, goto cleanup; } - if (virNodeInfoPopulate(dom->conn, &nodeinfo) < 0) + if (nodeGetInfo(dom->conn, &nodeinfo) < 0) goto cleanup; maxcpu = maplen * 8; @@ -5338,7 +5258,7 @@ static virDriver qemuDriver = { qemudGetVersion, /* version */ qemudGetHostname, /* getHostname */ qemudGetMaxVCPUs, /* getMaxVcpus */ - qemudGetNodeInfo, /* nodeGetInfo */ + nodeGetInfo, /* nodeGetInfo */ qemudGetCapabilities, /* getCapabilities */ qemudListDomains, /* listDomains */ qemudNumDomains, /* numOfDomains */ @@ -5392,13 +5312,8 @@ static virDriver qemuDriver = { qemudDomainInterfaceStats, /* domainInterfaceStats */ qemudDomainBlockPeek, /* domainBlockPeek */ qemudDomainMemoryPeek, /* domainMemoryPeek */ -#if HAVE_NUMACTL - qemudNodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ - qemudNodeGetFreeMemory, /* getFreeMemory */ -#else - NULL, /* nodeGetCellsFreeMemory */ - NULL, /* getFreeMemory */ -#endif + nodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ + nodeGetFreeMemory, /* getFreeMemory */ qemudDomainEventRegister, /* domainEventRegister */ qemudDomainEventDeregister, /* domainEventDeregister */ qemudDomainMigratePrepare2, /* domainMigratePrepare2 */ diff -r 91d1b6ebc112 src/uml_conf.c --- a/src/uml_conf.c Wed May 27 22:57:21 2009 +0100 +++ b/src/uml_conf.c Thu May 28 11:33:16 2009 +0100 @@ -62,7 +62,7 @@ virCapsPtr umlCapsInit(void) { 0, 0)) == NULL) goto no_memory; - if (virCapsInitNUMA(caps) < 0) + if (nodeCapsInitNUMA(caps) < 0) goto no_memory; if ((guest = virCapabilitiesAddGuest(caps, diff -r 91d1b6ebc112 src/uml_driver.c --- a/src/uml_driver.c Wed May 27 22:57:21 2009 +0100 +++ b/src/uml_driver.c Thu May 28 11:33:16 2009 +0100 @@ -45,11 +45,6 @@ #include <sys/ioctl.h> #include <sys/inotify.h> -#if HAVE_NUMACTL -#define NUMA_VERSION1_COMPATIBILITY 1 -#include <numa.h> -#endif - #include "uml_driver.h" #include "uml_conf.h" #include "event.h" @@ -935,11 +930,6 @@ static const char *umlGetType(virConnect return "UML"; } -static int umlGetNodeInfo(virConnectPtr conn, - virNodeInfoPtr nodeinfo) { - return virNodeInfoPopulate(conn, nodeinfo); -} - static char *umlGetCapabilities(virConnectPtr conn) { struct uml_driver *driver = (struct uml_driver *)conn->privateData; @@ -954,69 +944,6 @@ static char *umlGetCapabilities(virConne } -#if HAVE_NUMACTL -static int -umlNodeGetCellsFreeMemory(virConnectPtr conn, - unsigned long long *freeMems, - int startCell, - int maxCells) -{ - int n, lastCell, numCells; - int ret = -1; - - if (numa_available() < 0) { - umlReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT, - "%s", _("NUMA not supported on this host")); - goto cleanup; - } - 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) { - umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - "%s", _("Failed to query NUMA free memory")); - goto cleanup; - } - freeMems[numCells++] = mem; - } - ret = numCells; - -cleanup: - return ret; -} - -static unsigned long long -umlNodeGetFreeMemory (virConnectPtr conn) -{ - unsigned long long freeMem = 0; - unsigned long long ret = -1; - int n; - - if (numa_available() < 0) { - umlReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT, - "%s", _("NUMA not supported on this host")); - goto cleanup; - } - - for (n = 0 ; n <= numa_max_node() ; n++) { - long long mem; - if (numa_node_size64(n, &mem) < 0) { - umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - "%s", _("Failed to query NUMA free memory")); - goto cleanup; - } - freeMem += mem; - } - ret = freeMem; - -cleanup: - return ret; -} - -#endif static int umlGetProcessInfo(unsigned long long *cpuTime, int pid) { char proc[PATH_MAX]; @@ -1826,7 +1753,7 @@ static virDriver umlDriver = { umlGetVersion, /* version */ umlGetHostname, /* getHostname */ NULL, /* getMaxVcpus */ - umlGetNodeInfo, /* nodeGetInfo */ + nodeGetInfo, /* nodeGetInfo */ umlGetCapabilities, /* getCapabilities */ umlListDomains, /* listDomains */ umlNumDomains, /* numOfDomains */ @@ -1875,13 +1802,8 @@ static virDriver umlDriver = { NULL, /* domainInterfaceStats */ umlDomainBlockPeek, /* domainBlockPeek */ NULL, /* domainMemoryPeek */ -#if HAVE_NUMACTL - umlNodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ - umlNodeGetFreeMemory, /* getFreeMemory */ -#else - NULL, /* nodeGetCellsFreeMemory */ - NULL, /* getFreeMemory */ -#endif + virNodeGetCellsFreeMemory, /* nodeGetCellsFreeMemory */ + virNodeGetFreeMemory, /* getFreeMemory */ NULL, /* domainEventRegister */ NULL, /* domainEventDeregister */ NULL, /* domainMigratePrepare2 */ diff -r 91d1b6ebc112 src/vbox/vbox_tmpl.c --- a/src/vbox/vbox_tmpl.c Wed May 27 22:57:21 2009 +0100 +++ b/src/vbox/vbox_tmpl.c Thu May 28 11:33:16 2009 +0100 @@ -186,7 +186,7 @@ static virCapsPtr vboxCapsInit(void) { 0, 0)) == NULL) goto no_memory; - if (virCapsInitNUMA(caps) < 0) + if (nodeCapsInitNUMA(caps) < 0) goto no_memory; virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x08, 0x00, 0x27 }); @@ -413,9 +413,6 @@ static int vboxGetMaxVcpus(virConnectPtr return ret; } -static int vboxNodeGetInfo(virConnectPtr conn, virNodeInfoPtr nodeinfo) { - return virNodeInfoPopulate(conn, nodeinfo); -} static char *vboxGetCapabilities(virConnectPtr conn) { vboxGlobalData *data = conn->privateData; @@ -4909,7 +4906,7 @@ virDriver NAME(Driver) = { .version = vboxGetVersion, .getHostname = vboxGetHostname, .getMaxVcpus = vboxGetMaxVcpus, - .nodeGetInfo = vboxNodeGetInfo, + .nodeGetInfo = nodeGetInfo, .getCapabilities = vboxGetCapabilities, .listDomains = vboxListDomains, .numOfDomains = vboxNumOfDomains, -- |: 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 :|