Re: [libvirt] [PATCH v4 13/13] Remote protocol changes and implements virDomainSet/GetMemoryParameters

2010-10-13 Thread Daniel Veillard 
On Wed, Oct 13, 2010 at 11:12:49AM +0530, Nikunj A. Dadhania wrote:
 On Tue, 12 Oct 2010 19:27:05 +0200, Daniel Veillard veill...@redhat.com 
 wrote:
  On Fri, Oct 08, 2010 at 05:47:03PM +0530, Nikunj A. Dadhania wrote:
   From: Nikunj A. Dadhania nik...@linux.vnet.ibm.com
   
   v4:
   * prototype change: add unsigned int flags, regenerate files
   
   v3:
   * Squased all the remote driver changes to one single big patch and
 auto-generated is around 40%
   * Implements domainSetMemoryParameters and domainGetMemoryParameters for 
   remote
 driver
 daemon/remote.c
 src/remote/remote_driver.c
   
   * Auto generate the files using rpcgen and helper scripts in daemon/ 
   directory
 src/remote/remote_protocol.x
 daemon/remote_dispatch_args.h
 daemon/remote_dispatch_prototypes.h
 daemon/remote_dispatch_ret.h
 daemon/remote_dispatch_table.h
 src/remote/remote_protocol.c
 src/remote/remote_protocol.h
   
   Acked-by: Daniel P. Berrange berra...@redhat.com
   Signed-off-by: Nikunj A. Dadhania nik...@linux.vnet.ibm.com
  [...]
   +nparams = args-params.params_len;
   +nparams = args-flags;
  
obvious bug: flags = args-flags;
 Oops :) - CP
 
  
  That reminds me that I didn't see flags being checked against 0 on the
  QEmu and LXC drivers, we should check them !
  
 I will take care of this.


   virCheckFlags(0, -1);

as in qemuDomainGetBlockInfo() for example.

With that done, I think I can ACK the whole serie, and pushed it, but
  there is still a number of small TODOs for which I would appreciate
  patches,
  
thanks a lot !
  
 Thanks a lot to you and Daniel Berrange. Both of you have spend a lot of time
 on reviewing and on top of that fixing the patches as well.

  Well this would have been a bit simpler if you had used make syntax-check 
from the beginning, but most of the time spent was actually code review
and fixes, not those trivial issues :-)

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 13/13] Remote protocol changes and implements virDomainSet/GetMemoryParameters

2010-10-12 Thread Daniel P. Berrange 
On Tue, Oct 12, 2010 at 07:27:05PM +0200, Daniel Veillard wrote:
 On Fri, Oct 08, 2010 at 05:47:03PM +0530, Nikunj A. Dadhania wrote:
  From: Nikunj A. Dadhania nik...@linux.vnet.ibm.com
  
  v4:
  * prototype change: add unsigned int flags, regenerate files
  
  v3:
  * Squased all the remote driver changes to one single big patch and
auto-generated is around 40%
  * Implements domainSetMemoryParameters and domainGetMemoryParameters for 
  remote
