[libvirt] [PATCH] qemu: Ensure the cpuset is formated as expected before passing to cgroup
The parameter value for cpuset could be in special format like "0-10,^7", which is not recongnized by cgroup. This patch is to ensure the cpuset is formated as expected before passing it to cgroup. As a side effect, after the patch, it parses the cpuset early before cgroup setting, to avoid the rollback if cpuset parsing fails afterwards. --- src/qemu/qemu_driver.c | 77 +++++++++++++++++++++++++---------------------- 1 files changed, 41 insertions(+), 36 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 369e8ed..fc412a1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6952,8 +6952,23 @@ qemuDomainSetNumaParameters(virDomainPtr dom, } } else if (STREQ(param->field, VIR_DOMAIN_NUMA_NODESET)) { int rc; - bool savedmask; - char oldnodemask[VIR_DOMAIN_CPUMASK_LEN]; + char *nodeset = NULL; + char *nodeset_str = NULL; + + if (VIR_ALLOC_N(nodeset, VIR_DOMAIN_CPUMASK_LEN) < 0) { + virReportOOMError(); + ret = -1; + goto cleanup; + }; + + if (virDomainCpuSetParse(params[i].value.s, + 0, nodeset, + VIR_DOMAIN_CPUMASK_LEN) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to parse nodeset")); + ret = -1; + continue; + } if (flags & VIR_DOMAIN_AFFECT_LIVE) { if (vm->def->numatune.memory.mode != @@ -6961,72 +6976,62 @@ qemuDomainSetNumaParameters(virDomainPtr dom, virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("change of nodeset for running domain " "requires strict numa mode")); + VIR_FREE(nodeset); ret = -1; continue; } - rc = virCgroupSetCpusetMems(group, params[i].value.s); - if (rc != 0) { + + /* Ensure the cpuset string is formated before passing to cgroup */ + if (!(nodeset_str = virDomainCpuSetFormat(nodeset, + VIR_DOMAIN_CPUMASK_LEN))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to format nodeset")); + VIR_FREE(nodeset); + ret = -1; + continue; + } + + if ((rc = virCgroupSetCpusetMems(group, nodeset_str) != 0)) { virReportSystemError(-rc, "%s", _("unable to set numa tunable")); + VIR_FREE(nodeset); + VIR_FREE(nodeset_str); ret = -1; continue; } + VIR_FREE(nodeset_str); /* update vm->def here so that dumpxml can read the new * values from vm->def. */ - savedmask = false; if (!vm->def->numatune.memory.nodemask) { if (VIR_ALLOC_N(vm->def->numatune.memory.nodemask, VIR_DOMAIN_CPUMASK_LEN) < 0) { virReportOOMError(); + VIR_FREE(nodeset); ret = -1; goto cleanup; } } else { - memcpy(oldnodemask, vm->def->numatune.memory.nodemask, - VIR_DOMAIN_CPUMASK_LEN); - savedmask = true; - } - if (virDomainCpuSetParse(params[i].value.s, - 0, - vm->def->numatune.memory.nodemask, - VIR_DOMAIN_CPUMASK_LEN) < 0) { - if (savedmask) - memcpy(vm->def->numatune.memory.nodemask, - oldnodemask, VIR_DOMAIN_CPUMASK_LEN); - else - VIR_FREE(vm->def->numatune.memory.nodemask); - ret = -1; - continue; + VIR_FREE(vm->def->numatune.memory.nodemask); } + + vm->def->numatune.memory.nodemask = nodeset; } if (flags & VIR_DOMAIN_AFFECT_CONFIG) { - savedmask = false; if (!persistentDef->numatune.memory.nodemask) { if (VIR_ALLOC_N(persistentDef->numatune.memory.nodemask, VIR_DOMAIN_CPUMASK_LEN) < 0) { virReportOOMError(); + VIR_FREE(nodeset); ret = -1; goto cleanup; } } else { - memcpy(oldnodemask, persistentDef->numatune.memory.nodemask, - VIR_DOMAIN_CPUMASK_LEN); - savedmask = true; - } - if (virDomainCpuSetParse(params[i].value.s, - 0, - persistentDef->numatune.memory.nodemask, - VIR_DOMAIN_CPUMASK_LEN) < 0) { - if (savedmask) - memcpy(persistentDef->numatune.memory.nodemask, - oldnodemask, VIR_DOMAIN_CPUMASK_LEN); - else - VIR_FREE(persistentDef->numatune.memory.nodemask); - ret = -1; - continue; + VIR_FREE(persistentDef->numatune.memory.nodemask); } + + persistentDef->numatune.memory.nodemask = nodeset; } } } -- 1.7.7.3
On 08/15/2012 09:35 PM, Osier Yang wrote:
The parameter value for cpuset could be in special format like "0-10,^7", which is not recongnized by cgroup. This patch is to
s/recongnized/recognized/
ensure the cpuset is formated as expected before passing it to
s/formated/formatted/
cgroup. As a side effect, after the patch, it parses the cpuset early before cgroup setting, to avoid the rollback if cpuset parsing fails afterwards. --- src/qemu/qemu_driver.c | 77 +++++++++++++++++++++++++---------------------- 1 files changed, 41 insertions(+), 36 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 369e8ed..fc412a1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6952,8 +6952,23 @@ qemuDomainSetNumaParameters(virDomainPtr dom, } } else if (STREQ(param->field, VIR_DOMAIN_NUMA_NODESET)) { int rc; - bool savedmask; - char oldnodemask[VIR_DOMAIN_CPUMASK_LEN]; + char *nodeset = NULL; + char *nodeset_str = NULL; + + if (VIR_ALLOC_N(nodeset, VIR_DOMAIN_CPUMASK_LEN) < 0) { + virReportOOMError(); + ret = -1; + goto cleanup; + }; + + if (virDomainCpuSetParse(params[i].value.s, + 0, nodeset, + VIR_DOMAIN_CPUMASK_LEN) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to parse nodeset")); + ret = -1; + continue; + }
We have enough of this open-coded conversions in the code base that it would be worth common helper routines to make the conversion much easier to use in the clients (for a later patch). But for the purposes of fixing the bug at hand, your patch does the trick.
if (flags & VIR_DOMAIN_AFFECT_LIVE) { if (vm->def->numatune.memory.mode != @@ -6961,72 +6976,62 @@ qemuDomainSetNumaParameters(virDomainPtr dom, virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("change of nodeset for running domain " "requires strict numa mode")); + VIR_FREE(nodeset); ret = -1; continue; } - rc = virCgroupSetCpusetMems(group, params[i].value.s); - if (rc != 0) { + + /* Ensure the cpuset string is formated before passing to cgroup */
s/formated/formatted/ ACK with nits fixed. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 2012年08月16日 13:06, Eric Blake wrote:
On 08/15/2012 09:35 PM, Osier Yang wrote:
The parameter value for cpuset could be in special format like "0-10,^7", which is not recongnized by cgroup. This patch is to
s/recongnized/recognized/
ensure the cpuset is formated as expected before passing it to
s/formated/formatted/
cgroup. As a side effect, after the patch, it parses the cpuset early before cgroup setting, to avoid the rollback if cpuset parsing fails afterwards. --- src/qemu/qemu_driver.c | 77 +++++++++++++++++++++++++---------------------- 1 files changed, 41 insertions(+), 36 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 369e8ed..fc412a1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6952,8 +6952,23 @@ qemuDomainSetNumaParameters(virDomainPtr dom, } } else if (STREQ(param->field, VIR_DOMAIN_NUMA_NODESET)) { int rc; - bool savedmask; - char oldnodemask[VIR_DOMAIN_CPUMASK_LEN]; + char *nodeset = NULL; + char *nodeset_str = NULL; + + if (VIR_ALLOC_N(nodeset, VIR_DOMAIN_CPUMASK_LEN)< 0) { + virReportOOMError(); + ret = -1; + goto cleanup; + }; + + if (virDomainCpuSetParse(params[i].value.s, + 0, nodeset, + VIR_DOMAIN_CPUMASK_LEN)< 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to parse nodeset")); + ret = -1; + continue; + }
We have enough of this open-coded conversions in the code base that it would be worth common helper routines to make the conversion much easier to use in the clients (for a later patch). But for the purposes of fixing the bug at hand, your patch does the trick.
if (flags& VIR_DOMAIN_AFFECT_LIVE) { if (vm->def->numatune.memory.mode != @@ -6961,72 +6976,62 @@ qemuDomainSetNumaParameters(virDomainPtr dom, virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("change of nodeset for running domain " "requires strict numa mode")); + VIR_FREE(nodeset); ret = -1; continue; } - rc = virCgroupSetCpusetMems(group, params[i].value.s); - if (rc != 0) { + + /* Ensure the cpuset string is formated before passing to cgroup */
s/formated/formatted/
hum, you corrected my typos on "recongnized" and "formated" several times. /shy.
ACK with nits fixed.
Thanks, pushed with the nits fixed. Regards, Osier
participants (2)
-
Eric Blake -
Osier Yang