[libvirt] [PATCH 2/7] qemu: Abstract code for memory controller setting into a helper
Osier Yang
jyang at redhat.com
Mon May 20 11:35:01 UTC 2013
On 20/05/13 19:08, Daniel P. Berrange wrote:
> 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
I found no trailing spaces.
>
>> +static int
>> +qemuSetupMemoryCgroup(virDomainObjPtr vm) {
> Put the '{' on a new line as you did with previous patches
Okay.
>
>> + 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.
Agreed. Having a warning for no XML config is confused. I removed it.
>
>
>> @@ -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
>
>
Pushed. Thanks.
Osier
More information about the libvir-list
mailing list