[libvirt] [PATCH] qemu: Don't ignore CPU tuning config if required cgroups are missing
--- src/qemu/qemu_cgroup.c | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index df67ff3..7298e28 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -546,16 +546,21 @@ int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm) unsigned long long period = vm->def->cputune.period; long long quota = vm->def->cputune.quota; - if (driver->cgroup == NULL) - return 0; /* Not supported, so claim success */ - if ((period || quota) && - !qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) { - virReportError(VIR_ERR_SYSTEM_ERROR, "%s", - _("cgroup cpu is not active")); + (!driver->cgroup || + !qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("cgroup cpu is required for scheduler tuning")); return -1; } + /* We are trying to setup cgroups for CPU pinning, which can also be done + * with virProcessInfoSetAffinity, thus the lack of cgroups is not fatal + * here. + */ + if (driver->cgroup == NULL) + return 0; + rc = virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0); if (rc != 0) { virReportSystemError(-rc, @@ -636,6 +641,14 @@ int qemuSetupCgroupForEmulator(struct qemud_driver *driver, long long quota = vm->def->cputune.emulator_quota; int rc, i; + if ((period || quota) && + (!driver->cgroup || + !qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("cgroup cpu is required for scheduler tuning")); + return -1; + } + if (driver->cgroup == NULL) return 0; /* Not supported, so claim success */ @@ -656,10 +669,8 @@ int qemuSetupCgroupForEmulator(struct qemud_driver *driver, } for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { - if (!qemuCgroupControllerActive(driver, i)) { - VIR_WARN("cgroup %d is not active", i); + if (!qemuCgroupControllerActive(driver, i)) continue; - } rc = virCgroupMoveTask(cgroup, cgroup_emulator, i); if (rc < 0) { virReportSystemError(-rc, -- 1.7.12
A bit of context would be nice On Fri, Aug 31, 2012 at 08:23:15AM +0200, Jiri Denemark wrote:
--- src/qemu/qemu_cgroup.c | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index df67ff3..7298e28 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -546,16 +546,21 @@ int qemuSetupCgroupForVcpu(struct qemud_driver *driver, virDomainObjPtr vm) unsigned long long period = vm->def->cputune.period; long long quota = vm->def->cputune.quota;
- if (driver->cgroup == NULL) - return 0; /* Not supported, so claim success */ - if ((period || quota) && - !qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) { - virReportError(VIR_ERR_SYSTEM_ERROR, "%s", - _("cgroup cpu is not active")); + (!driver->cgroup || + !qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("cgroup cpu is required for scheduler tuning")); return -1; }
+ /* We are trying to setup cgroups for CPU pinning, which can also be done + * with virProcessInfoSetAffinity, thus the lack of cgroups is not fatal + * here. + */ + if (driver->cgroup == NULL) + return 0; + rc = virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0); if (rc != 0) { virReportSystemError(-rc, @@ -636,6 +641,14 @@ int qemuSetupCgroupForEmulator(struct qemud_driver *driver, long long quota = vm->def->cputune.emulator_quota; int rc, i;
+ if ((period || quota) && + (!driver->cgroup || + !qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU))) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("cgroup cpu is required for scheduler tuning")); + return -1; + } + if (driver->cgroup == NULL) return 0; /* Not supported, so claim success */
@@ -656,10 +669,8 @@ int qemuSetupCgroupForEmulator(struct qemud_driver *driver, }
for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { - if (!qemuCgroupControllerActive(driver, i)) { - VIR_WARN("cgroup %d is not active", i); + if (!qemuCgroupControllerActive(driver, i)) continue; - } rc = virCgroupMoveTask(cgroup, cgroup_emulator, i); if (rc < 0) { virReportSystemError(-rc,
ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Fri, Aug 31, 2012 at 14:50:32 +0800, Daniel Veillard wrote:
A bit of context would be nice
On Fri, Aug 31, 2012 at 08:23:15AM +0200, Jiri Denemark wrote:
--- src/qemu/qemu_cgroup.c | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-)
ACK,
Thanks, I added the following background story to the commit and pushed: When domain XML contains any of the elements for setting up CPU scheduling parameters (period, quota, emulator_period, or emulator_quota) we need cpu cgroup to enforce the configuration. However, the existing code would just ignore silently such settings if either cgroups were not available at all cpu cgroup was not available. Moreover, APIs for manipulating CPU scheduler parameters were already failing if cpu cgroup was not available. This patch makes cpu cgroup mandatory for all domains that use CPU scheduling elements in their XML. Jirka
participants (2)
-
Daniel Veillard -
Jiri Denemark