On 08/16/2012 12:37 PM, Osier Yang wrote:
Setting hard_limit larger than previous swap_hard_limit must fail,
it's not that good if one wants to change the swap_hard_limit
and hard_limit together. E.g.
% virsh memtune rhel6
hard_limit : 1000000
soft_limit : 1000000
swap_hard_limit: 1000000
% virsh memtune rhel6 --hard-limit 1000020 --soft-limit 1000020 \
--swap-hard-limit 1000020 --live
This patch reoder the limits setting to set the swap_hard_limit
first, hard_limit then, and soft_limit last if it's greater than
current swap_hard_limit. And soft_limit first, hard_limit then,
swap_hard_limit last, if not.
---
src/qemu/qemu_driver.c | 59 +++++++++++++++++++++++++++++++++++++++++------
1 files changed, 51 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 80cfa84..a26d3cd 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6651,11 +6651,18 @@ qemuDomainSetMemoryParameters(virDomainPtr dom,
virDomainDefPtr persistentDef = NULL;
virCgroupPtr group = NULL;
virDomainObjPtr vm = NULL;
+ virTypedParameterPtr hard_limit = NULL;
+ virTypedParameterPtr soft_limit = NULL;
+ virTypedParameterPtr swap_hard_limit = NULL;
+ virTypedParameterPtr sorted_params[nparams];
You allocate the correct number of params here, but ...
@@ -6694,13 +6701,49 @@ qemuDomainSetMemoryParameters(virDomainPtr
dom,
}
}
+ for (i = 0; i < nparams; i++) {
+ if (STREQ(params[i].field, VIR_DOMAIN_MEMORY_HARD_LIMIT))
+ hard_limit = ¶ms[i];
+ else if (STREQ(params[i].field, VIR_DOMAIN_MEMORY_SOFT_LIMIT))
+ soft_limit = ¶ms[i];
+ else if (STREQ(params[i].field, VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT))
+ swap_hard_limit = ¶ms[i];
+ }
+
+ /* It will fail if hard limit greater than swap hard limit anyway */
+ if (swap_hard_limit &&
+ hard_limit &&
+ (hard_limit->value.ul > swap_hard_limit->value.ul)) {
+ virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("hard limit must be lower than swap hard limit"));
+ goto cleanup;
+ }
+
+ /* Get current swap hard limit */
+ rc = virCgroupGetMemSwapHardLimit(group, &val);
+ if (rc != 0) {
+ virReportSystemError(-rc, "%s",
+ _("unable to get swap hard limit"));
+ goto cleanup;
+ }
+
+ if (swap_hard_limit->value.ul > val) {
+ sorted_params[0] = swap_hard_limit;
+ sorted_params[1] = hard_limit;
+ sorted_params[2] = soft_limit;
+ } else {
+ sorted_params[0] = soft_limit;
+ sorted_params[1] = hard_limit;
+ sorted_params[2] = swap_hard_limit;
+ }
... you always access all 3 of them and you go through this list.
Unfortunately this means that in case you want to change only one value
(e.g. hard limit), you will dereference NULL couple of lines later and
it will crash libvirtd (tried). To test this, you can try running 'virsh
memtune rhel6 --hard-limit 1000020' in your previous scenario.
Martin.