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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list