Re: [libvirt] [PATCH 04/10] remove dependence on libnuma

On Thu, Nov 03, 2011 at 07:55:19PM +0800, Hu Tao wrote: > Since we use cpuset to manage numa, we can safely remove dependence > on libnuma. > --- > src/conf/domain_conf.c | 24 +---------- > src/conf/domain_conf.h | 1 - > src/qemu/qemu_process.c | 111 ----------------------------------------------- > 3 files changed, 1 insertions(+), 135 deletions(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 6e2d421..0cf3bb7 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -583,11 +583,6 @@ VIR_ENUM_IMPL(virDomainTimerMode, VIR_DOMAIN_TIMER_MODE_LAST, > "paravirt", > "smpsafe"); > > -VIR_ENUM_IMPL(virDomainNumatuneMemMode, VIR_DOMAIN_NUMATUNE_MEM_LAST, > - "strict", > - "preferred", > - "interleave"); > - > VIR_ENUM_IMPL(virDomainStartupPolicy, VIR_DOMAIN_STARTUP_POLICY_LAST, > "default", > "mandatory", > @@ -6834,20 +6829,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, > "%s", _("nodeset for NUMA memory tuning must be set")); > goto error; > } > - > - tmp = virXPathString("string(./numatune/memory/@mode)", ctxt); > - if (tmp) { > - if ((def->numatune.memory.mode = > - virDomainNumatuneMemModeTypeFromString(tmp)) < 0) { > - virDomainReportError(VIR_ERR_INTERNAL_ERROR, > - _("Unsupported NUMA memory tuning mode '%s'"), > - tmp); > - goto error; > - } > - VIR_FREE(tmp); > - } else { > - def->numatune.memory.mode = VIR_DOMAIN_NUMATUNE_MEM_STRICT; > - } > } > > n = virXPathNodeSet("./features/*", ctxt, &nodes); > @@ -10882,7 +10863,6 @@ virDomainDefFormatInternal(virDomainDefPtr def, > virBufferAddLit(buf, " </cputune>\n"); > > if (def->numatune.memory.nodemask) { > - const char *mode; > char *nodemask = NULL; > > virBufferAddLit(buf, " <numatune>\n"); > @@ -10894,9 +10874,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, > goto cleanup; > } > > - mode = virDomainNumatuneMemModeTypeToString(def->numatune.memory.mode); > - virBufferAsprintf(buf, " <memory mode='%s' nodeset='%s'/>\n", > - mode, nodemask); > + virBufferAsprintf(buf, " <memory nodeset='%s'/>\n", nodemask); > VIR_FREE(nodemask); > virBufferAddLit(buf, " </numatune>\n"); > } > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h > index f74f4bb..ca68437 100644 > --- a/src/conf/domain_conf.h > +++ b/src/conf/domain_conf.h > @@ -1353,7 +1353,6 @@ typedef virDomainNumatuneDef *virDomainNumatuneDefPtr; > struct _virDomainNumatuneDef { > struct { > char *nodemask; > - int mode; > } memory; > > /* Future NUMA tuning related stuff should go here. */ You can't remove this stuff from the XML - this is part of our public stability guarentee. The way it is modelled is not specific to libnuma anyway, so there shouldn't be this tie. > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c > index 47164f7..5969b34 100644 > --- a/src/qemu/qemu_process.c > +++ b/src/qemu/qemu_process.c > @@ -39,11 +39,6 @@ > #include "qemu_bridge_filter.h" > #include "qemu_migration.h" > > -#if HAVE_NUMACTL > -# define NUMA_VERSION1_COMPATIBILITY 1 > -# include <numa.h> > -#endif > - > #include "datatypes.h" > #include "logging.h" > #include "virterror_internal.h" > @@ -1314,109 +1309,6 @@ qemuProcessDetectVcpuPIDs(struct qemud_driver *driver, > return 0; > } > > - > -/* > - * Set NUMA memory policy for qemu process, to be run between > - * fork/exec of QEMU only. > - */ > -#if HAVE_NUMACTL > -static int > -qemuProcessInitNumaMemoryPolicy(virDomainObjPtr vm) > -{ > - nodemask_t mask; > - int mode = -1; > - int node = -1; > - int ret = -1; > - int i = 0; > - int maxnode = 0; > - bool warned = false; > - > - if (!vm->def->numatune.memory.nodemask) > - return 0; > - > - VIR_DEBUG("Setting NUMA memory policy"); > - > - if (numa_available() < 0) { > - qemuReportError(VIR_ERR_INTERNAL_ERROR, > - "%s", _("Host kernel is not aware of NUMA.")); > - return -1; > - } > - > - maxnode = numa_max_node() + 1; > - > - /* Convert nodemask to NUMA bitmask. */ > - nodemask_zero(&mask); > - for (i = 0; i < VIR_DOMAIN_CPUMASK_LEN; i++) { > - if (vm->def->numatune.memory.nodemask[i]) { > - if (i > NUMA_NUM_NODES) { > - qemuReportError(VIR_ERR_INTERNAL_ERROR, > - _("Host cannot support NUMA node %d"), i); > - return -1; > - } > - if (i > maxnode && !warned) { > - VIR_WARN("nodeset is out of range, there is only %d NUMA " > - "nodes on host", maxnode); > - warned = true; > - } > - nodemask_set(&mask, i); > - } > - } > - > - mode = vm->def->numatune.memory.mode; > - > - if (mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT) { > - numa_set_bind_policy(1); > - numa_set_membind(&mask); > - numa_set_bind_policy(0); > - } else if (mode == VIR_DOMAIN_NUMATUNE_MEM_PREFERRED) { > - int nnodes = 0; > - for (i = 0; i < NUMA_NUM_NODES; i++) { > - if (nodemask_isset(&mask, i)) { > - node = i; > - nnodes++; > - } > - } > - > - if (nnodes != 1) { > - qemuReportError(VIR_ERR_INTERNAL_ERROR, > - "%s", _("NUMA memory tuning in 'preferred' mode " > - "only supports single node")); > - goto cleanup; > - } > - > - numa_set_bind_policy(0); > - numa_set_preferred(node); > - } else if (mode == VIR_DOMAIN_NUMATUNE_MEM_INTERLEAVE) { > - numa_set_interleave_mask(&mask); > - } else { > - /* XXX: Shouldn't go here, as we already do checking when > - * parsing domain XML. > - */ > - qemuReportError(VIR_ERR_XML_ERROR, > - "%s", _("Invalid mode for memory NUMA tuning.")); > - goto cleanup; > - } > - > - ret = 0; > - > -cleanup: > - return ret; > -} > -#else > -static int > -qemuProcessInitNumaMemoryPolicy(virDomainObjPtr vm) > -{ > - if (vm->def->numatune.memory.nodemask) { > - qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", > - _("libvirt is compiled without NUMA tuning support")); > - > - return -1; > - } > - > - return 0; > -} > -#endif > - > /* > * To be run between fork/exec of QEMU only > */ > @@ -2179,9 +2071,6 @@ static int qemuProcessHook(void *data) > if (qemuProcessInitCpuAffinity(h->vm) < 0) > goto cleanup; > > - if (qemuProcessInitNumaMemoryPolicy(h->vm) < 0) > - return -1; > - > VIR_DEBUG("Setting up security labelling"); > if (virSecurityManagerSetProcessLabel(h->driver->securityManager, h->vm) < 0) > goto cleanup; NACK to this removal of libnuma. The libvirt QEMU driver still supports deployment on RHEL5 vintage hosts where there is no cgroups mechanism at all. I'm all for using the cpuset controller *if* it is available. We must fallback to libnuma if it is not present though, to prevent functional regressions for existing users of libvirt