On Tue, 12 Oct 2010 17:51:54 +0200, Daniel Veillard <veillard@redhat.com> wrote:
On Fri, Oct 08, 2010 at 05:45:55PM +0530, Nikunj A. Dadhania wrote:
From: Nikunj A. Dadhania <nikunj@linux.vnet.ibm.com>
V4: * prototype change: add unsigned int flags
Driver interface for getting memory parameters, eg. hard_limit, soft_limit and swap_hard_limit.
+ qemuReportError(VIR_ERR_INVALID_ARG, + "%s", _("Invalid parameter count")); + goto cleanup; + }
okay, this mean the application must always call with 0 first to get the exact value or this will break, fine but probably need to be made more clear from the description in libvirt.c .... TODO
+ if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot find cgroup for domain %s"), vm->def->name); + goto cleanup; + } + + for (i = 0; i < *nparams; i++) { + virMemoryParameterPtr param = ¶ms[i]; + val = 0; + param->value.ul = 0; + param->type = VIR_DOMAIN_MEMORY_FIELD_ULLONG; + + switch(i) { + case 0: /* fill memory hard limit here */ + rc = virCgroupGetMemoryHardLimit(group, &val); + if (rc != 0) { + virReportSystemError(-rc, "%s", + _("unable to get memory hard limit")); + continue; + } + if (virStrcpyStatic(param->field, VIR_DOMAIN_MEMORY_HARD_LIMIT) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Field memory hard limit too long for destination")); + continue; + } + param->value.ul = val; + break; + + case 1: /* fill memory soft limit here */ + rc = virCgroupGetMemorySoftLimit(group, &val); + if (rc != 0) { + virReportSystemError(-rc, "%s", + _("unable to get memory soft limit")); + continue; + } + if (virStrcpyStatic(param->field, VIR_DOMAIN_MEMORY_SOFT_LIMIT) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Field memory soft limit too long for destination")); + continue; + } + param->value.ul = val; + break; + + case 2: /* fill swap hard limit here */ + rc = virCgroupGetSwapHardLimit(group, &val); + if (rc != 0) { + virReportSystemError(-rc, "%s", + _("unable to get swap hard limit")); + continue; + } + if (virStrcpyStatic(param->field, VIR_DOMAIN_SWAP_HARD_LIMIT) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Field swap hard limit too long for destination")); + continue; + } + param->value.ul = val; + break; + + default: + break; + /* should not hit here */ + } + }
Okay, I'm not sure we actually need a loop here, but it may help refactoring... I guess this is related to my previous thinking, if nparams < QEMU_NB_MEM_PARAM, fill only till nparams and return. But with the change of
Sure, I will take care of updating the api desc in libvirt.c, I haven't used word always there. the logic, I think loop may not be required now.
I'm still having a problem with the code ignoring any error occuring in the loop, and fixing this in the same way. If there is an error the application *must* learn about it instead of trusting uninitialized memory as being data ! Maybe a memset is in order actually before entering that loop to avoid edge case problems... TODO too
By TODO you mean the error handling, right? I am taking care of setting the values to zero currently, and it does not tell the application whether to use this value or not. One option could be adding VIR_DOMAIN_MEMORY_INVALID in virMemoryParameterType and setting it in the beginning of the loop. Comments? Nikunj