On Fri, May 17, 2013 at 07:59:32PM +0800, Osier Yang wrote:
--- src/qemu/qemu_cgroup.c | 120 ++++++++++++++++++++++++++++--------------------- 1 file changed, 68 insertions(+), 52 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 0c4792e..2751be0 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -455,6 +455,72 @@ qemuSetupBlkioCgroup(virDomainObjPtr vm) return 0; }
My email client is showing trailing whitespace here. Not sure if that is genuine or not. Also 2 blank lines between functions
+static int +qemuSetupMemoryCgroup(virDomainObjPtr vm) {
Put the '{' on a new line as you did with previous patches
+ qemuDomainObjPrivatePtr priv = vm->privateData; + unsigned long long hard_limit; + int rc; + int i; + + if (!virCgroupHasController(priv->cgroup,VIR_CGROUP_CONTROLLER_MEMORY)) { + if (vm->def->mem.hard_limit != 0 || + vm->def->mem.soft_limit != 0 || + vm->def->mem.swap_hard_limit != 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Memory cgroup is not available on this host")); + return -1; + } else { + VIR_WARN("Could not autoset a RSS limit for domain %s", vm->def->name); + return 0; + }
Not sure why we need this VIR_WARN at all. If no limits are set in the XML, then we should not warn about a missing feature that we don't actually need.
@@ -668,58 +734,8 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, if (qemuSetupBlkioCgroup(vm) < 0) goto cleanup;
- if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_MEMORY)) { - unsigned long long hard_limit = vm->def->mem.hard_limit; - - if (!hard_limit) { - /* If there is no hard_limit set, set a reasonable one to avoid - * system thrashing caused by exploited qemu. A 'reasonable - * limit' has been chosen: - * (1 + k) * (domain memory + total video memory) + (32MB for - * cache per each disk) + F - * where k = 0.5 and F = 200MB. The cache for disks is important as - * kernel cache on the host side counts into the RSS limit. */ - hard_limit = vm->def->mem.max_balloon; - for (i = 0; i < vm->def->nvideos; i++) - hard_limit += vm->def->videos[i]->vram; - hard_limit = hard_limit * 1.5 + 204800; - hard_limit += vm->def->ndisks * 32768; - } - - rc = virCgroupSetMemoryHardLimit(priv->cgroup, hard_limit); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to set memory hard limit for domain %s"), - vm->def->name); - goto cleanup; - } - if (vm->def->mem.soft_limit != 0) { - rc = virCgroupSetMemorySoftLimit(priv->cgroup, vm->def->mem.soft_limit); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to set memory soft limit for domain %s"), - vm->def->name); - goto cleanup; - } - } - - if (vm->def->mem.swap_hard_limit != 0) { - rc = virCgroupSetMemSwapHardLimit(priv->cgroup, vm->def->mem.swap_hard_limit); - if (rc != 0) { - virReportSystemError(-rc, - _("Unable to set swap hard limit for domain %s"), - vm->def->name); - goto cleanup; - } - } - } else if (vm->def->mem.hard_limit != 0 || - vm->def->mem.soft_limit != 0 || - vm->def->mem.swap_hard_limit != 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Memory cgroup is not available on this host")); - } else { - VIR_WARN("Could not autoset a RSS limit for domain %s", vm->def->name); - } + if (qemuSetupMemoryCgroup(vm) < 0) + goto cleanup;
ACK with warning removed Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|