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

2010-10-13 Thread Daniel Veillard 
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

2010-10-12 Thread Daniel Veillard 
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

2010-10-12 Thread Nikunj A. Dadhania 
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

2010-10-08 Thread Nikunj A. Dadhania 
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