On Tue, Jul 17, 2012 at 11:14:43PM -0500, Doug Goldstein wrote:
> On Tue, Jul 17, 2012 at 11:45 AM, Michal Privoznik <mprivozn@xxxxxxxxxx> wrote:
> > If there's a memory leak in qemu or qemu is exploited the host's
> > system will sooner or later start trashing instead of killing
> > the bad process. This however has impact on performance and other
> > guests as well. Therefore we should set a reasonable RSS limit
> > even when user hasn't set any. It's better to be secure by default.
> > ---
> > src/qemu/qemu_cgroup.c | 73 +++++++++++++++++++++++++++---------------------
> > 1 files changed, 41 insertions(+), 32 deletions(-)
> >
> > diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
> > index e39f5e1..2c158c1 100644
> > --- a/src/qemu/qemu_cgroup.c
> > +++ b/src/qemu/qemu_cgroup.c
> > @@ -339,42 +339,51 @@ int qemuSetupCgroup(struct qemud_driver *driver,
> > }
> > }
> >
> > - if (vm->def->mem.hard_limit != 0 ||
> > - vm->def->mem.soft_limit != 0 ||
> > - vm->def->mem.swap_hard_limit != 0) {
> > - if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_MEMORY)) {
> > - if (vm->def->mem.hard_limit != 0) {
> > - rc = virCgroupSetMemoryHardLimit(cgroup, vm->def->mem.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(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 (qemuCgroupControllerActive(driver, 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 trashing caused by exploited qemu.
> > + * As 'reasonable limit' has been chosen:
> > + * (1 + k) * (domain memory) + F
> > + * where k = 0.02 and F = 200MB. */
> > + hard_limit = vm->def->mem.max_balloon * 1.02 + 204800;
> > + }
> > +
> > + rc = virCgroupSetMemoryHardLimit(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(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(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;
> > - }
> > + if (vm->def->mem.swap_hard_limit != 0) {
> > + rc = virCgroupSetMemSwapHardLimit(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 {
> > - qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > - _("Memory cgroup is not available on this host"));
> > }
> > + } else if (vm->def->mem.hard_limit != 0 ||
> > + vm->def->mem.soft_limit != 0 ||
> > + vm->def->mem.swap_hard_limit != 0) {
> > + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > + _("Memory cgroup is not available on this host"));
> > + } else {
> > + VIR_WARN("Could not autoset a RSS limit for domain %s", vm->def->name);
> > }
> >
> > if (vm->def->cputune.shares != 0) {
> > --
> > 1.7.8.6
> >
>
> Looks like a good change. My only question would be if its better to
> look up the guest video RAM instead of assuming that QEMU overhead +
> guest video RAM will amount to no more than about 200MB (I say about
> because of the 1.02 fudge factor). If its not really possible to grab
> that or it just makes sense to go with a set value like this then ACK
> from me.
Yeah, since we go to the trouble of keeping video RAM in the XML, I
say we ought to use it, instead of the 200MB fudge factor.
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