driver
  daemon/remote.c
  src/remote/remote_driver.c
  
  * Auto generate the files using rpcgen and helper scripts in daemon/ 
  directory
  src/remote/remote_protocol.x
  daemon/remote_dispatch_args.h
  daemon/remote_dispatch_prototypes.h
  daemon/remote_dispatch_ret.h
  daemon/remote_dispatch_table.h
  src/remote/remote_protocol.c
  src/remote/remote_protocol.h
  
  Acked-by: Daniel P. Berrange berra...@redhat.com
  Signed-off-by: Nikunj A. Dadhania nik...@linux.vnet.ibm.com
 [...]
   static int
  +remoteDispatchDomainSetMemoryParameters (struct qemud_server *server 
  ATTRIBUTE_UNUSED,
  + struct qemud_client *client 
  ATTRIBUTE_UNUSED,
  + virConnectPtr conn,
  + remote_message_header *hdr 
  ATTRIBUTE_UNUSED,
  + remote_error *rerr,
  + 
  remote_domain_set_memory_parameters_args *args,
  + void *ret ATTRIBUTE_UNUSED)
  +{
  +virDomainPtr dom;
  +int i, r, nparams;
  +virMemoryParameterPtr params;
  +unsigned int flags;
  +
  +nparams = args-params.params_len;
  +nparams = args-flags;
 
   obvious bug: flags = args-flags;
 
 That reminds me that I didn't see flags being checked against 0 on the
 QEmu and LXC drivers, we should check them !
 
  +if (nparams  REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX) {
  +remoteDispatchFormatError (rerr, %s, _(nparams too large));
  +return -1;
 
   I did a lot of reformating and code cleanups in the non-generated
   files, to try to keep the source readable on a 80 columns editor.
 
  diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
  index 8af469c..f5dcb5c 100644
  --- a/src/remote/remote_protocol.x
  +++ b/src/remote/remote_protocol.x
  @@ -128,6 +128,9 @@ const REMOTE_NWFILTER_NAME_LIST_MAX = 1024;
   /* Upper limit on list of scheduler parameters. */
   const REMOTE_DOMAIN_SCHEDULER_PARAMETERS_MAX = 16;
   
  +/* Upper limit on list of memory parameters. */
  +const REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX = 16;
  +
 
   Hopefully we won't exhaust that crucial limit

That limit isn't ABI critical. It is simply to prevent DOS when
de-marshalling data, so we can raise it at will in the future.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London-o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org-o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v4 13/13] Remote protocol changes and implements virDomainSet/GetMemoryParameters

2010-10-12 Thread Nikunj A. Dadhania 
On Tue, 12 Oct 2010 19:27:05 +0200, Daniel Veillard veill...@redhat.com wrote:
 On Fri, Oct 08, 2010 at 05:47:03PM +0530, Nikunj A. Dadhania wrote:
  From: Nikunj A. Dadhania nik...@linux.vnet.ibm.com
  
  v4:
  * prototype change: add unsigned int flags, regenerate files
  
  v3:
  * Squased all the remote driver changes to one single big patch and
auto-generated is around 40%
  * Implements domainSetMemoryParameters and domainGetMemoryParameters for 
  remote
driver
  daemon/remote.c
  src/remote/remote_driver.c
  
  * Auto generate the files using rpcgen and helper scripts in daemon/ 
  directory
  src/remote/remote_protocol.x
  daemon/remote_dispatch_args.h
  daemon/remote_dispatch_prototypes.h
  daemon/remote_dispatch_ret.h
  daemon/remote_dispatch_table.h
  src/remote/remote_protocol.c
  src/remote/remote_protocol.h
  
  Acked-by: Daniel P. Berrange berra...@redhat.com
  Signed-off-by: Nikunj A. Dadhania nik...@linux.vnet.ibm.com
 [...]
  +nparams = args-params.params_len;
  +nparams = args-flags;
 
   obvious bug: flags = args-flags;
Oops :) - CP

 
 That reminds me that I didn't see flags being checked against 0 on the
 QEmu and LXC drivers, we should check them !
 
I will take care of this.

  +if (nparams  REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX) {
  +remoteDispatchFormatError (rerr, %s, _(nparams too large));
  +return -1;
 
   I did a lot of reformating and code cleanups in the non-generated
   files, to try to keep the source readable on a 80 columns editor.

Thanks,
 
   With that done, I think I can ACK the whole serie, and pushed it, but
 there is still a number of small TODOs for which I would appreciate
 patches,
 
   thanks a lot !
 
Thanks a lot to you and Daniel Berrange. Both of you have spend a lot of time
on reviewing and on top of that fixing the patches as well.

Regards,
Nikunj

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v4 13/13] Remote protocol changes and implements virDomainSet/GetMemoryParameters

2010-10-08 Thread Nikunj A. Dadhania 
From: Nikunj A. Dadhania nik...@linux.vnet.ibm.com

v4:
* prototype change: add unsigned int flags, regenerate files

v3:
* Squased all the remote driver changes to one single big patch and
  auto-generated is around 40%
* Implements domainSetMemoryParameters and domainGetMemoryParameters for remote
  driver
daemon/remote.c
src/remote/remote_driver.c

* Auto generate the files using rpcgen and helper scripts in daemon/ directory
src/remote/remote_protocol.x
daemon/remote_dispatch_args.h
daemon/remote_dispatch_prototypes.h
daemon/remote_dispatch_ret.h
daemon/remote_dispatch_table.h
src/remote/remote_protocol.c
src/remote/remote_protocol.h

