Re: [libvirt] [PATCH v4 10/13] Implement driver interface domainSetMemoryParamters for LXC

On Fri, Oct 08, 2010 at 05:46:28PM +0530, Nikunj A. Dadhania wrote: > From: Nikunj A. Dadhania <nikunj(a)linux.vnet.ibm.com&gt; > > 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(a)redhat.com&gt; > Signed-off-by: Nikunj A. Dadhania <nikunj(a)linux.vnet.ibm.com&gt;

> + 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.

> + for (i = 0; i < nparams; i++) { > + virMemoryParameterPtr param = &params[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,