On Fri, Oct 08, 2010 at 05:46:28PM +0530, Nikunj A. Dadhania wrote:
From: Nikunj A. Dadhania <nikunj@linux.vnet.ibm.com>
Add support in the lxc driver for various memory controllable parameters
v4: + prototype change: add unsigned int flags
v2: + Use #define string constants for "hard_limit", etc + fix typo: min_guarantee
Acked-by: "Daniel P. Berrange" <berrange@redhat.com> Signed-off-by: Nikunj A. Dadhania <nikunj@linux.vnet.ibm.com> [...] +static int lxcDomainSetMemoryParameters(virDomainPtr dom, + virMemoryParameterPtr params, + int nparams, + unsigned int flags ATTRIBUTE_UNUSED) +{ + lxc_driver_t *driver = dom->conn->privateData; + int i; + virCgroupPtr cgroup = NULL; + virDomainObjPtr vm = NULL; + int ret = -1; + + lxcDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + + if (vm == NULL) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + lxcError(VIR_ERR_NO_DOMAIN, + _("No domain with matching uuid '%s'"), uuidstr); + goto cleanup; + }
Hum, the qemu driver was reporting if (vm == NULL) { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("No such domain %s"), dom->uuid); goto cleanup; } the 2 should be harmonized I guess, but since the LXC reporting is better I left this as a TODO, seems that's a more general cleanup needed between drivers.
+ if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) != 0) { + lxcError(VIR_ERR_INTERNAL_ERROR, + _("Unable to get cgroup for %s"), vm->def->name); + goto cleanup; + }
And QEmu reported here: qemuReportError(VIR_ERR_INTERNAL_ERROR, _("cannot find cgroup for domain %s"), vm->def->name); goto cleanup; why diverging strings ? ... I used the same string instead !
+ for (i = 0; i < nparams; i++) { + virMemoryParameterPtr param = ¶ms[i]; + + if (STREQ(param->field, VIR_DOMAIN_MEMORY_HARD_LIMIT)) { + int rc; + if (param->type != VIR_DOMAIN_MEMORY_FIELD_ULLONG) { + lxcError(VIR_ERR_INVALID_ARG, "%s", + _("invalid type for memory hard_limit tunable, expected a 'ullong'")); + continue; + } + + rc = virCgroupSetMemoryHardLimit(cgroup, params[i].value.ul); + if (rc != 0) { + virReportSystemError(-rc, "%s", + _("unable to set memory hard_limit tunable")); + } + } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_SOFT_LIMIT)) { + int rc; + if (param->type != VIR_DOMAIN_MEMORY_FIELD_ULLONG) { + lxcError(VIR_ERR_INVALID_ARG, "%s", + _("invalid type for memory soft_limit tunable, expected a 'ullong'")); + continue; + } + + rc = virCgroupSetMemorySoftLimit(cgroup, params[i].value.ul); + if (rc != 0) { + virReportSystemError(-rc, "%s", + _("unable to set memory soft_limit tunable")); + } + } else if (STREQ(param->field, VIR_DOMAIN_SWAP_HARD_LIMIT)) { + int rc; + if (param->type != VIR_DOMAIN_MEMORY_FIELD_ULLONG) { + lxcError(VIR_ERR_INVALID_ARG, "%s", + _("invalid type for swap_hard_limit tunable, expected a 'ullong'")); + continue; + } + + rc = virCgroupSetSwapHardLimit(cgroup, params[i].value.ul); + if (rc != 0) { + virReportSystemError(-rc, "%s", + _("unable to set swap_hard_limit tunable")); + } + } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_MIN_GUARANTEE)) { + lxcError(VIR_ERR_INVALID_ARG, + _("Memory tunable `%s' not implemented"), param->field); + } else { + lxcError(VIR_ERR_INVALID_ARG, + _("Parameter `%s' not supported"), param->field); + } + } + ret = 0;
Same problem of error reporting as in the QEmu driver, I moved ret = 0; before the loop and et ret = -1; on all errors !
+cleanup: + if (cgroup) + virCgroupFree(&cgroup); + if (vm) + virDomainObjUnlock(vm); + lxcDriverUnlock(driver); + return ret; +} +
After those modifications, ACK, applied to my tree, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/