Acked-by: Daniel P. Berrange berra...@redhat.com
Signed-off-by: Nikunj A. Dadhania nik...@linux.vnet.ibm.com
---
 daemon/remote.c |  162 +++
 daemon/remote_dispatch_args.h   |2 
 daemon/remote_dispatch_prototypes.h |   16 +++
 daemon/remote_dispatch_ret.h|1 
 daemon/remote_dispatch_table.h  |   10 ++
 src/remote/remote_driver.c  |  153 +
 src/remote/remote_protocol.c|   88 +++
 src/remote/remote_protocol.h|   58 +
 src/remote/remote_protocol.x|   44 +-
 9 files changed, 531 insertions(+), 3 deletions(-)

diff --git a/daemon/remote.c b/daemon/remote.c
index 6b67678..d5a8a9c 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -2332,6 +2332,168 @@ remoteDispatchDomainSetMemory (struct qemud_server 
*server ATTRIBUTE_UNUSED,
 }
 
 static int
+remoteDispatchDomainSetMemoryParameters (struct qemud_server *server 
ATTRIBUTE_UNUSED,
+ struct qemud_client *client 
ATTRIBUTE_UNUSED,
+ virConnectPtr conn,
+ remote_message_header *hdr 
ATTRIBUTE_UNUSED,
+ remote_error *rerr,
+ 
remote_domain_set_memory_parameters_args *args,
+ void *ret ATTRIBUTE_UNUSED)
+{
+virDomainPtr dom;
+int i, r, nparams;
+virMemoryParameterPtr params;
+unsigned int flags;
+
+nparams = args-params.params_len;
+nparams = args-flags;
+
+if (nparams  REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX) {
+remoteDispatchFormatError (rerr, %s, _(nparams too large));
+return -1;
+}
+if (VIR_ALLOC_N(params, nparams)  0) {
+remoteDispatchOOMError(rerr);
+return -1;
+}
+
+/* Deserialise parameters. */
+for (i = 0; i  nparams; ++i) {
+if (virStrcpyStatic(params[i].field, args-params.params_val[i].field) 
== NULL) {
+remoteDispatchFormatError(rerr, _(Field %s too big for 
destination),
+  args-params.params_val[i].field);
+return -1;
+}
+params[i].type = args-params.params_val[i].value.type;
+switch (params[i].type) {
+case VIR_DOMAIN_MEMORY_FIELD_INT:
+params[i].value.i = 
args-params.params_val[i].value.remote_memory_param_value_u.i; break;
+case VIR_DOMAIN_MEMORY_FIELD_UINT:
+params[i].value.ui = 
args-params.params_val[i].value.remote_memory_param_value_u.ui; break;
+case VIR_DOMAIN_MEMORY_FIELD_LLONG:
+params[i].value.l = 
args-params.params_val[i].value.remote_memory_param_value_u.l; break;
+case VIR_DOMAIN_MEMORY_FIELD_ULLONG:
+params[i].value.ul = 
args-params.params_val[i].value.remote_memory_param_value_u.ul; break;
+case VIR_DOMAIN_MEMORY_FIELD_DOUBLE:
+params[i].value.d = 
args-params.params_val[i].value.remote_memory_param_value_u.d; break;
+case VIR_DOMAIN_MEMORY_FIELD_BOOLEAN:
+params[i].value.b = 
args-params.params_val[i].value.remote_memory_param_value_u.b; break;
+}
+}
+
+dom = get_nonnull_domain (conn, args-dom);
+if (dom == NULL) {
+VIR_FREE(params);
+remoteDispatchConnError(rerr, conn);
+return -1;
+}
+
+r = virDomainSetMemoryParameters (dom, params, nparams, flags);
+virDomainFree(dom);
+VIR_FREE(params);
+if (r == -1) {
+remoteDispatchConnError(rerr, conn);
+return -1;
+}
+
+return 0;
+}
+
+static int remoteDispatchDomainGetMemoryParameters (struct qemud_server 
*server,
+struct qemud_client 
*client,
+virConnectPtr conn,
+remote_message_header *hdr,
+remote_error *rerr,
+
remote_domain_get_memory_parameters_args *args,
+