virDomainNumatuneGetMode: Report if numatune was defined

So far, we are not reporting if numatune was even defined. The
value of zero is blindly returned (which maps onto
VIR_DOMAIN_NUMATUNE_MEM_STRICT). Unfortunately, we are making
decisions based on this value. Instead, we should not only return
the correct value, but report to the caller if the value is valid
at all.

For better viewing of this patch use '-w'.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This commit is contained in:
Michal Privoznik 2015-05-19 11:55:26 +02:00
parent 99a2d6af2b
commit bcd9a564b6
9 changed files with 93 additions and 59 deletions

View File

@ -351,20 +351,40 @@ virDomainNumaFree(virDomainNumaPtr numa)
VIR_FREE(numa);
}
virDomainNumatuneMemMode
virDomainNumatuneGetMode(virDomainNumaPtr numatune,
int cellid)
/**
* virDomainNumatuneGetMode:
* @numatune: pointer to numatune definition
* @cellid: cell selector
* @mode: where to store the result
*
* Get the defined mode for domain's memory. It's safe to pass
* NULL to @mode if the return value is the only info needed.
*
* Returns: 0 on success (with @mode updated)
* -1 if no mode was defined in XML
*/
int virDomainNumatuneGetMode(virDomainNumaPtr numatune,
int cellid,
virDomainNumatuneMemMode *mode)
{
int ret = -1;
virDomainNumatuneMemMode tmp_mode;
if (!numatune)
return 0;
return ret;
if (virDomainNumatuneNodeSpecified(numatune, cellid))
return numatune->mem_nodes[cellid].mode;
tmp_mode = numatune->mem_nodes[cellid].mode;
else if (numatune->memory.specified)
tmp_mode = numatune->memory.mode;
else
goto cleanup;
if (numatune->memory.specified)
return numatune->memory.mode;
return 0;
if (mode)
*mode = tmp_mode;
ret = 0;
cleanup:
return ret;
}
virBitmapPtr

View File

