[libvirt] [PATCH] qemu: Fix setting of memory tunables
Peter Krempa
pkrempa at redhat.com
Fri Apr 19 10:01:21 UTC 2013
Refactoring done in 19c6ad9ac7e7eb2fd3c8262bff5f087b508ad07f didn't
correctly take into account the order cgroup limit modification needs to
be done in. This resulted into errors when decreasing the limits.
The operations need to take place in this order:
decrease hard limit
change swap hard limit
or
change swap hard limit
increase hard limit
This patch also fixes the check if the hard_limit is less than
swap_hard_limit to print better error messages. For this purpose I
introduced a helper function virCompareLimitUlong to compare limit
values where value of 0 is equal to unlimited. Additionally the check is
now applied also when the user does not provide all of the tunables
through the API and in that case the currently set values are used.
This patch resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=950478
---
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 95 ++++++++++++++++++++++++------------------------
src/util/virutil.c | 12 ++++++
src/util/virutil.h | 2 +
4 files changed, 63 insertions(+), 47 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 30fdcd7..ec644ee 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1835,6 +1835,7 @@ safezero;
virArgvToString;
virAsprintf;
virBuildPathInternal;
+virCompareLimitUlong;
virDirCreate;
virDoubleToStr;
virEnumFromString;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index cee5557..a174fb0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7103,11 +7103,11 @@ qemuDomainSetMemoryParameters(virDomainPtr dom,
virDomainDefPtr persistentDef = NULL;
virDomainObjPtr vm = NULL;
unsigned long long swap_hard_limit;
- unsigned long long memory_hard_limit;
- unsigned long long memory_soft_limit;
+ unsigned long long hard_limit = 0;
+ unsigned long long soft_limit = 0;
bool set_swap_hard_limit = false;
- bool set_memory_hard_limit = false;
- bool set_memory_soft_limit = false;
+ bool set_hard_limit = false;
+ bool set_soft_limit = false;
virQEMUDriverConfigPtr cfg = NULL;
int ret = -1;
int rc;
@@ -7157,62 +7157,63 @@ qemuDomainSetMemoryParameters(virDomainPtr dom,
set_ ## VALUE = true;
VIR_GET_LIMIT_PARAMETER(VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT, swap_hard_limit)
- VIR_GET_LIMIT_PARAMETER(VIR_DOMAIN_MEMORY_HARD_LIMIT, memory_hard_limit)
- VIR_GET_LIMIT_PARAMETER(VIR_DOMAIN_MEMORY_SOFT_LIMIT, memory_soft_limit)
+ VIR_GET_LIMIT_PARAMETER(VIR_DOMAIN_MEMORY_HARD_LIMIT, hard_limit)
+ VIR_GET_LIMIT_PARAMETER(VIR_DOMAIN_MEMORY_SOFT_LIMIT, soft_limit)
#undef VIR_GET_LIMIT_PARAMETER
+ /* Swap hard limit must be greater than hard limit.
+ * Note that limit of 0 denotes unlimited */
+ if (set_swap_hard_limit || set_hard_limit) {
+ unsigned long long mem_limit = vm->def->mem.hard_limit;
+ unsigned long long swap_limit = vm->def->mem.swap_hard_limit;
- /* It will fail if hard limit greater than swap hard limit anyway */
- if (set_swap_hard_limit && set_memory_hard_limit &&
- memory_hard_limit > swap_hard_limit) {
- virReportError(VIR_ERR_INVALID_ARG, "%s",
- _("memory hard_limit tunable value must be lower than "
- "swap_hard_limit"));
- goto cleanup;
- }
+ if (set_swap_hard_limit)
+ swap_limit = swap_hard_limit;
- if (set_swap_hard_limit) {
- if (flags & VIR_DOMAIN_AFFECT_LIVE) {
- if ((rc = virCgroupSetMemSwapHardLimit(priv->cgroup, swap_hard_limit)) < 0) {
- virReportSystemError(-rc, "%s",
- _("unable to set memory swap_hard_limit tunable"));
- goto cleanup;
- }
- vm->def->mem.swap_hard_limit = swap_hard_limit;
+ if (set_hard_limit)
+ mem_limit = hard_limit;
+
+ if (virCompareLimitUlong(mem_limit, swap_limit) > 0) {
+ virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("memory hard_limit tunable value must be lower "
+ "than swap_hard_limit"));
+ goto cleanup;
}
+ }
- if (flags & VIR_DOMAIN_AFFECT_CONFIG)
- persistentDef->mem.swap_hard_limit = swap_hard_limit;
+#define QEMU_SET_MEM_PARAMETER(FUNC, VALUE) \
+ if (set_ ## VALUE) { \
+ if (flags & VIR_DOMAIN_AFFECT_LIVE) { \
+ if ((rc = FUNC(priv->cgroup, VALUE)) < 0) { \
+ virReportSystemError(-rc, _("unable to set memory %s tunable"), \
+ #VALUE); \
+ \
+ goto cleanup; \
+ } \
+ vm->def->mem.VALUE = VALUE; \
+ } \
+ \
+ if (flags & VIR_DOMAIN_AFFECT_CONFIG) \
+ persistentDef->mem.VALUE = VALUE; \
}
- if (set_memory_hard_limit) {
- if (flags & VIR_DOMAIN_AFFECT_LIVE) {
- if ((rc = virCgroupSetMemoryHardLimit(priv->cgroup, memory_hard_limit)) < 0) {
- virReportSystemError(-rc, "%s",
- _("unable to set memory hard_limit tunable"));
- goto cleanup;
- }
- vm->def->mem.hard_limit = memory_hard_limit;
- }
+ /* Soft limit doesn't clash with the others */
+ QEMU_SET_MEM_PARAMETER(virCgroupSetMemorySoftLimit, soft_limit);
- if (flags & VIR_DOMAIN_AFFECT_CONFIG)
- persistentDef->mem.hard_limit = memory_hard_limit;
+ /* set hard limit before swap hard limit if decreasing it */
+ if (virCompareLimitUlong(vm->def->mem.hard_limit, hard_limit) > 0) {
+ QEMU_SET_MEM_PARAMETER(virCgroupSetMemoryHardLimit, hard_limit);
+ /* inhibit changing the limit a second time */
+ set_hard_limit = false;
}
- if (set_memory_soft_limit) {
- if (flags & VIR_DOMAIN_AFFECT_LIVE) {
- if ((rc = virCgroupSetMemorySoftLimit(priv->cgroup, memory_soft_limit)) < 0) {
- virReportSystemError(-rc, "%s",
- _("unable to set memory soft_limit tunable"));
- goto cleanup;
- }
- vm->def->mem.soft_limit = memory_soft_limit;
- }
+ QEMU_SET_MEM_PARAMETER(virCgroupSetMemSwapHardLimit, swap_hard_limit);
- if (flags & VIR_DOMAIN_AFFECT_CONFIG)
- persistentDef->mem.soft_limit = memory_soft_limit;
- }
+ /* otherwise increase it after swap hard limit */
+ QEMU_SET_MEM_PARAMETER(virCgroupSetMemoryHardLimit, hard_limit);
+
+#undef QEMU_SET_MEM_PARAMETER
if (flags & VIR_DOMAIN_AFFECT_CONFIG &&
virDomainSaveConfig(cfg->configDir, persistentDef) < 0)
diff --git a/src/util/virutil.c b/src/util/virutil.c
index 9cc3672..491a004 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -3832,3 +3832,15 @@ virFindFCHostCapableVport(const char *sysfs_prefix ATTRIBUTE_UNUSED)
}
#endif /* __linux__ */
+
+int
+virCompareLimitUlong(unsigned long long a, unsigned long b)
+{
+ if (a == b)
+ return 0;
+
+ if (a == 0 || a > b)
+ return 1;
+
+ return -1;
+}
diff --git a/src/util/virutil.h b/src/util/virutil.h
index 7b37d65..39033db 100644
--- a/src/util/virutil.h
+++ b/src/util/virutil.h
@@ -324,4 +324,6 @@ char *virGetFCHostNameByWWN(const char *sysfs_prefix,
char *virFindFCHostCapableVport(const char *sysfs_prefix);
+int virCompareLimitUlong(unsigned long long a, unsigned long b);
+
#endif /* __VIR_UTIL_H__ */
--
1.8.1.5
More information about the libvir-list
mailing list