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
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list