Re: [libvirt] [PATCH v4 10/13] Implement driver interface domainSetMemoryParamters for LXC
On Wed, Oct 13, 2010 at 11:07:47AM +0530, Nikunj A. Dadhania wrote: On Tue, 12 Oct 2010 18:32:19 +0200, Daniel Veillard veill...@redhat.com wrote: On Fri, Oct 08, 2010 at 05:46:28PM +0530, Nikunj A. Dadhania wrote: From: Nikunj A. Dadhania nik...@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 berra...@redhat.com Signed-off-by: Nikunj A. Dadhania nik...@linux.vnet.ibm.com [...] +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. Let me look at this and I will provide a patch. Probably too complex and outside the scope of this patch, I would rather prefer if you focused on the documentation patch(es) still needed at this point, Same problem of error reporting as in the QEmu driver, I moved ret = 0; before the loop and et ret = -1; on all errors ! One clarification: Will it return error back immediately if an error occurs? No Or will it try setting all of them one by one and if anyone of them succeed, success is returned. Check the code from git ! It will try all of them. If any of them fails it will return an error. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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 nik...@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 berra...@redhat.com Signed-off-by: Nikunj A. Dadhania nik...@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 = 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, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 10/13] Implement driver interface domainSetMemoryParamters for LXC
On Tue, 12 Oct 2010 18:32:19 +0200, Daniel Veillard veill...@redhat.com wrote: On Fri, Oct 08, 2010 at 05:46:28PM +0530, Nikunj A. Dadhania wrote: From: Nikunj A. Dadhania nik...@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 berra...@redhat.com Signed-off-by: Nikunj A. Dadhania nik...@linux.vnet.ibm.com [...] +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. Let me look at this and I will provide a patch. +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 ! One clarification: Will it return error back immediately if an error occurs? Or will it try setting all of them one by one and if anyone of them succeed, success is returned. +cleanup: +if (cgroup) +virCgroupFree(cgroup); +if (vm) +virDomainObjUnlock(vm); +lxcDriverUnlock(driver); +return ret; +} + After those modifications, ACK, applied to my tree, Thanks Nikunj -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 10/13] Implement driver interface domainSetMemoryParamters for LXC
From: Nikunj A. Dadhania nik...@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 berra...@redhat.com Signed-off-by: Nikunj A. Dadhania nik...@linux.vnet.ibm.com --- src/lxc/lxc_driver.c | 91 +- 1 files changed, 90 insertions(+), 1 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 8977835..984a5fa 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -677,6 +677,95 @@ cleanup: return ret; } +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; +} + +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; +} + +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; + +cleanup: +if (cgroup) +virCgroupFree(cgroup); +if (vm) +virDomainObjUnlock(vm); +lxcDriverUnlock(driver); +return ret; +} + static char *lxcDomainDumpXML(virDomainPtr dom, int flags) { @@ -2620,7 +2709,7 @@ static virDriver lxcDriver = { NULL, /* domainRevertToSnapshot */ NULL, /* domainSnapshotDelete */ NULL, /* qemuDomainMonitorCommand */ -NULL, /* domainSetMemoryParameters */ +lxcDomainSetMemoryParameters, /* domainSetMemoryParameters */ NULL, /* domainGetMemoryParameters */ }; -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list