[libvirt] [PATCH] qemuSetupMemoryCgroup: Handle hard_limit properly

Since 16bcb3 we have a regression. The hard_limit is set unconditionally. By default, the limit is zero. Hence, if user hasn't configured any, we set the zero in cgroup subsystem making the kernel kill the corresponding qemu process immediately. The proper fix is to set hard_limit iff user has configure any. --- src/qemu/qemu_cgroup.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 9673e8e..e27945e 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -428,7 +428,8 @@ qemuSetupMemoryCgroup(virDomainObjPtr vm) } } - if (virCgroupSetMemoryHardLimit(priv->cgroup, vm->def->mem.hard_limit) < 0) + if (vm->def->mem.hard_limit != 0 && + virCgroupSetMemoryHardLimit(priv->cgroup, vm->def->mem.hard_limit) < 0) return -1; if (vm->def->mem.soft_limit != 0 && -- 1.8.1.5

On 20/08/13 17:10, Michal Privoznik wrote:
Since 16bcb3 we have a regression. The hard_limit is set unconditionally. By default, the limit is zero. Hence, if user hasn't configured any, we set the zero in cgroup subsystem making the kernel kill the corresponding qemu process immediately. The proper fix is to set hard_limit iff user has configure any.
s/iff/if/,
--- src/qemu/qemu_cgroup.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 9673e8e..e27945e 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -428,7 +428,8 @@ qemuSetupMemoryCgroup(virDomainObjPtr vm) } }
- if (virCgroupSetMemoryHardLimit(priv->cgroup, vm->def->mem.hard_limit) < 0) + if (vm->def->mem.hard_limit != 0 && + virCgroupSetMemoryHardLimit(priv->cgroup, vm->def->mem.hard_limit) < 0) return -1;
if (vm->def->mem.soft_limit != 0 &&
ACK

On 08/20/13 12:08, Osier Yang wrote:
On 20/08/13 17:10, Michal Privoznik wrote:
Since 16bcb3 we have a regression. The hard_limit is set unconditionally. By default, the limit is zero. Hence, if user hasn't
s/default,/default/
configured any, we set the zero in cgroup subsystem making the kernel kill the corresponding qemu process immediately. The proper fix is to set hard_limit iff user has configure any.
s/iff/if/,
iff means 'if and only if': http://en.wikipedia.org/wiki/If_and_only_if A typo is in s/configure/configured/ . Peter

On 08/20/2013 11:10 AM, Michal Privoznik wrote:
Since 16bcb3 we have a regression. The hard_limit is set unconditionally. By default, the limit is zero. Hence, if user hasn't configured any, we set the zero in cgroup subsystem making the kernel kill the corresponding qemu process immediately. The proper fix is to set hard_limit iff user has configure any. --- src/qemu/qemu_cgroup.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 9673e8e..e27945e 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -428,7 +428,8 @@ qemuSetupMemoryCgroup(virDomainObjPtr vm) } }
- if (virCgroupSetMemoryHardLimit(priv->cgroup, vm->def->mem.hard_limit) < 0) + if (vm->def->mem.hard_limit != 0 && + virCgroupSetMemoryHardLimit(priv->cgroup, vm->def->mem.hard_limit) < 0) return -1;
if (vm->def->mem.soft_limit != 0 &&
This still calls SetMaxMemLock with 0 if mem.hard_limit is 0. According to commit 9395894 [1], it seems this won't be enough for VFIO passthrough (although I haven't tested it). (Commit 6d8ebc7 [2] changed this to use qemuDomainMemoryLimit) Jan [1] http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=9395894 [2] http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=6d8ebc7

On 20.08.2013 14:11, Ján Tomko wrote:
On 08/20/2013 11:10 AM, Michal Privoznik wrote:
Since 16bcb3 we have a regression. The hard_limit is set unconditionally. By default, the limit is zero. Hence, if user hasn't configured any, we set the zero in cgroup subsystem making the kernel kill the corresponding qemu process immediately. The proper fix is to set hard_limit iff user has configure any. --- src/qemu/qemu_cgroup.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 9673e8e..e27945e 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -428,7 +428,8 @@ qemuSetupMemoryCgroup(virDomainObjPtr vm) } }
- if (virCgroupSetMemoryHardLimit(priv->cgroup, vm->def->mem.hard_limit) < 0) + if (vm->def->mem.hard_limit != 0 && + virCgroupSetMemoryHardLimit(priv->cgroup, vm->def->mem.hard_limit) < 0) return -1;
if (vm->def->mem.soft_limit != 0 &&
This still calls SetMaxMemLock with 0 if mem.hard_limit is 0. According to commit 9395894 [1], it seems this won't be enough for VFIO passthrough (although I haven't tested it). (Commit 6d8ebc7 [2] changed this to use qemuDomainMemoryLimit)
Jan
[1] http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=9395894 [2] http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=6d8ebc7
I've posted a separate patch for that. So if you agree, can I push this one? Michal

On 08/20/2013 03:09 PM, Michal Privoznik wrote:
On 20.08.2013 14:11, Ján Tomko wrote:
On 08/20/2013 11:10 AM, Michal Privoznik wrote:
Since 16bcb3 we have a regression. The hard_limit is set unconditionally. By default, the limit is zero. Hence, if user hasn't configured any, we set the zero in cgroup subsystem making the kernel kill the corresponding qemu process immediately. The proper fix is to set hard_limit iff user has configure any. --- ...
This still calls SetMaxMemLock with 0 if mem.hard_limit is 0. According to commit 9395894 [1], it seems this won't be enough for VFIO passthrough (although I haven't tested it). (Commit 6d8ebc7 [2] changed this to use qemuDomainMemoryLimit)
Jan
[1] http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=9395894 [2] http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=6d8ebc7
I've posted a separate patch for that. So if you agree, can I push this one?
Sure, go ahead. Jan
participants (4)
-
Ján Tomko
-
Michal Privoznik
-
Osier Yang
-
Peter Krempa