3:08 a.m.
In a few API implementations, we use virDomainLiveConfigHelperMethod
to check the VIR_DOMAIN_AFFECT_* flags (and change the flags variable
to either AFFECT_LIVE or AFFECT_CONFIG if AFFECT_CURRENT was requested)
and optionally give us a pointer to the persistent definition.
This function can also create the persistent definition in vm->newDef
for a live domain if it does not exist yet. This cannot be the case
in LXC driver because we create it unconditionally on domain startup.
By switching to virDomainObjGetDefs we do not need to pass virCaps
and virDomainXMLOption (because the defintion does not need to be copied).
Also, instead of altering the flags, it fills the pointers to live and persistent
definitions depending on which one was requested to be modified,
making the code easier to read for static analyzers.
Ján Tomko (9):
lxc: rename vmdef to persistentDef
Use virDomainObjGetDefs in lxcDomainSetMemoryFlags
Use virDomainObjGetDefs in lxcDomainSetSchedulerParametersFlags
Use virDomainObjGetDefs in lxcDomainGetSchedulerParametersFlags
Use virDomainObjGetDefs in lxcDomainGetMemoryParameters
Use virDomainObjGetDefs in lxcDomainSetBlkioParameters
Use virDomainObjGetDefs in lxcDomainGetBlkioParameters
Export virDomainGetBlkioParametersAssignFromDef
lxc: simplify lxcDomainGetBlkioParameters
src/conf/domain_conf.c | 51 +++++
src/conf/domain_conf.h | 6 +
src/libvirt_private.syms | 1 +
src/lxc/lxc_driver.c | 511 +++++++++--------------------------------------
src/qemu/qemu_driver.c | 59 +-----
5 files changed, 154 insertions(+), 474 deletions(-)
--
2.7.3
3:08 a.m.
New subject: [libvirt] [PATCH 1/9] lxc: rename vmdef to persistentDef
A few functions using virDomainLiveConfigHelperMethod use the generic
name 'vmdef' to point to the persistent definition.
Use persistentDef and/or persistentDefCopy to make its purpose obvious.
---
src/lxc/lxc_driver.c | 43 ++++++++++++++++++++++---------------------
1 file changed, 22 insertions(+), 21 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index f0948ea..e854f35 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -806,7 +806,7 @@ lxcDomainSetMemoryParameters(virDomainPtr dom,
unsigned int flags)
{
virCapsPtr caps = NULL;
- virDomainDefPtr vmdef = NULL;
+ virDomainDefPtr persistentDef = NULL;
virDomainObjPtr vm = NULL;
virLXCDomainObjPrivatePtr priv = NULL;
virLXCDriverConfigPtr cfg = NULL;
@@ -847,7 +847,7 @@ lxcDomainSetMemoryParameters(virDomainPtr dom,
goto cleanup;
if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt,
- vm, &flags, &vmdef) < 0)
+ vm, &flags, &persistentDef) < 0)
goto endjob;
if (flags & VIR_DOMAIN_AFFECT_LIVE &&
@@ -903,7 +903,7 @@ lxcDomainSetMemoryParameters(virDomainPtr dom,
} \
\
if (flags & VIR_DOMAIN_AFFECT_CONFIG) \
- vmdef->mem.VALUE = VALUE; \
+ persistentDef->mem.VALUE = VALUE; \
}
/* Soft limit doesn't clash with the others */
@@ -924,7 +924,7 @@ lxcDomainSetMemoryParameters(virDomainPtr dom,
#undef LXC_SET_MEM_PARAMETER
if (flags & VIR_DOMAIN_AFFECT_CONFIG &&
- virDomainSaveConfig(cfg->configDir, driver->caps, vmdef) < 0)
+ virDomainSaveConfig(cfg->configDir, driver->caps, persistentDef) < 0)
goto endjob;
ret = 0;
@@ -946,7 +946,7 @@ lxcDomainGetMemoryParameters(virDomainPtr dom,
unsigned int flags)
{
virCapsPtr caps = NULL;
- virDomainDefPtr vmdef = NULL;
+ virDomainDefPtr persistentDef = NULL;
virDomainObjPtr vm = NULL;
virLXCDomainObjPrivatePtr priv = NULL;
virLXCDriverPtr driver = dom->conn->privateData;
@@ -969,7 +969,7 @@ lxcDomainGetMemoryParameters(virDomainPtr dom,
if (virDomainGetMemoryParametersEnsureACL(dom->conn, vm->def) < 0 ||
!(caps = virLXCDriverGetCapabilities(driver, false)) ||
virDomainLiveConfigHelperMethod(caps, driver->xmlopt,
- vm, &flags, &vmdef) < 0)
+ vm, &flags, &persistentDef) < 0)
goto cleanup;
if (flags & VIR_DOMAIN_AFFECT_LIVE &&
@@ -993,7 +993,7 @@ lxcDomainGetMemoryParameters(virDomainPtr dom,
switch (i) {
case 0: /* fill memory hard limit here */
if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
- val = vmdef->mem.hard_limit;
+ val = persistentDef->mem.hard_limit;
} else if (virCgroupGetMemoryHardLimit(priv->cgroup, &val) < 0) {
goto cleanup;
}
@@ -1003,7 +1003,7 @@ lxcDomainGetMemoryParameters(virDomainPtr dom,
break;
case 1: /* fill memory soft limit here */
if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
- val = vmdef->mem.soft_limit;
+ val = persistentDef->mem.soft_limit;
} else if (virCgroupGetMemorySoftLimit(priv->cgroup, &val) < 0) {
goto cleanup;
}
@@ -1013,7 +1013,7 @@ lxcDomainGetMemoryParameters(virDomainPtr dom,
break;
case 2: /* fill swap hard limit here */
if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
- val = vmdef->mem.swap_hard_limit;
+ val = persistentDef->mem.swap_hard_limit;
} else if (virCgroupGetMemSwapHardLimit(priv->cgroup, &val) < 0) {
goto cleanup;
}
@@ -1951,7 +1951,8 @@ lxcDomainSetSchedulerParametersFlags(virDomainPtr dom,
virCapsPtr caps = NULL;
size_t i;
virDomainObjPtr vm = NULL;
- virDomainDefPtr vmdef = NULL;
+ virDomainDefPtr persistentDefCopy = NULL;
+ virDomainDefPtr persistentDef = NULL;
int ret = -1;
int rc;
virLXCDomainObjPrivatePtr priv;
@@ -1984,13 +1985,13 @@ lxcDomainSetSchedulerParametersFlags(virDomainPtr dom,
goto cleanup;
if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt,
- vm, &flags, &vmdef) < 0)
+ vm, &flags, &persistentDef) < 0)
goto endjob;
if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
/* Make a copy for updated domain. */
- vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt);
- if (!vmdef)
+ persistentDefCopy = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt);
+ if (!persistentDefCopy)
goto endjob;
}
@@ -2019,8 +2020,8 @@ lxcDomainSetSchedulerParametersFlags(virDomainPtr dom,
}
if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
- vmdef->cputune.shares = params[i].value.ul;
- vmdef->cputune.sharesSpecified = true;
+ persistentDefCopy->cputune.shares = params[i].value.ul;
+ persistentDefCopy->cputune.sharesSpecified = true;
}
} else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_VCPU_PERIOD)) {
if (flags & VIR_DOMAIN_AFFECT_LIVE) {
@@ -2033,7 +2034,7 @@ lxcDomainSetSchedulerParametersFlags(virDomainPtr dom,
}
if (flags & VIR_DOMAIN_AFFECT_CONFIG)
- vmdef->cputune.period = params[i].value.ul;
+ persistentDefCopy->cputune.period = params[i].value.ul;
} else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_VCPU_QUOTA)) {
if (flags & VIR_DOMAIN_AFFECT_LIVE) {
rc = lxcSetVcpuBWLive(priv->cgroup, 0, params[i].value.l);
@@ -2045,7 +2046,7 @@ lxcDomainSetSchedulerParametersFlags(virDomainPtr dom,
}
if (flags & VIR_DOMAIN_AFFECT_CONFIG)
- vmdef->cputune.quota = params[i].value.l;
+ persistentDefCopy->cputune.quota = params[i].value.l;
}
}
@@ -2054,12 +2055,12 @@ lxcDomainSetSchedulerParametersFlags(virDomainPtr dom,
if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
- rc = virDomainSaveConfig(cfg->configDir, driver->caps, vmdef);
+ rc = virDomainSaveConfig(cfg->configDir, driver->caps, persistentDefCopy);
if (rc < 0)
goto endjob;
- virDomainObjAssignDef(vm, vmdef, false, NULL);
- vmdef = NULL;
+ virDomainObjAssignDef(vm, persistentDefCopy, false, NULL);
+ persistentDefCopy = NULL;
}
ret = 0;
@@ -2068,7 +2069,7 @@ lxcDomainSetSchedulerParametersFlags(virDomainPtr dom,
virLXCDomainObjEndJob(driver, vm);
cleanup:
- virDomainDefFree(vmdef);
+ virDomainDefFree(persistentDefCopy);
virDomainObjEndAPI(&vm);
virObjectUnref(caps);
virObjectUnref(cfg);
--
2.7.3
3:08 a.m.
New subject: [libvirt] [PATCH 2/9] Use virDomainObjGetDefs in lxcDomainSetMemoryFlags
On LXC domain startup we have already called virDomainObjSetDefTransient
to fill vm->newDef.
There is no need to call virDomainLiveConfigHelperMethod which has the
ability to fill newDef if it's NULL.
---
src/lxc/lxc_driver.c | 26 ++++++++++----------------
1 file changed, 10 insertions(+), 16 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index e854f35..72a144b 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -694,8 +694,7 @@ static int lxcDomainSetMemoryFlags(virDomainPtr dom, unsigned long
newmem,
unsigned int flags)
{
virDomainObjPtr vm;
- virDomainDefPtr persistentDef = NULL;
- virCapsPtr caps = NULL;
+ virDomainDefPtr def = NULL, persistentDef = NULL;
int ret = -1;
virLXCDomainObjPrivatePtr priv;
virLXCDriverPtr driver = dom->conn->privateData;
@@ -718,22 +717,18 @@ static int lxcDomainSetMemoryFlags(virDomainPtr dom, unsigned long
newmem,
if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0)
goto cleanup;
- if (!(caps = virLXCDriverGetCapabilities(driver, false)))
- goto endjob;
-
- if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, vm, &flags,
- &persistentDef) < 0)
+ if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0)
goto endjob;
if (flags & VIR_DOMAIN_MEM_MAXIMUM) {
- if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+ if (def) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("Cannot resize the max memory "
"on an active domain"));
goto endjob;
}
- if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+ if (persistentDef) {
virDomainDefSetMemoryTotal(persistentDef, newmem);
if (persistentDef->mem.cur_balloon > newmem)
persistentDef->mem.cur_balloon = newmem;
@@ -744,9 +739,9 @@ static int lxcDomainSetMemoryFlags(virDomainPtr dom, unsigned long
newmem,
} else {
unsigned long oldmax = 0;
- if (flags & VIR_DOMAIN_AFFECT_LIVE)
- oldmax = virDomainDefGetMemoryActual(vm->def);
- if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+ if (def)
+ oldmax = virDomainDefGetMemoryActual(def);
+ if (persistentDef) {
if (!oldmax || oldmax > virDomainDefGetMemoryActual(persistentDef))
oldmax = virDomainDefGetMemoryActual(persistentDef);
}
@@ -757,19 +752,19 @@ static int lxcDomainSetMemoryFlags(virDomainPtr dom, unsigned long
newmem,
goto endjob;
}
- if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+ if (def) {
if (virCgroupSetMemory(priv->cgroup, newmem) < 0) {
virReportError(VIR_ERR_OPERATION_FAILED,
"%s", _("Failed to set memory for
domain"));
goto endjob;
}
- vm->def->mem.cur_balloon = newmem;
+ def->mem.cur_balloon = newmem;
if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm,
driver->caps) < 0)
goto endjob;
}
- if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+ if (persistentDef) {
persistentDef->mem.cur_balloon = newmem;
if (virDomainSaveConfig(cfg->configDir, driver->caps,
persistentDef) < 0)
@@ -784,7 +779,6 @@ static int lxcDomainSetMemoryFlags(virDomainPtr dom, unsigned long
newmem,
cleanup:
virDomainObjEndAPI(&vm);
- virObjectUnref(caps);
virObjectUnref(cfg);
return ret;
}
--
2.7.3
7:44 a.m.
New subject: [libvirt] [PATCH 2/9] Use virDomainObjGetDefs in lxcDomainSetMemoryFlags
On 06/06/2016 04:08 AM, Ján Tomko wrote:
On LXC domain startup we have already called
virDomainObjSetDefTransient
to fill vm->newDef.
There is no need to call virDomainLiveConfigHelperMethod which has the
ability to fill newDef if it's NULL.
---
src/lxc/lxc_driver.c | 26 ++++++++++----------------
1 file changed, 10 insertions(+), 16 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index e854f35..72a144b 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -694,8 +694,7 @@ static int lxcDomainSetMemoryFlags(virDomainPtr dom, unsigned long
newmem,
unsigned int flags)
{
virDomainObjPtr vm;
- virDomainDefPtr persistentDef = NULL;
- virCapsPtr caps = NULL;
+ virDomainDefPtr def = NULL, persistentDef = NULL;
NIT: Other code has these on separate lines
John
int ret = -1;
virLXCDomainObjPrivatePtr priv;
virLXCDriverPtr driver = dom->conn->privateData;
@@ -718,22 +717,18 @@ static int lxcDomainSetMemoryFlags(virDomainPtr dom, unsigned long
newmem,
if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0)
goto cleanup;
- if (!(caps = virLXCDriverGetCapabilities(driver, false)))
- goto endjob;
-
- if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, vm, &flags,
- &persistentDef) < 0)
+ if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0)
goto endjob;
if (flags & VIR_DOMAIN_MEM_MAXIMUM) {
- if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+ if (def) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("Cannot resize the max memory "
"on an active domain"));
goto endjob;
}
- if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+ if (persistentDef) {
virDomainDefSetMemoryTotal(persistentDef, newmem);
if (persistentDef->mem.cur_balloon > newmem)
persistentDef->mem.cur_balloon = newmem;
@@ -744,9 +739,9 @@ static int lxcDomainSetMemoryFlags(virDomainPtr dom, unsigned long
newmem,
} else {
unsigned long oldmax = 0;
- if (flags & VIR_DOMAIN_AFFECT_LIVE)
- oldmax = virDomainDefGetMemoryActual(vm->def);
- if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+ if (def)
+ oldmax = virDomainDefGetMemoryActual(def);
+ if (persistentDef) {
if (!oldmax || oldmax > virDomainDefGetMemoryActual(persistentDef))
oldmax = virDomainDefGetMemoryActual(persistentDef);
}
@@ -757,19 +752,19 @@ static int lxcDomainSetMemoryFlags(virDomainPtr dom, unsigned long
newmem,
goto endjob;
}
- if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+ if (def) {
if (virCgroupSetMemory(priv->cgroup, newmem) < 0) {
virReportError(VIR_ERR_OPERATION_FAILED,
"%s", _("Failed to set memory for
domain"));
goto endjob;
}
- vm->def->mem.cur_balloon = newmem;
+ def->mem.cur_balloon = newmem;
if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm,
driver->caps) < 0)
goto endjob;
}
- if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+ if (persistentDef) {
persistentDef->mem.cur_balloon = newmem;
if (virDomainSaveConfig(cfg->configDir, driver->caps,
persistentDef) < 0)
@@ -784,7 +779,6 @@ static int lxcDomainSetMemoryFlags(virDomainPtr dom, unsigned long
newmem,
cleanup:
virDomainObjEndAPI(&vm);
- virObjectUnref(caps);
virObjectUnref(cfg);
return ret;
}
3:08 a.m.
New subject: [libvirt] [PATCH 3/9] Use virDomainObjGetDefs in lxcDomainSetSchedulerParametersFlags
On LXC domain startup we have already called virDomainObjSetDefTransient
to fill vm->newDef.
There is no need to call virDomainLiveConfigHelperMethod which has the
ability to fill newDef if it's NULL.
---
src/lxc/lxc_driver.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 72a144b..363b0b0 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1945,6 +1945,7 @@ lxcDomainSetSchedulerParametersFlags(virDomainPtr dom,
virCapsPtr caps = NULL;
size_t i;
virDomainObjPtr vm = NULL;
+ virDomainDefPtr def = NULL;
virDomainDefPtr persistentDefCopy = NULL;
virDomainDefPtr persistentDef = NULL;
int ret = -1;
@@ -1978,18 +1979,17 @@ lxcDomainSetSchedulerParametersFlags(virDomainPtr dom,
if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0)
goto cleanup;
- if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt,
- vm, &flags, &persistentDef) < 0)
+ if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0)
goto endjob;
- if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+ if (persistentDef) {
/* Make a copy for updated domain. */
persistentDefCopy = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt);
if (!persistentDefCopy)
goto endjob;
}
- if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+ if (def) {
if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) {
virReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("cgroup CPU controller is not
mounted"));
@@ -2001,7 +2001,7 @@ lxcDomainSetSchedulerParametersFlags(virDomainPtr dom,
virTypedParameterPtr param = ¶ms[i];
if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_CPU_SHARES)) {
- if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+ if (def) {
unsigned long long val;
if (virCgroupSetCpuShares(priv->cgroup, params[i].value.ul) < 0)
goto endjob;
@@ -2009,37 +2009,37 @@ lxcDomainSetSchedulerParametersFlags(virDomainPtr dom,
if (virCgroupGetCpuShares(priv->cgroup, &val) < 0)
goto endjob;
- vm->def->cputune.shares = val;
- vm->def->cputune.sharesSpecified = true;
+ def->cputune.shares = val;
+ def->cputune.sharesSpecified = true;
}
- if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+ if (persistentDef) {
persistentDefCopy->cputune.shares = params[i].value.ul;
persistentDefCopy->cputune.sharesSpecified = true;
}
} else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_VCPU_PERIOD)) {
- if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+ if (def) {
rc = lxcSetVcpuBWLive(priv->cgroup, params[i].value.ul, 0);
if (rc != 0)
goto endjob;
if (params[i].value.ul)
- vm->def->cputune.period = params[i].value.ul;
+ def->cputune.period = params[i].value.ul;
}
- if (flags & VIR_DOMAIN_AFFECT_CONFIG)
+ if (persistentDef)
persistentDefCopy->cputune.period = params[i].value.ul;
} else if (STREQ(param->field, VIR_DOMAIN_SCHEDULER_VCPU_QUOTA)) {
- if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+ if (def) {
rc = lxcSetVcpuBWLive(priv->cgroup, 0, params[i].value.l);
if (rc != 0)
goto endjob;
if (params[i].value.l)
- vm->def->cputune.quota = params[i].value.l;
+ def->cputune.quota = params[i].value.l;
}
- if (flags & VIR_DOMAIN_AFFECT_CONFIG)
+ if (persistentDef)
persistentDefCopy->cputune.quota = params[i].value.l;
}
}
@@ -2048,7 +2048,7 @@ lxcDomainSetSchedulerParametersFlags(virDomainPtr dom,
goto endjob;
- if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+ if (persistentDef) {
rc = virDomainSaveConfig(cfg->configDir, driver->caps, persistentDefCopy);
if (rc < 0)
goto endjob;
--
2.7.3
3:08 a.m.
New subject: [libvirt] [PATCH 4/9] Use virDomainObjGetDefs in lxcDomainGetSchedulerParametersFlags
On LXC domain startup we have already called virDomainObjSetDefTransient
to fill vm->newDef.
There is no need to call virDomainLiveConfigHelperMethod which has the
ability to fill newDef if it's NULL.
---
src/lxc/lxc_driver.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 363b0b0..c39b4b4 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -2084,10 +2084,8 @@ lxcDomainGetSchedulerParametersFlags(virDomainPtr dom,
int *nparams,
unsigned int flags)
{
- virLXCDriverPtr driver = dom->conn->privateData;
- virCapsPtr caps = NULL;
virDomainObjPtr vm = NULL;
- virDomainDefPtr persistentDef;
+ virDomainDefPtr def, persistentDef;
unsigned long long shares = 0;
unsigned long long period = 0;
long long quota = 0;
@@ -2115,14 +2113,10 @@ lxcDomainGetSchedulerParametersFlags(virDomainPtr dom,
if (*nparams > 1)
cpu_bw_status = virCgroupSupportsCpuBW(priv->cgroup);
- if (!(caps = virLXCDriverGetCapabilities(driver, false)))
- goto cleanup;
-
- if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt,
- vm, &flags, &persistentDef) < 0)
+ if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0)
goto cleanup;
- if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+ if (persistentDef) {
shares = persistentDef->cputune.shares;
if (*nparams > 1) {
period = persistentDef->cputune.period;
@@ -2176,7 +2170,6 @@ lxcDomainGetSchedulerParametersFlags(virDomainPtr dom,
cleanup:
virDomainObjEndAPI(&vm);
- virObjectUnref(caps);
return ret;
}
--
2.7.3
7:44 a.m.
New subject: [libvirt] [PATCH 4/9] Use virDomainObjGetDefs in lxcDomainGetSchedulerParametersFlags
On 06/06/2016 04:08 AM, Ján Tomko wrote:
On LXC domain startup we have already called
virDomainObjSetDefTransient
to fill vm->newDef.
There is no need to call virDomainLiveConfigHelperMethod which has the
ability to fill newDef if it's NULL.
---
src/lxc/lxc_driver.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 363b0b0..c39b4b4 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -2084,10 +2084,8 @@ lxcDomainGetSchedulerParametersFlags(virDomainPtr dom,
int *nparams,
unsigned int flags)
{
- virLXCDriverPtr driver = dom->conn->privateData;
- virCapsPtr caps = NULL;
virDomainObjPtr vm = NULL;
- virDomainDefPtr persistentDef;
+ virDomainDefPtr def, persistentDef;
NIT: Other code has these on separate lines and set to NULL
John
unsigned long long shares = 0;
unsigned long long period = 0;
long long quota = 0;
@@ -2115,14 +2113,10 @@ lxcDomainGetSchedulerParametersFlags(virDomainPtr dom,
if (*nparams > 1)
cpu_bw_status = virCgroupSupportsCpuBW(priv->cgroup);
- if (!(caps = virLXCDriverGetCapabilities(driver, false)))
- goto cleanup;
-
- if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt,
- vm, &flags, &persistentDef) < 0)
+ if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0)
goto cleanup;
- if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+ if (persistentDef) {
shares = persistentDef->cputune.shares;
if (*nparams > 1) {
period = persistentDef->cputune.period;
@@ -2176,7 +2170,6 @@ lxcDomainGetSchedulerParametersFlags(virDomainPtr dom,
cleanup:
virDomainObjEndAPI(&vm);
- virObjectUnref(caps);
return ret;
}
3:08 a.m.
New subject: [libvirt] [PATCH 5/9] Use virDomainObjGetDefs in lxcDomainGetMemoryParameters
Instead of virDomainLiveConfigHelperMethod.
---
src/lxc/lxc_driver.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index c39b4b4..00b4df2 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -941,9 +941,9 @@ lxcDomainGetMemoryParameters(virDomainPtr dom,
{
virCapsPtr caps = NULL;
virDomainDefPtr persistentDef = NULL;
+ virDomainDefPtr def = NULL;
virDomainObjPtr vm = NULL;
virLXCDomainObjPrivatePtr priv = NULL;
- virLXCDriverPtr driver = dom->conn->privateData;
unsigned long long val;
int ret = -1;
size_t i;
@@ -960,13 +960,13 @@ lxcDomainGetMemoryParameters(virDomainPtr dom,
priv = vm->privateData;
- if (virDomainGetMemoryParametersEnsureACL(dom->conn, vm->def) < 0 ||
- !(caps = virLXCDriverGetCapabilities(driver, false)) ||
- virDomainLiveConfigHelperMethod(caps, driver->xmlopt,
- vm, &flags, &persistentDef) < 0)
+ if (virDomainGetMemoryParametersEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
- if (flags & VIR_DOMAIN_AFFECT_LIVE &&
+ if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0)
+ goto cleanup;
+
+ if (def &&
!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_MEMORY)) {
virReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("cgroup memory controller is not
mounted"));
@@ -986,7 +986,7 @@ lxcDomainGetMemoryParameters(virDomainPtr dom,
switch (i) {
case 0: /* fill memory hard limit here */
- if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+ if (persistentDef) {
val = persistentDef->mem.hard_limit;
} else if (virCgroupGetMemoryHardLimit(priv->cgroup, &val) < 0) {
goto cleanup;
@@ -996,7 +996,7 @@ lxcDomainGetMemoryParameters(virDomainPtr dom,
goto cleanup;
break;
case 1: /* fill memory soft limit here */
- if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+ if (persistentDef) {
val = persistentDef->mem.soft_limit;
} else if (virCgroupGetMemorySoftLimit(priv->cgroup, &val) < 0) {
goto cleanup;
@@ -1006,7 +1006,7 @@ lxcDomainGetMemoryParameters(virDomainPtr dom,
goto cleanup;
break;
case 2: /* fill swap hard limit here */
- if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+ if (persistentDef) {
val = persistentDef->mem.swap_hard_limit;
} else if (virCgroupGetMemSwapHardLimit(priv->cgroup, &val) < 0) {
goto cleanup;
--
2.7.3
7:44 a.m.
New subject: [libvirt] [PATCH 5/9] Use virDomainObjGetDefs in lxcDomainGetMemoryParameters
On 06/06/2016 04:08 AM, Ján Tomko wrote:
Instead of virDomainLiveConfigHelperMethod.
---
src/lxc/lxc_driver.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index c39b4b4..00b4df2 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -941,9 +941,9 @@ lxcDomainGetMemoryParameters(virDomainPtr dom,
{
virCapsPtr caps = NULL;
^^
Unused now too and could be removed.
John
virDomainDefPtr persistentDef = NULL;
+ virDomainDefPtr def = NULL;
virDomainObjPtr vm = NULL;
virLXCDomainObjPrivatePtr priv = NULL;
- virLXCDriverPtr driver = dom->conn->privateData;
unsigned long long val;
int ret = -1;
size_t i;
@@ -960,13 +960,13 @@ lxcDomainGetMemoryParameters(virDomainPtr dom,
priv = vm->privateData;
- if (virDomainGetMemoryParametersEnsureACL(dom->conn, vm->def) < 0 ||
- !(caps = virLXCDriverGetCapabilities(driver, false)) ||
- virDomainLiveConfigHelperMethod(caps, driver->xmlopt,
- vm, &flags, &persistentDef) < 0)
+ if (virDomainGetMemoryParametersEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
- if (flags & VIR_DOMAIN_AFFECT_LIVE &&
+ if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0)
+ goto cleanup;
+
+ if (def &&
!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_MEMORY)) {
virReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("cgroup memory controller is not
mounted"));
@@ -986,7 +986,7 @@ lxcDomainGetMemoryParameters(virDomainPtr dom,
switch (i) {
case 0: /* fill memory hard limit here */
- if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+ if (persistentDef) {
val = persistentDef->mem.hard_limit;
} else if (virCgroupGetMemoryHardLimit(priv->cgroup, &val) < 0) {
goto cleanup;
@@ -996,7 +996,7 @@ lxcDomainGetMemoryParameters(virDomainPtr dom,
goto cleanup;
break;
case 1: /* fill memory soft limit here */
- if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+ if (persistentDef) {
val = persistentDef->mem.soft_limit;
} else if (virCgroupGetMemorySoftLimit(priv->cgroup, &val) < 0) {
goto cleanup;
@@ -1006,7 +1006,7 @@ lxcDomainGetMemoryParameters(virDomainPtr dom,
goto cleanup;
break;
case 2: /* fill swap hard limit here */
- if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+ if (persistentDef) {
val = persistentDef->mem.swap_hard_limit;
} else if (virCgroupGetMemSwapHardLimit(priv->cgroup, &val) < 0)
{
goto cleanup;
3:08 a.m.
New subject: [libvirt] [PATCH 6/9] Use virDomainObjGetDefs in lxcDomainSetBlkioParameters
Remove yet another usage of virDomainLiveConfigHelperMethod
along with an sa_assert that helped clang understand the code flow.
---
src/lxc/lxc_driver.c | 22 +++++++---------------
1 file changed, 7 insertions(+), 15 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 00b4df2..a074aa3 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -2570,10 +2570,10 @@ lxcDomainSetBlkioParameters(virDomainPtr dom,
virLXCDriverPtr driver = dom->conn->privateData;
size_t i;
virDomainObjPtr vm = NULL;
+ virDomainDefPtr def = NULL;
virDomainDefPtr persistentDef = NULL;
int ret = -1;
virLXCDriverConfigPtr cfg = NULL;
- virCapsPtr caps = NULL;
virLXCDomainObjPrivatePtr priv;
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
@@ -2603,17 +2603,13 @@ lxcDomainSetBlkioParameters(virDomainPtr dom,
if (virDomainSetBlkioParametersEnsureACL(dom->conn, vm->def, flags) < 0)
goto cleanup;
- if (!(caps = virLXCDriverGetCapabilities(driver, false)))
- goto cleanup;
-
if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0)
goto cleanup;
- if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, vm, &flags,
- &persistentDef) < 0)
+ if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0)
goto endjob;
- if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+ if (def) {
if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("blkio cgroup isn't mounted"));
@@ -2622,7 +2618,7 @@ lxcDomainSetBlkioParameters(virDomainPtr dom,
}
ret = 0;
- if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+ if (def) {
for (i = 0; i < nparams; i++) {
virTypedParameterPtr param = ¶ms[i];
@@ -2717,8 +2713,8 @@ lxcDomainSetBlkioParameters(virDomainPtr dom,
}
if (j != ndevices ||
- lxcDomainMergeBlkioDevice(&vm->def->blkio.devices,
- &vm->def->blkio.ndevices,
+ lxcDomainMergeBlkioDevice(&def->blkio.devices,
+ &def->blkio.ndevices,
devices, ndevices, param->field) <
0)
ret = -1;
virBlkioDeviceArrayClear(devices, ndevices);
@@ -2728,10 +2724,7 @@ lxcDomainSetBlkioParameters(virDomainPtr dom,
}
if (ret < 0)
goto endjob;
- if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
- /* Clang can't see that if we get here, persistentDef was set. */
- sa_assert(persistentDef);
-
+ if (persistentDef) {
for (i = 0; i < nparams; i++) {
virTypedParameterPtr param = ¶ms[i];
@@ -2770,7 +2763,6 @@ lxcDomainSetBlkioParameters(virDomainPtr dom,
cleanup:
virDomainObjEndAPI(&vm);
- virObjectUnref(caps);
virObjectUnref(cfg);
return ret;
}
--
2.7.3
3:08 a.m.
New subject: [libvirt] [PATCH 7/9] Use virDomainObjGetDefs in lxcDomainGetBlkioParameters
---
src/lxc/lxc_driver.c | 64 ++++++++++++++++++++++++----------------------------
1 file changed, 29 insertions(+), 35 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index a074aa3..9c89f29 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -2776,13 +2776,12 @@ lxcDomainGetBlkioParameters(virDomainPtr dom,
int *nparams,
unsigned int flags)
{
- virLXCDriverPtr driver = dom->conn->privateData;
size_t i, j;
virDomainObjPtr vm = NULL;
+ virDomainDefPtr def = NULL;
virDomainDefPtr persistentDef = NULL;
unsigned int val;
int ret = -1;
- virCapsPtr caps = NULL;
virLXCDomainObjPrivatePtr priv;
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
@@ -2802,9 +2801,6 @@ lxcDomainGetBlkioParameters(virDomainPtr dom,
if (virDomainGetBlkioParametersEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
- if (!(caps = virLXCDriverGetCapabilities(driver, false)))
- goto cleanup;
-
if ((*nparams) == 0) {
/* Current number of blkio parameters supported by cgroups */
*nparams = LXC_NB_BLKIO_PARAM;
@@ -2812,11 +2808,10 @@ lxcDomainGetBlkioParameters(virDomainPtr dom,
goto cleanup;
}
- if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, vm, &flags,
- &persistentDef) < 0)
+ if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0)
goto cleanup;
- if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+ if (def) {
if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("blkio cgroup isn't mounted"));
@@ -2837,20 +2832,20 @@ lxcDomainGetBlkioParameters(virDomainPtr dom,
break;
case 1: /* blkiotune.device_weight */
- if (vm->def->blkio.ndevices > 0) {
+ if (def->blkio.ndevices > 0) {
virBuffer buf = VIR_BUFFER_INITIALIZER;
bool comma = false;
- for (j = 0; j < vm->def->blkio.ndevices; j++) {
- if (!vm->def->blkio.devices[j].weight)
+ for (j = 0; j < def->blkio.ndevices; j++) {
+ if (!def->blkio.devices[j].weight)
continue;
if (comma)
virBufferAddChar(&buf, ',');
else
comma = true;
virBufferAsprintf(&buf, "%s,%u",
- vm->def->blkio.devices[j].path,
- vm->def->blkio.devices[j].weight);
+ def->blkio.devices[j].path,
+ def->blkio.devices[j].weight);
}
if (virBufferCheckError(&buf) < 0)
goto cleanup;
@@ -2864,20 +2859,20 @@ lxcDomainGetBlkioParameters(virDomainPtr dom,
break;
case 2: /* blkiotune.device_read_iops */
- if (vm->def->blkio.ndevices > 0) {
+ if (def->blkio.ndevices > 0) {
virBuffer buf = VIR_BUFFER_INITIALIZER;
bool comma = false;
- for (j = 0; j < vm->def->blkio.ndevices; j++) {
- if (!vm->def->blkio.devices[j].riops)
+ for (j = 0; j < def->blkio.ndevices; j++) {
+ if (!def->blkio.devices[j].riops)
continue;
if (comma)
virBufferAddChar(&buf, ',');
else
comma = true;
virBufferAsprintf(&buf, "%s,%u",
- vm->def->blkio.devices[j].path,
- vm->def->blkio.devices[j].riops);
+ def->blkio.devices[j].path,
+ def->blkio.devices[j].riops);
}
if (virBufferCheckError(&buf) < 0)
goto cleanup;
@@ -2891,20 +2886,20 @@ lxcDomainGetBlkioParameters(virDomainPtr dom,
break;
case 3: /* blkiotune.device_write_iops */
- if (vm->def->blkio.ndevices > 0) {
+ if (def->blkio.ndevices > 0) {
virBuffer buf = VIR_BUFFER_INITIALIZER;
bool comma = false;
- for (j = 0; j < vm->def->blkio.ndevices; j++) {
- if (!vm->def->blkio.devices[j].wiops)
+ for (j = 0; j < def->blkio.ndevices; j++) {
+ if (!def->blkio.devices[j].wiops)
continue;
if (comma)
virBufferAddChar(&buf, ',');
else
comma = true;
virBufferAsprintf(&buf, "%s,%u",
- vm->def->blkio.devices[j].path,
- vm->def->blkio.devices[j].wiops);
+ def->blkio.devices[j].path,
+ def->blkio.devices[j].wiops);
}
if (virBufferCheckError(&buf) < 0)
goto cleanup;
@@ -2918,20 +2913,20 @@ lxcDomainGetBlkioParameters(virDomainPtr dom,
break;
case 4: /* blkiotune.device_read_bps */
- if (vm->def->blkio.ndevices > 0) {
+ if (def->blkio.ndevices > 0) {
virBuffer buf = VIR_BUFFER_INITIALIZER;
bool comma = false;
- for (j = 0; j < vm->def->blkio.ndevices; j++) {
- if (!vm->def->blkio.devices[j].rbps)
+ for (j = 0; j < def->blkio.ndevices; j++) {
+ if (!def->blkio.devices[j].rbps)
continue;
if (comma)
virBufferAddChar(&buf, ',');
else
comma = true;
virBufferAsprintf(&buf, "%s,%llu",
- vm->def->blkio.devices[j].path,
- vm->def->blkio.devices[j].rbps);
+ def->blkio.devices[j].path,
+ def->blkio.devices[j].rbps);
}
if (virBufferCheckError(&buf) < 0)
goto cleanup;
@@ -2945,20 +2940,20 @@ lxcDomainGetBlkioParameters(virDomainPtr dom,
break;
case 5: /* blkiotune.device_write_bps */
- if (vm->def->blkio.ndevices > 0) {
+ if (def->blkio.ndevices > 0) {
virBuffer buf = VIR_BUFFER_INITIALIZER;
bool comma = false;
- for (j = 0; j < vm->def->blkio.ndevices; j++) {
- if (!vm->def->blkio.devices[j].wbps)
+ for (j = 0; j < def->blkio.ndevices; j++) {
+ if (!def->blkio.devices[j].wbps)
continue;
if (comma)
virBufferAddChar(&buf, ',');
else
comma = true;
virBufferAsprintf(&buf, "%s,%llu",
- vm->def->blkio.devices[j].path,
- vm->def->blkio.devices[j].wbps);
+ def->blkio.devices[j].path,
+ def->blkio.devices[j].wbps);
}
if (virBufferCheckError(&buf) < 0)
goto cleanup;
@@ -2972,7 +2967,7 @@ lxcDomainGetBlkioParameters(virDomainPtr dom,
break;
}
}
- } else if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+ } else if (persistentDef) {
for (i = 0; i < *nparams && i < LXC_NB_BLKIO_PARAM; i++) {
virTypedParameterPtr param = ¶ms[i];
val = 0;
@@ -3157,7 +3152,6 @@ lxcDomainGetBlkioParameters(virDomainPtr dom,
cleanup:
virDomainObjEndAPI(&vm);
- virObjectUnref(caps);
return ret;
}
--
2.7.3
3:08 a.m.
New subject: [libvirt] [PATCH 8/9] Export virDomainGetBlkioParametersAssignFromDef
Move qemuDomainGetBlkioParametersAssignFromDef into domain_conf
and export it, to allow reuse in the LXC driver.
---
src/conf/domain_conf.c | 51 +++++++++++++++++++++++++++++++++++++++++
src/conf/domain_conf.h | 6 +++++
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 59 ++++--------------------------------------------
4 files changed, 62 insertions(+), 55 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 11ec80d..0c09546 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -24558,3 +24558,54 @@ virDomainObjGetShortName(virDomainObjPtr vm)
return ret;
}
+
+
+int
+virDomainGetBlkioParametersAssignFromDef(virDomainDefPtr def,
+ virTypedParameterPtr params,
+ int *nparams,
+ int maxparams)
+{
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ char *data = NULL;
+ size_t i;
+
+#define VIR_BLKIO_ASSIGN(param, format, name) \
+ if (*nparams < maxparams) { \
+ for (i = 0; i < def->blkio.ndevices; i++) { \
+ if (!def->blkio.devices[i].param) \
+ continue; \
+ virBufferAsprintf(&buf, "%s," format ",",
\
+ def->blkio.devices[i].path, \
+ def->blkio.devices[i].param); \
+ } \
+ virBufferTrim(&buf, ",", -1);
\
+ if (virBufferCheckError(&buf) < 0) \
+ goto error; \
+ data = virBufferContentAndReset(&buf); \
+ if (virTypedParameterAssign(&(params[(*nparams)++]), name, \
+ VIR_TYPED_PARAM_STRING, data) < 0) \
+ goto error; \
+ VIR_FREE(data); \
+ }
+
+ /* blkiotune.device_weight */
+ VIR_BLKIO_ASSIGN(weight, "%u", VIR_DOMAIN_BLKIO_DEVICE_WEIGHT);
+ /* blkiotune.device_read_iops */
+ VIR_BLKIO_ASSIGN(riops, "%u", VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS);
+ /* blkiotune.device_write_iops */
+ VIR_BLKIO_ASSIGN(wiops, "%u", VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS);
+ /* blkiotune.device_read_bps */
+ VIR_BLKIO_ASSIGN(rbps, "%llu", VIR_DOMAIN_BLKIO_DEVICE_READ_BPS);
+ /* blkiotune.device_write_bps */
+ VIR_BLKIO_ASSIGN(wbps, "%llu", VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS);
+
+#undef VIR_BLKIO_ASSIGN
+
+ return 0;
+
+ error:
+ VIR_FREE(data);
+ virBufferFreeAndReset(&buf);
+ return -1;
+}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index c182747..0d18304 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -52,6 +52,7 @@
# include "virprocess.h"
# include "virgic.h"
# include "virperf.h"
+# include "virtypedparam.h"
/* forward declarations of all device types, required by
* virDomainDeviceDef
@@ -3062,4 +3063,9 @@ bool virDomainDefHasMemballoon(const virDomainDef *def)
ATTRIBUTE_NONNULL(1);
char *virDomainObjGetShortName(virDomainObjPtr vm);
+int
+virDomainGetBlkioParametersAssignFromDef(virDomainDefPtr def,
+ virTypedParameterPtr params,
+ int *nparams,
+ int maxparams);
#endif /* __DOMAIN_CONF_H */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 333bf7c..3968a8d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -93,6 +93,7 @@ virDomainCCWAddressReleaseAddr;
virDomainCCWAddressSetCreate;
virDomainCCWAddressSetFree;
virDomainCCWAddressValidate;
+virDomainGetBlkioParametersAssignFromDef;
virDomainPCIAddressAsString;
virDomainPCIAddressBusSetModel;
virDomainPCIAddressEnsureAddr;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 10d3e3d..1a674af 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9083,57 +9083,6 @@ qemuDomainSetBlkioParameters(virDomainPtr dom,
static int
-qemuDomainGetBlkioParametersAssignFromDef(virDomainDefPtr def,
- virTypedParameterPtr params,
- int *nparams,
- int maxparams)
-{
- virBuffer buf = VIR_BUFFER_INITIALIZER;
- char *data = NULL;
- size_t i;
-
-#define QEMU_BLKIO_ASSIGN(param, format, name) \
- if (*nparams < maxparams) { \
- for (i = 0; i < def->blkio.ndevices; i++) { \
- if (!def->blkio.devices[i].param) \
- continue; \
- virBufferAsprintf(&buf, "%s," format ",",
\
- def->blkio.devices[i].path, \
- def->blkio.devices[i].param); \
- } \
- virBufferTrim(&buf, ",", -1);
\
- if (virBufferCheckError(&buf) < 0) \
- goto error; \
- data = virBufferContentAndReset(&buf); \
- if (virTypedParameterAssign(&(params[(*nparams)++]), name, \
- VIR_TYPED_PARAM_STRING, data) < 0) \
- goto error; \
- VIR_FREE(data); \
- }
-
- /* blkiotune.device_weight */
- QEMU_BLKIO_ASSIGN(weight, "%u", VIR_DOMAIN_BLKIO_DEVICE_WEIGHT);
- /* blkiotune.device_read_iops */
- QEMU_BLKIO_ASSIGN(riops, "%u", VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS);
- /* blkiotune.device_write_iops */
- QEMU_BLKIO_ASSIGN(wiops, "%u", VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS);
- /* blkiotune.device_read_bps */
- QEMU_BLKIO_ASSIGN(rbps, "%llu", VIR_DOMAIN_BLKIO_DEVICE_READ_BPS);
- /* blkiotune.device_write_bps */
- QEMU_BLKIO_ASSIGN(wbps, "%llu", VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS);
-
-#undef QEMU_BLKIO_ASSIGN
-
- return 0;
-
- error:
- VIR_FREE(data);
- virBufferFreeAndReset(&buf);
- return -1;
-}
-
-
-static int
qemuDomainGetBlkioParameters(virDomainPtr dom,
virTypedParameterPtr params,
int *nparams,
@@ -9200,8 +9149,8 @@ qemuDomainGetBlkioParameters(virDomainPtr dom,
VIR_TYPED_PARAM_UINT, val) < 0)
goto cleanup;
- if (qemuDomainGetBlkioParametersAssignFromDef(def, params, nparams,
- maxparams) < 0)
+ if (virDomainGetBlkioParametersAssignFromDef(def, params, nparams,
+ maxparams) < 0)
goto cleanup;
} else if (persistentDef) {
@@ -9212,8 +9161,8 @@ qemuDomainGetBlkioParameters(virDomainPtr dom,
persistentDef->blkio.weight) < 0)
goto cleanup;
- if (qemuDomainGetBlkioParametersAssignFromDef(persistentDef, params,
- nparams, maxparams) < 0)
+ if (virDomainGetBlkioParametersAssignFromDef(persistentDef, params,
+ nparams, maxparams) < 0)
goto cleanup;
}
--
2.7.3
3:08 a.m.
New subject: [libvirt] [PATCH 9/9] lxc: simplify lxcDomainGetBlkioParameters
Replace all the repetitive code by using
virDomainGetBlkioParametersAssignFromDef.
---
src/lxc/lxc_driver.c | 349 ++++-----------------------------------------------
1 file changed, 24 insertions(+), 325 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 9c89f29..ff04d5d 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -2776,10 +2776,10 @@ lxcDomainGetBlkioParameters(virDomainPtr dom,
int *nparams,
unsigned int flags)
{
- size_t i, j;
virDomainObjPtr vm = NULL;
virDomainDefPtr def = NULL;
virDomainDefPtr persistentDef = NULL;
+ int maxparams = LXC_NB_BLKIO_PARAM;
unsigned int val;
int ret = -1;
virLXCDomainObjPrivatePtr priv;
@@ -2806,8 +2806,12 @@ lxcDomainGetBlkioParameters(virDomainPtr dom,
*nparams = LXC_NB_BLKIO_PARAM;
ret = 0;
goto cleanup;
+ } else if (*nparams < maxparams) {
+ maxparams = *nparams;
}
+ *nparams = 0;
+
if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0)
goto cleanup;
@@ -2818,336 +2822,31 @@ lxcDomainGetBlkioParameters(virDomainPtr dom,
goto cleanup;
}
- for (i = 0; i < *nparams && i < LXC_NB_BLKIO_PARAM; i++) {
- virTypedParameterPtr param = ¶ms[i];
- val = 0;
-
- switch (i) {
- case 0: /* fill blkio weight here */
- if (virCgroupGetBlkioWeight(priv->cgroup, &val) < 0)
- goto cleanup;
- if (virTypedParameterAssign(param, VIR_DOMAIN_BLKIO_WEIGHT,
- VIR_TYPED_PARAM_UINT, val) < 0)
- goto cleanup;
- break;
-
- case 1: /* blkiotune.device_weight */
- if (def->blkio.ndevices > 0) {
- virBuffer buf = VIR_BUFFER_INITIALIZER;
- bool comma = false;
-
- for (j = 0; j < def->blkio.ndevices; j++) {
- if (!def->blkio.devices[j].weight)
- continue;
- if (comma)
- virBufferAddChar(&buf, ',');
- else
- comma = true;
- virBufferAsprintf(&buf, "%s,%u",
- def->blkio.devices[j].path,
- def->blkio.devices[j].weight);
- }
- if (virBufferCheckError(&buf) < 0)
- goto cleanup;
- param->value.s = virBufferContentAndReset(&buf);
- }
- if (virTypedParameterAssign(param,
- VIR_DOMAIN_BLKIO_DEVICE_WEIGHT,
- VIR_TYPED_PARAM_STRING,
- param->value.s) < 0)
- goto cleanup;
- break;
-
- case 2: /* blkiotune.device_read_iops */
- if (def->blkio.ndevices > 0) {
- virBuffer buf = VIR_BUFFER_INITIALIZER;
- bool comma = false;
-
- for (j = 0; j < def->blkio.ndevices; j++) {
- if (!def->blkio.devices[j].riops)
- continue;
- if (comma)
- virBufferAddChar(&buf, ',');
- else
- comma = true;
- virBufferAsprintf(&buf, "%s,%u",
- def->blkio.devices[j].path,
- def->blkio.devices[j].riops);
- }
- if (virBufferCheckError(&buf) < 0)
- goto cleanup;
- param->value.s = virBufferContentAndReset(&buf);
- }
- if (virTypedParameterAssign(param,
- VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS,
- VIR_TYPED_PARAM_STRING,
- param->value.s) < 0)
- goto cleanup;
- break;
-
- case 3: /* blkiotune.device_write_iops */
- if (def->blkio.ndevices > 0) {
- virBuffer buf = VIR_BUFFER_INITIALIZER;
- bool comma = false;
-
- for (j = 0; j < def->blkio.ndevices; j++) {
- if (!def->blkio.devices[j].wiops)
- continue;
- if (comma)
- virBufferAddChar(&buf, ',');
- else
- comma = true;
- virBufferAsprintf(&buf, "%s,%u",
- def->blkio.devices[j].path,
- def->blkio.devices[j].wiops);
- }
- if (virBufferCheckError(&buf) < 0)
- goto cleanup;
- param->value.s = virBufferContentAndReset(&buf);
- }
- if (virTypedParameterAssign(param,
- VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS,
- VIR_TYPED_PARAM_STRING,
- param->value.s) < 0)
- goto cleanup;
- break;
+ /* fill blkio weight here */
+ if (virCgroupGetBlkioWeight(priv->cgroup, &val) < 0)
+ goto cleanup;
+ if (virTypedParameterAssign(&(params[(*nparams)++]),
+ VIR_DOMAIN_BLKIO_WEIGHT,
+ VIR_TYPED_PARAM_UINT, val) < 0)
+ goto cleanup;
- case 4: /* blkiotune.device_read_bps */
- if (def->blkio.ndevices > 0) {
- virBuffer buf = VIR_BUFFER_INITIALIZER;
- bool comma = false;
-
- for (j = 0; j < def->blkio.ndevices; j++) {
- if (!def->blkio.devices[j].rbps)
- continue;
- if (comma)
- virBufferAddChar(&buf, ',');
- else
- comma = true;
- virBufferAsprintf(&buf, "%s,%llu",
- def->blkio.devices[j].path,
- def->blkio.devices[j].rbps);
- }
- if (virBufferCheckError(&buf) < 0)
- goto cleanup;
- param->value.s = virBufferContentAndReset(&buf);
- }
- if (virTypedParameterAssign(param,
- VIR_DOMAIN_BLKIO_DEVICE_READ_BPS,
- VIR_TYPED_PARAM_STRING,
- param->value.s) < 0)
- goto cleanup;
- break;
+ if (virDomainGetBlkioParametersAssignFromDef(def, params, nparams,
+ maxparams) < 0)
+ goto cleanup;
- case 5: /* blkiotune.device_write_bps */
- if (def->blkio.ndevices > 0) {
- virBuffer buf = VIR_BUFFER_INITIALIZER;
- bool comma = false;
-
- for (j = 0; j < def->blkio.ndevices; j++) {
- if (!def->blkio.devices[j].wbps)
- continue;
- if (comma)
- virBufferAddChar(&buf, ',');
- else
- comma = true;
- virBufferAsprintf(&buf, "%s,%llu",
- def->blkio.devices[j].path,
- def->blkio.devices[j].wbps);
- }
- if (virBufferCheckError(&buf) < 0)
- goto cleanup;
- param->value.s = virBufferContentAndReset(&buf);
- }
- if (virTypedParameterAssign(param,
- VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS,
- VIR_TYPED_PARAM_STRING,
- param->value.s) < 0)
- goto cleanup;
- break;
- }
- }
} else if (persistentDef) {
- for (i = 0; i < *nparams && i < LXC_NB_BLKIO_PARAM; i++) {
- virTypedParameterPtr param = ¶ms[i];
- val = 0;
- param->value.ui = 0;
- param->type = VIR_TYPED_PARAM_UINT;
-
- switch (i) {
- case 0: /* fill blkio weight here */
- if (virStrcpyStatic(param->field, VIR_DOMAIN_BLKIO_WEIGHT) == NULL) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Field name '%s' too long"),
- VIR_DOMAIN_BLKIO_WEIGHT);
- goto cleanup;
- }
- param->value.ui = persistentDef->blkio.weight;
- break;
-
- case 1: /* blkiotune.device_weight */
- if (persistentDef->blkio.ndevices > 0) {
- virBuffer buf = VIR_BUFFER_INITIALIZER;
- bool comma = false;
-
- for (j = 0; j < persistentDef->blkio.ndevices; j++) {
- if (!persistentDef->blkio.devices[j].weight)
- continue;
- if (comma)
- virBufferAddChar(&buf, ',');
- else
- comma = true;
- virBufferAsprintf(&buf, "%s,%u",
- persistentDef->blkio.devices[j].path,
- persistentDef->blkio.devices[j].weight);
- }
- if (virBufferCheckError(&buf) < 0)
- goto cleanup;
- param->value.s = virBufferContentAndReset(&buf);
- }
- if (!param->value.s && VIR_STRDUP(param->value.s,
"") < 0)
- goto cleanup;
- param->type = VIR_TYPED_PARAM_STRING;
- if (virStrcpyStatic(param->field,
- VIR_DOMAIN_BLKIO_DEVICE_WEIGHT) == NULL) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Field name '%s' too long"),
- VIR_DOMAIN_BLKIO_DEVICE_WEIGHT);
- goto cleanup;
- }
- break;
-
- case 2: /* blkiotune.device_read_iops */
- if (persistentDef->blkio.ndevices > 0) {
- virBuffer buf = VIR_BUFFER_INITIALIZER;
- bool comma = false;
-
- for (j = 0; j < persistentDef->blkio.ndevices; j++) {
- if (!persistentDef->blkio.devices[j].riops)
- continue;
- if (comma)
- virBufferAddChar(&buf, ',');
- else
- comma = true;
- virBufferAsprintf(&buf, "%s,%u",
- persistentDef->blkio.devices[j].path,
- persistentDef->blkio.devices[j].riops);
- }
- if (virBufferCheckError(&buf) < 0)
- goto cleanup;
- param->value.s = virBufferContentAndReset(&buf);
- }
- if (!param->value.s && VIR_STRDUP(param->value.s,
"") < 0)
- goto cleanup;
- param->type = VIR_TYPED_PARAM_STRING;
- if (virStrcpyStatic(param->field,
- VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS) == NULL) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Field name '%s' too long"),
- VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS);
- goto cleanup;
- }
- break;
- case 3: /* blkiotune.device_write_iops */
- if (persistentDef->blkio.ndevices > 0) {
- virBuffer buf = VIR_BUFFER_INITIALIZER;
- bool comma = false;
-
- for (j = 0; j < persistentDef->blkio.ndevices; j++) {
- if (!persistentDef->blkio.devices[j].wiops)
- continue;
- if (comma)
- virBufferAddChar(&buf, ',');
- else
- comma = true;
- virBufferAsprintf(&buf, "%s,%u",
- persistentDef->blkio.devices[j].path,
- persistentDef->blkio.devices[j].wiops);
- }
- if (virBufferCheckError(&buf) < 0)
- goto cleanup;
- param->value.s = virBufferContentAndReset(&buf);
- }
- if (!param->value.s && VIR_STRDUP(param->value.s,
"") < 0)
- goto cleanup;
- param->type = VIR_TYPED_PARAM_STRING;
- if (virStrcpyStatic(param->field,
- VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS) == NULL) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Field name '%s' too long"),
- VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS);
- goto cleanup;
- }
- break;
- case 4: /* blkiotune.device_read_bps */
- if (persistentDef->blkio.ndevices > 0) {
- virBuffer buf = VIR_BUFFER_INITIALIZER;
- bool comma = false;
-
- for (j = 0; j < persistentDef->blkio.ndevices; j++) {
- if (!persistentDef->blkio.devices[j].rbps)
- continue;
- if (comma)
- virBufferAddChar(&buf, ',');
- else
- comma = true;
- virBufferAsprintf(&buf, "%s,%llu",
- persistentDef->blkio.devices[j].path,
- persistentDef->blkio.devices[j].rbps);
- }
- if (virBufferCheckError(&buf) < 0)
- goto cleanup;
- param->value.s = virBufferContentAndReset(&buf);
- }
- if (!param->value.s && VIR_STRDUP(param->value.s,
"") < 0)
- goto cleanup;
- param->type = VIR_TYPED_PARAM_STRING;
- if (virStrcpyStatic(param->field,
- VIR_DOMAIN_BLKIO_DEVICE_READ_BPS) == NULL) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Field name '%s' too long"),
- VIR_DOMAIN_BLKIO_DEVICE_READ_BPS);
- goto cleanup;
- }
- break;
+ /* fill blkio weight here */
+ if (virTypedParameterAssign(&(params[(*nparams)++]),
+ VIR_DOMAIN_BLKIO_WEIGHT,
+ VIR_TYPED_PARAM_UINT,
+ persistentDef->blkio.weight) < 0)
+ goto cleanup;
- case 5: /* blkiotune.device_write_bps */
- if (persistentDef->blkio.ndevices > 0) {
- virBuffer buf = VIR_BUFFER_INITIALIZER;
- bool comma = false;
-
- for (j = 0; j < persistentDef->blkio.ndevices; j++) {
- if (!persistentDef->blkio.devices[j].wbps)
- continue;
- if (comma)
- virBufferAddChar(&buf, ',');
- else
- comma = true;
- virBufferAsprintf(&buf, "%s,%llu",
- persistentDef->blkio.devices[j].path,
- persistentDef->blkio.devices[j].wbps);
- }
- if (virBufferCheckError(&buf) < 0)
- goto cleanup;
- param->value.s = virBufferContentAndReset(&buf);
- }
- if (!param->value.s && VIR_STRDUP(param->value.s,
"") < 0)
- goto cleanup;
- param->type = VIR_TYPED_PARAM_STRING;
- if (virStrcpyStatic(param->field,
- VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS) == NULL) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Field name '%s' too long"),
- VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS);
- goto cleanup;
- }
- break;
- }
- }
+ if (virDomainGetBlkioParametersAssignFromDef(persistentDef, params,
+ nparams, maxparams) < 0)
+ goto cleanup;
}
- if (LXC_NB_BLKIO_PARAM < *nparams)
- *nparams = LXC_NB_BLKIO_PARAM;
ret = 0;
cleanup:
--
2.7.3
7:45 a.m.
On 06/06/2016 04:08 AM, Ján Tomko wrote:
In a few API implementations, we use virDomainLiveConfigHelperMethod
to check the VIR_DOMAIN_AFFECT_* flags (and change the flags variable
to either AFFECT_LIVE or AFFECT_CONFIG if AFFECT_CURRENT was requested)
and optionally give us a pointer to the persistent definition.
This function can also create the persistent definition in vm->newDef
for a live domain if it does not exist yet. This cannot be the case
in LXC driver because we create it unconditionally on domain startup.
By switching to virDomainObjGetDefs we do not need to pass virCaps
and virDomainXMLOption (because the defintion does not need to be copied).
Also, instead of altering the flags, it fills the pointers to live and persistent
definitions depending on which one was requested to be modified,
making the code easier to read for static analyzers.
Ján Tomko (9):
lxc: rename vmdef to persistentDef
Use virDomainObjGetDefs in lxcDomainSetMemoryFlags
Use virDomainObjGetDefs in lxcDomainSetSchedulerParametersFlags
Use virDomainObjGetDefs in lxcDomainGetSchedulerParametersFlags
Use virDomainObjGetDefs in lxcDomainGetMemoryParameters
Use virDomainObjGetDefs in lxcDomainSetBlkioParameters
Use virDomainObjGetDefs in lxcDomainGetBlkioParameters
Export virDomainGetBlkioParametersAssignFromDef
lxc: simplify lxcDomainGetBlkioParameters
src/conf/domain_conf.c | 51 +++++
src/conf/domain_conf.h | 6 +
src/libvirt_private.syms | 1 +
src/lxc/lxc_driver.c | 511 +++++++++--------------------------------------
src/qemu/qemu_driver.c | 59 +-----
5 files changed, 154 insertions(+), 474 deletions(-)
In patch 9 you could reference commit id '9f50f6e28' ... although it's
also possible to figure it out with a bit of work...
Should lxcDomainSetMemoryParameters get the same virDomainObjGetDefs usage?
ACK series - just adjust patch 5 to remove the caps.
John
8:37 a.m.
On Wed, Jun 08, 2016 at 08:45:21AM -0400, John Ferlan wrote:
On 06/06/2016 04:08 AM, Ján Tomko wrote:
> In a few API implementations, we use virDomainLiveConfigHelperMethod
> to check the VIR_DOMAIN_AFFECT_* flags (and change the flags variable
> to either AFFECT_LIVE or AFFECT_CONFIG if AFFECT_CURRENT was requested)
> and optionally give us a pointer to the persistent definition.
>
> This function can also create the persistent definition in vm->newDef
> for a live domain if it does not exist yet. This cannot be the case
> in LXC driver because we create it unconditionally on domain startup.
>
>
> By switching to virDomainObjGetDefs we do not need to pass virCaps
> and virDomainXMLOption (because the defintion does not need to be copied).
>
> Also, instead of altering the flags, it fills the pointers to live and persistent
> definitions depending on which one was requested to be modified,
> making the code easier to read for static analyzers.
>
>
> Ján Tomko (9):
> lxc: rename vmdef to persistentDef
> Use virDomainObjGetDefs in lxcDomainSetMemoryFlags
> Use virDomainObjGetDefs in lxcDomainSetSchedulerParametersFlags
> Use virDomainObjGetDefs in lxcDomainGetSchedulerParametersFlags
> Use virDomainObjGetDefs in lxcDomainGetMemoryParameters
> Use virDomainObjGetDefs in lxcDomainSetBlkioParameters
> Use virDomainObjGetDefs in lxcDomainGetBlkioParameters
> Export virDomainGetBlkioParametersAssignFromDef
> lxc: simplify lxcDomainGetBlkioParameters
>
> src/conf/domain_conf.c | 51 +++++
> src/conf/domain_conf.h | 6 +
> src/libvirt_private.syms | 1 +
> src/lxc/lxc_driver.c | 511 +++++++++--------------------------------------
> src/qemu/qemu_driver.c | 59 +-----
> 5 files changed, 154 insertions(+), 474 deletions(-)
>
In patch 9 you could reference commit id '9f50f6e28' ... although it's
also possible to figure it out with a bit of work...
Should lxcDomainSetMemoryParameters get the same virDomainObjGetDefs usage?
Yes, I must've missed it for some reason.
ACK series - just adjust patch 5 to remove the caps.
Thanks, I have added the commit reference, fixed the nits and pushed the
series.
Jan
3089
days inactive
3091
days old
14 comments
2 participants
participants (2)
-
John Ferlan -
Ján Tomko