@ -72,8 +72,9 @@ int virDomainNumatuneFormatXML(virBufferPtr buf, virDomainNumaPtr numatune)
/*
* Getters
*/
virDomainNumatuneMemMode virDomainNumatuneGetMode(virDomainNumaPtr numatune,
int cellid);
int virDomainNumatuneGetMode(virDomainNumaPtr numatune,
int cellid,
virDomainNumatuneMemMode *mode);
virBitmapPtr virDomainNumatuneGetNodeset(virDomainNumaPtr numatune,
virBitmapPtr auto_nodeset,

View File

@ -69,6 +69,7 @@ static int virLXCCgroupSetupCpusetTune(virDomainDefPtr def,
{
int ret = -1;
char *mask = NULL;
virDomainNumatuneMemMode mode;
if (def->placement_mode != VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO &&
def->cpumask) {
@ -81,8 +82,8 @@ static int virLXCCgroupSetupCpusetTune(virDomainDefPtr def,
VIR_FREE(mask);
}
if (virDomainNumatuneGetMode(def->numa, -1) !=
VIR_DOMAIN_NUMATUNE_MEM_STRICT) {
if (virDomainNumatuneGetMode(def->numa, -1, &mode) < 0 ||
mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT) {
ret = 0;
goto cleanup;
}

View File

@ -741,25 +741,25 @@ static int virLXCControllerSetupResourceLimits(virLXCControllerPtr ctrl)
virBitmapPtr nodeset = NULL;
virDomainNumatuneMemMode mode;
mode = virDomainNumatuneGetMode(ctrl->def->numa, -1);
if (virDomainNumatuneGetMode(ctrl->def->numa, -1, &mode) == 0) {
if (mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT &&
virCgroupControllerAvailable(VIR_CGROUP_CONTROLLER_CPUSET)) {
/* Use virNuma* API iff necessary. Once set and child is exec()-ed,
* there's no way for us to change it. Rely on cgroups (if available
* and enabled in the config) rather than virNuma*. */
VIR_DEBUG("Relying on CGroups for memory binding");
} else {
if (mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT &&
virCgroupControllerAvailable(VIR_CGROUP_CONTROLLER_CPUSET)) {
/* Use virNuma* API iff necessary. Once set and child is exec()-ed,
* there's no way for us to change it. Rely on cgroups (if available
* and enabled in the config) rather than virNuma*. */
VIR_DEBUG("Relying on CGroups for memory binding");
} else {
VIR_DEBUG("Setting up process resource limits");
VIR_DEBUG("Setting up process resource limits");
if (virLXCControllerGetNumadAdvice(ctrl, &auto_nodeset) < 0)
goto cleanup;
if (virLXCControllerGetNumadAdvice(ctrl, &auto_nodeset) < 0)
goto cleanup;
nodeset = virDomainNumatuneGetNodeset(ctrl->def->numa, auto_nodeset, -1);
nodeset = virDomainNumatuneGetNodeset(ctrl->def->numa, auto_nodeset, -1);
if (virNumaSetupMemoryPolicy(mode, nodeset) < 0)
goto cleanup;
if (virNumaSetupMemoryPolicy(mode, nodeset) < 0)
goto cleanup;
}
}
if (virLXCControllerSetupCpuAffinity(ctrl) < 0)

View File

@ -1845,6 +1845,7 @@ prlsdkCheckUnsupportedParams(PRL_HANDLE sdkdom, virDomainDefPtr def)
size_t i;
PRL_VM_TYPE vmType;
PRL_RESULT pret;
virDomainNumatuneMemMode memMode;
if (def->title) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@ -1924,8 +1925,8 @@ prlsdkCheckUnsupportedParams(PRL_HANDLE sdkdom, virDomainDefPtr def)
* virDomainDefPtr always contain non zero NUMA configuration
* So, just make sure this configuration does't differ from auto generated.
*/
if ((virDomainNumatuneGetMode(def->numa, -1) !=
VIR_DOMAIN_NUMATUNE_MEM_STRICT) ||
if ((virDomainNumatuneGetMode(def->numa, -1, &memMode) == 0 &&
memMode == VIR_DOMAIN_NUMATUNE_MEM_STRICT) ||
virDomainNumatuneHasPerNodeBinding(def->numa)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("numa parameters are not supported "

View File

@ -613,14 +613,15 @@ qemuSetupCpusetMems(virDomainObjPtr vm)
{
virCgroupPtr cgroup_temp = NULL;
qemuDomainObjPrivatePtr priv = vm->privateData;
virDomainNumatuneMemMode mode;
char *mem_mask = NULL;
int ret = -1;
if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET))
return 0;
if (virDomainNumatuneGetMode(vm->def->numa, -1) !=
VIR_DOMAIN_NUMATUNE_MEM_STRICT)
if (virDomainNumatuneGetMode(vm->def->numa, -1, &mode) < 0 ||
mode != VIR_DOMAIN_NUMATUNE_MEM_STRICT)
return 0;
if (virDomainNumatuneMaybeFormatNodeset(vm->def->numa,
@ -979,6 +980,7 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm)
unsigned long long period = vm->def->cputune.period;
long long quota = vm->def->cputune.quota;
char *mem_mask = NULL;
virDomainNumatuneMemMode mem_mode;
if ((period || quota) &&
!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) {
@ -1009,8 +1011,8 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm)
return 0;
}
if (virDomainNumatuneGetMode(vm->def->numa, -1) ==
VIR_DOMAIN_NUMATUNE_MEM_STRICT &&
if (virDomainNumatuneGetMode(vm->def->numa, -1, &mem_mode) == 0 &&
mem_mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT &&
virDomainNumatuneMaybeFormatNodeset(vm->def->numa,
priv->autoNodeset,
&mem_mask, -1) < 0)
@ -1153,6 +1155,7 @@ qemuSetupCgroupForIOThreads(virDomainObjPtr vm)
unsigned long long period = vm->def->cputune.period;
long long quota = vm->def->cputune.quota;
char *mem_mask = NULL;
virDomainNumatuneMemMode mem_mode;
if ((period || quota) &&
!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) {
@ -1176,8 +1179,8 @@ qemuSetupCgroupForIOThreads(virDomainObjPtr vm)
if (priv->cgroup == NULL)
return 0;
if (virDomainNumatuneGetMode(vm->def->numa, -1) ==
VIR_DOMAIN_NUMATUNE_MEM_STRICT &&
if (virDomainNumatuneGetMode(vm->def->numa, -1, &mem_mode) == 0 &&
mem_mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT &&
virDomainNumatuneMaybeFormatNodeset(vm->def->numa,
priv->autoNodeset,
&mem_mask, -1) < 0)

View File

@ -4707,7 +4707,9 @@ qemuBuildMemoryBackendStr(unsigned long long size,
return -1;
memAccess = virDomainNumaGetNodeMemoryAccessMode(def->numa, guestNode);
mode = virDomainNumatuneGetMode(def->numa, guestNode);
if (virDomainNumatuneGetMode(def->numa, guestNode, &mode) < 0 &&
virDomainNumatuneGetMode(def->numa, -1, &mode) < 0)
mode = VIR_DOMAIN_NUMATUNE_MEM_STRICT;
if (pagesize == 0 || pagesize != system_page_size) {
/* Find the huge page size we want to use */

View File

@ -4737,6 +4737,7 @@ qemuDomainHotplugVcpus(virQEMUDriverPtr driver,
int ncpupids;
virCgroupPtr cgroup_vcpu = NULL;
char *mem_mask = NULL;
virDomainNumatuneMemMode mem_mode;
qemuDomainObjEnterMonitor(driver, vm);
@ -4804,8 +4805,8 @@ qemuDomainHotplugVcpus(virQEMUDriverPtr driver,
goto cleanup;
}
if (virDomainNumatuneGetMode(vm->def->numa, -1) ==
VIR_DOMAIN_NUMATUNE_MEM_STRICT &&
if (virDomainNumatuneGetMode(vm->def->numa, -1, &mem_mode) == 0 &&
mem_mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT &&
virDomainNumatuneMaybeFormatNodeset(vm->def->numa,
priv->autoNodeset,
&mem_mask, -1) < 0)
@ -6113,6 +6114,7 @@ qemuDomainHotplugAddIOThread(virQEMUDriverPtr driver,
qemuMonitorIOThreadInfoPtr *new_iothreads = NULL;
virCgroupPtr cgroup_iothread = NULL;
char *mem_mask = NULL;
virDomainNumatuneMemMode mode;
virDomainIOThreadIDDefPtr iothrid;
virBitmapPtr cpumask;
@ -6154,8 +6156,8 @@ qemuDomainHotplugAddIOThread(virQEMUDriverPtr driver,
}
vm->def->iothreads = exp_niothreads;
if (virDomainNumatuneGetMode(vm->def->numa, -1) ==
VIR_DOMAIN_NUMATUNE_MEM_STRICT &&
if (virDomainNumatuneGetMode(vm->def->numa, -1, &mode) == 0 &&
mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT &&
virDomainNumatuneMaybeFormatNodeset(vm->def->numa,
priv->autoNodeset,
&mem_mask, -1) < 0)
@ -10331,11 +10333,12 @@ qemuDomainSetNumaParamsLive(virDomainObjPtr vm,
virCgroupPtr cgroup_temp = NULL;
qemuDomainObjPrivatePtr priv = vm->privateData;
char *nodeset_str = NULL;
virDomainNumatuneMemMode mode;
size_t i = 0;
int ret = -1;
if (virDomainNumatuneGetMode(vm->def->numa, -1) !=
VIR_DOMAIN_NUMATUNE_MEM_STRICT) {
if (virDomainNumatuneGetMode(vm->def->numa, -1, &mode) < 0 ||
mode != VIR_DOMAIN_NUMATUNE_MEM_STRICT) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("change of nodeset for running domain "
"requires strict numa mode"));
@ -10392,6 +10395,7 @@ qemuDomainSetNumaParameters(virDomainPtr dom,
virCapsPtr caps = NULL;
qemuDomainObjPrivatePtr priv;
virBitmapPtr nodeset = NULL;
virDomainNumatuneMemMode config_mode;
int mode = -1;
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
@ -10467,7 +10471,8 @@ qemuDomainSetNumaParameters(virDomainPtr dom,
if (flags & VIR_DOMAIN_AFFECT_LIVE) {
if (mode != -1 &&
virDomainNumatuneGetMode(vm->def->numa, -1) != mode) {
virDomainNumatuneGetMode(vm->def->numa, -1, &config_mode) == 0 &&
config_mode != mode) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("can't change numatune mode for running domain"));
goto endjob;
@ -10568,7 +10573,8 @@ qemuDomainGetNumaParameters(virDomainPtr dom,
VIR_TYPED_PARAM_INT, 0) < 0)
goto cleanup;
param->value.i = virDomainNumatuneGetMode(def->numa, -1);
virDomainNumatuneGetMode(def->numa, -1,
(virDomainNumatuneMemMode *) &param->value.i);
break;
case 1: /* fill numa nodeset here */

View File

@ -3135,21 +3135,21 @@ static int qemuProcessHook(void *data)
if (virSecurityManagerClearSocketLabel(h->driver->securityManager, h->vm->def) < 0)
goto cleanup;
mode = virDomainNumatuneGetMode(h->vm->def->numa, -1);
if (virDomainNumatuneGetMode(h->vm->def->numa, -1, &mode) == 0) {
if (mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT &&
h->cfg->cgroupControllers & (1 << VIR_CGROUP_CONTROLLER_CPUSET) &&
virCgroupControllerAvailable(VIR_CGROUP_CONTROLLER_CPUSET)) {
/* Use virNuma* API iff necessary. Once set and child is exec()-ed,
* there's no way for us to change it. Rely on cgroups (if available
* and enabled in the config) rather than virNuma*. */
VIR_DEBUG("Relying on CGroups for memory binding");
} else {
nodeset = virDomainNumatuneGetNodeset(h->vm->def->numa,
priv->autoNodeset, -1);
if (mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT &&
h->cfg->cgroupControllers & (1 << VIR_CGROUP_CONTROLLER_CPUSET) &&
virCgroupControllerAvailable(VIR_CGROUP_CONTROLLER_CPUSET)) {
/* Use virNuma* API iff necessary. Once set and child is exec()-ed,
* there's no way for us to change it. Rely on cgroups (if available
* and enabled in the config) rather than virNuma*. */
VIR_DEBUG("Relying on CGroups for memory binding");
} else {
nodeset = virDomainNumatuneGetNodeset(h->vm->def->numa,
priv->autoNodeset, -1);
if (virNumaSetupMemoryPolicy(mode, nodeset) < 0)
goto cleanup;
if (virNumaSetupMemoryPolicy(mode, nodeset) < 0)
goto cleanup;
}
}
ret = 0;