On Fri, Oct 08, 2010 at 05:46:28PM +0530, Nikunj A. Dadhania wrote: > From: Nikunj A. Dadhania <nikunj@xxxxxxxxxxxxxxxxxx> > > 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@xxxxxxxxxx> > Signed-off-by: Nikunj A. Dadhania <nikunj@xxxxxxxxxxxxxxxxxx> [...] > +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@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list