On 2013年03月01日 14:52, Gao feng wrote:
Intend to reduce the redundant code,use virSetupNumaMemoryPolicy
to replace virLXCControllerSetupNUMAPolicy and
qemuProcessInitNumaMemoryPolicy.
Signed-off-by: Gao feng<gaofeng@xxxxxxxxxxxxxx>
---
src/conf/domain_conf.h | 23 +--------
src/libvirt_private.syms | 1 +
src/lxc/lxc_controller.c | 114 +-------------------------------------------
src/qemu/qemu_process.c | 121 +----------------------------------------------
src/util/virnuma.c | 114 ++++++++++++++++++++++++++++++++++++++++++++
src/util/virnuma.h | 24 ++++++++++
6 files changed, 143 insertions(+), 254 deletions(-)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 5828ae2..2a8dff3 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -47,6 +47,7 @@
# include "device_conf.h"
# include "virbitmap.h"
# include "virstoragefile.h"
+# include "virnuma.h"
/* forward declarations of all device types, required by
* virDomainDeviceDef
@@ -1589,14 +1590,6 @@ enum virDomainCpuPlacementMode {
VIR_DOMAIN_CPU_PLACEMENT_MODE_LAST
};
-enum virDomainNumatuneMemPlacementMode {
- VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_DEFAULT = 0,
- VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_STATIC,
- VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_AUTO,
-
- VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_LAST
-};
-
Given that you move this into virnuma.h, VIR_ENUM_DECL and
VIR_ENUM_IMPL also need to be moved. And I don't see changes
on things like this:
virDomainNumatuneMemPlacementModeTypeFromString
in domain_conf.c, I bet the domain conf parsing and formating
are now broken with this patch applied.
typedef struct _virDomainTimerCatchupDef virDomainTimerCatchupDef;
typedef virDomainTimerCatchupDef *virDomainTimerCatchupDefPtr;
struct _virDomainTimerCatchupDef {
@@ -1685,18 +1678,6 @@ virDomainVcpuPinDefPtr virDomainVcpuPinFindByVcpu(virDomainVcpuPinDefPtr *def,
int nvcpupin,
int vcpu);
-typedef struct _virDomainNumatuneDef virDomainNumatuneDef;
-typedef virDomainNumatuneDef *virDomainNumatuneDefPtr;
-struct _virDomainNumatuneDef {
- struct {
- virBitmapPtr nodemask;
- int mode;
- int placement_mode; /* enum virDomainNumatuneMemPlacementMode */
- } memory;
-
- /* Future NUMA tuning related stuff should go here. */
-};
-
typedef struct _virBlkioDeviceWeight virBlkioDeviceWeight;
typedef virBlkioDeviceWeight *virBlkioDeviceWeightPtr;
struct _virBlkioDeviceWeight {
@@ -1784,7 +1765,7 @@ struct _virDomainDef {
virDomainVcpuPinDefPtr emulatorpin;
} cputune;
- virDomainNumatuneDef numatune;
+ virNumaTuneParams numatune;
A bad new name, why not virNumatuneDef? the new name can be confused,
because we use params for other meaning in the project.
/* These 3 are based on virDomainLifeCycleAction enum flags */
int onReboot;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 6aee6fa..56c466a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1567,6 +1567,7 @@ virNodeSuspendGetTargetMask;
# util/virnuma.h
virGetNumadAdvice;
+virSetupNumaMemoryPolicy;
Generally we want to use virNuma As the prefix for the helpers. This
applies to virGetNumadAdvice too (I didn't realized it when reviewing
1/4).
# util/virobject.h
virClassForObject;
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index b6c1fe8..3db0a88 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -46,11 +46,6 @@
# include<cap-ng.h>
#endif
-#if WITH_NUMACTL
-# define NUMA_VERSION1_COMPATIBILITY 1
-# include<numa.h>
-#endif
-
#include "virerror.h"
#include "virlog.h"
#include "virutil.h"
@@ -409,113 +404,6 @@ cleanup:
return ret;
}
-#if WITH_NUMACTL
-static int virLXCControllerSetupNUMAPolicy(virLXCControllerPtr ctrl,
- virBitmapPtr nodemask)
-{
- nodemask_t mask;
- int mode = -1;
- int node = -1;
- int ret = -1;
- int i = 0;
- int maxnode = 0;
- bool warned = false;
- virDomainNumatuneDef numatune = ctrl->def->numatune;
- virBitmapPtr tmp_nodemask = NULL;
-
- if (numatune.memory.placement_mode ==
- VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_STATIC) {
- if (!numatune.memory.nodemask)
- return 0;
- VIR_DEBUG("Set NUMA memory policy with specified nodeset");
- tmp_nodemask = numatune.memory.nodemask;
- } else if (numatune.memory.placement_mode ==
- VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_AUTO) {
- VIR_DEBUG("Set NUMA memory policy with advisory nodeset from numad");
- tmp_nodemask = nodemask;
- } else {
- return 0;
- }
-
- VIR_DEBUG("Setting NUMA memory policy");
-
- if (numa_available()< 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- "%s", _("Host kernel is not aware of NUMA."));
- return -1;
- }
-
- maxnode = numa_max_node() + 1;
-
- /* Convert nodemask to NUMA bitmask. */
- nodemask_zero(&mask);
- i = -1;
- while ((i = virBitmapNextSetBit(tmp_nodemask, i))>= 0) {
- if (i> NUMA_NUM_NODES) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("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 = ctrl->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) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- "%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 {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Unable to set NUMA policy %s"),
- virDomainNumatuneMemModeTypeToString(mode));
- goto cleanup;
- }
-
- ret = 0;
-
-cleanup:
- return ret;
-}
-#else
-static int virLXCControllerSetupNUMAPolicy(virLXCControllerPtr ctrl,
- virBitmapPtr nodemask ATTRIBUTE_UNUSED)
-{
- if (ctrl->def->numatune.memory.nodemask) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("NUMA policy is not available on this platform"));
- return -1;
- }
-
- return 0;
-}
-#endif
-
/*
* To be run while still single threaded
@@ -621,7 +509,7 @@ static int virLXCControllerSetupResourceLimits(virLXCControllerPtr ctrl)
if (ret< 0)
goto cleanup;
- ret = virLXCControllerSetupNUMAPolicy(ctrl, nodemask);
+ ret = virSetupNumaMemoryPolicy(ctrl->def->numatune, nodemask);
if (ret< 0)
goto cleanup;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 20d41e3..2fa85aa 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -45,11 +45,6 @@
#include "qemu_bridge_filter.h"
#include "qemu_migration.h"
-#if WITH_NUMACTL
-# define NUMA_VERSION1_COMPATIBILITY 1
-# include<numa.h>
-#endif
-
#include "datatypes.h"
#include "virlog.h"
#include "virerror.h"
@@ -1869,120 +1864,6 @@ qemuProcessDetectVcpuPIDs(virQEMUDriverPtr driver,
}
-/*
- * Set NUMA memory policy for qemu process, to be run between
- * fork/exec of QEMU only.
- */
-#if WITH_NUMACTL
-static int
-qemuProcessInitNumaMemoryPolicy(virDomainObjPtr vm,
- virBitmapPtr nodemask)
-{
- nodemask_t mask;
- int mode = -1;
- int node = -1;
- int ret = -1;
- int i = 0;
- int maxnode = 0;
- bool warned = false;
- virDomainNumatuneDef numatune = vm->def->numatune;
- virBitmapPtr tmp_nodemask = NULL;
-
- if (numatune.memory.placement_mode ==
- VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_STATIC) {
- if (!numatune.memory.nodemask)
- return 0;
- VIR_DEBUG("Set NUMA memory policy with specified nodeset");
- tmp_nodemask = numatune.memory.nodemask;
- } else if (numatune.memory.placement_mode ==
- VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_AUTO) {
- VIR_DEBUG("Set NUMA memory policy with advisory nodeset from numad");
- tmp_nodemask = nodemask;
- } else {
- return 0;
- }
-
- if (numa_available()< 0) {
- virReportError(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);
- i = -1;
- while ((i = virBitmapNextSetBit(tmp_nodemask, i))>= 0) {
- if (i> NUMA_NUM_NODES) {
- virReportError(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 = 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) {
- virReportError(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.
- */
- virReportError(VIR_ERR_XML_ERROR,
- "%s", _("Invalid mode for memory NUMA tuning."));
- goto cleanup;
- }
-
- ret = 0;
-
-cleanup:
- return ret;
-}
-#else
-static int
-qemuProcessInitNumaMemoryPolicy(virDomainObjPtr vm,
- virBitmapPtr nodemask ATTRIBUTE_UNUSED)
-{
- if (vm->def->numatune.memory.nodemask) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("libvirt is compiled without NUMA tuning support"));
-
- return -1;
- }
-
- return 0;
-}
-#endif
-
-
/* Helper to prepare cpumap for affinity setting, convert
* NUMA nodeset into cpuset if @nodemask is not NULL, otherwise
* just return a new allocated bitmap.
@@ -2732,7 +2613,7 @@ static int qemuProcessHook(void *data)
qemuProcessInitCpuAffinity(h->driver, h->vm, h->nodemask)< 0)
goto cleanup;
- if (qemuProcessInitNumaMemoryPolicy(h->vm, h->nodemask)< 0)
+ if (virSetupNumaMemoryPolicy(h->vm->def->numatune, h->nodemask)< 0)
goto cleanup;
ret = 0;
diff --git a/src/util/virnuma.c b/src/util/virnuma.c
index 37931fe..75dcede 100644
--- a/src/util/virnuma.c
+++ b/src/util/virnuma.c
@@ -21,9 +21,15 @@
#include<config.h>
+#if WITH_NUMACTL
+# define NUMA_VERSION1_COMPATIBILITY 1
+# include<numa.h>
+#endif
+
#include "virnuma.h"
#include "vircommand.h"
#include "virerror.h"
+#include "virlog.h"
#define VIR_FROM_THIS VIR_FROM_NONE
@@ -58,3 +64,111 @@ virGetNumadAdvice(unsigned short vcpus ATTRIBUTE_UNUSED,
return NULL;
}
#endif
+
+#if WITH_NUMACTL
+int
+virSetupNumaMemoryPolicy(virNumaTuneParams numatune,
+ virBitmapPtr nodemask)
+{
+ nodemask_t mask;
+ int mode = -1;
+ int node = -1;
+ int ret = -1;
+ int i = 0;
+ int maxnode = 0;
+ bool warned = false;
+ virBitmapPtr tmp_nodemask = NULL;
+
+ if (numatune.memory.placement_mode ==
+ VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_STATIC) {
+ if (!numatune.memory.nodemask)
+ return 0;
+ VIR_DEBUG("Set NUMA memory policy with specified nodeset");
+ tmp_nodemask = numatune.memory.nodemask;
+ } else if (numatune.memory.placement_mode ==
+ VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_AUTO) {
+ VIR_DEBUG("Set NUMA memory policy with advisory nodeset from numad");
+ tmp_nodemask = nodemask;
+ } else {
+ return 0;
+ }
+
+ if (numa_available()< 0) {
+ virReportError(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);
+ i = -1;
+ while ((i = virBitmapNextSetBit(tmp_nodemask, i))>= 0) {
+ if (i> NUMA_NUM_NODES) {
+ virReportError(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 = 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) {
+ virReportError(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.
+ */
+ virReportError(VIR_ERR_XML_ERROR,
+ "%s", _("Invalid mode for memory NUMA tuning."));
+ goto cleanup;
+ }
+
+ ret = 0;
+
+cleanup:
+ return ret;
+}
+#else
+int
+virSetupNumaMemoryPolicy(virNumaTuneParams numatune,
+ virBitmapPtr nodemask ATTRIBUTE_UNUSED)
+{
+ if (numatune.memory.nodemask) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("libvirt is compiled without NUMA tuning support"));
+
+ return -1;
+ }
+
+ return 0;
+}
+#endif
diff --git a/src/util/virnuma.h b/src/util/virnuma.h
index b9046c2..8d9f14d 100644
--- a/src/util/virnuma.h
+++ b/src/util/virnuma.h
@@ -22,7 +22,31 @@
#ifndef __VIR_NUMA_H__
# define __VIR_NUMA_H__
+#include "virbitmap.h"
+
+enum virDomainNumatuneMemPlacementMode {
+ VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_DEFAULT = 0,
+ VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_STATIC,
+ VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_AUTO,
+
+ VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_LAST
+};
+
+typedef struct _virNumaTuneParams virNumaTuneParams;
+typedef virNumaTuneParams *virNumaTuneParamsPtr;
+struct _virNumaTuneParams {
+ struct {
+ virBitmapPtr nodemask;
+ int mode;
+ int placement_mode; /* enum virDomainNumatuneMemPlacementMode */
+ } memory;
+
+ /* Future NUMA tuning related stuff should go here. */
+};
+
Except the pointed out nits, others are simply code moving, looks good
to me. This needs a v2 too.
Osier
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list