[libvirt] [PATCH 0/9] LXC: Use virDomainObjGetDefs more
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
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
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
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; }
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
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
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; }
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
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;
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
--- 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
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
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
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
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
participants (2)
-
John Ferlan -
Ján Tomko