Re: [libvirt] [PATCH]: fix ruby-libvirt bindings when virConnectNum* returns 0

2008-06-19 Thread Chris Lalancette
Daniel Veillard wrote:
   Hum, looking at the bugzilla it's for storage that the problem was raised.
 It's a bit annoying, the main domain and network functions accept a max = 0
 The documentation of virConnectListDefinedStoragePools doesn't prevent
 maxnames = 0
 
   Actually the check done in the function is 
 if ((names == NULL) || (maxnames  0))
 
 i.e. it allows 0, and pass it to the underlying driver. One solution
 would be to just return 0 there, as attached in this patch, the other
 solution would be to check which underlying storage driver failed and
 fix it,

Right.  After going through this, I found out that it is actually a NULL check
firing, not a 0 check (see my posted patch for details).  That being said, a
patch of the type posted here might be worthwhile; it would short-circuit the
rest of the calls through the stack, and in particular would avoid on-the-wire
calls in the remote case, to do essentially no work.  I think it would be a nice
optimization for all of the List type operations, but not necessarily required.

Chris Lalancette

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


[libvirt] [PATCH]: fix ruby-libvirt bindings when virConnectNum* returns 0

2008-06-17 Thread Chris Lalancette
Attached is a simple patch to fix a problem I ran into when using the
ruby-libvirt bindings with libvirt 0.4.3.  Basically, if you call any of the
virConnectList* functions with a max of 0, it returns Invalid Arg.  To get
around this, modify the ruby-libvirt bindings to return an empty list if we get
num == 0 when calling the corresponding virConnectNumOf* function.  This should
solve: https://bugzilla.redhat.com/show_bug.cgi?id=451666

Signed-off-by: Chris Lalancette [EMAIL PROTECTED]
diff -r b02171cfad7d ext/libvirt/_libvirt.c
--- a/ext/libvirt/_libvirt.c	Wed Apr 16 09:47:27 2008 -0700
+++ b/ext/libvirt/_libvirt.c	Tue Jun 17 11:06:26 2008 +0200
@@ -256,7 +256,11 @@ static VALUE vol_new(virStorageVolPtr n,
 \
 num = virConnectNumOf##objs(conn);  \
 _E(num  0, create_error(e_RetrieveError, virConnectNumOf # objs, , conn));   \
-\
+if (num == 0) { \
+/* if num is 0, don't call virConnectList* function */  \
+result = rb_ary_new2(num);  \
+return result;  \
+}   \
 names = ALLOC_N(char *, num);   \
 r = virConnectList##objs(conn, names, num); \
 if (r  0) {\
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH]: fix ruby-libvirt bindings when virConnectNum* returns 0

2008-06-17 Thread Daniel Veillard
On Tue, Jun 17, 2008 at 11:16:31AM +0200, Chris Lalancette wrote:
 Attached is a simple patch to fix a problem I ran into when using the
 ruby-libvirt bindings with libvirt 0.4.3.  Basically, if you call any of the
 virConnectList* functions with a max of 0, it returns Invalid Arg.  To get
 around this, modify the ruby-libvirt bindings to return an empty list if we 
 get
 num == 0 when calling the corresponding virConnectNumOf* function.  This 
 should
 solve: https://bugzilla.redhat.com/show_bug.cgi?id=451666

  That doesn't sound proper to me. I re-checked, all virConnectList* functions
in libvirt.c raise with an error only if the connection is invalid, if 
the pointer to the storage is NULL or if the max value is  0. the max value
being 0 should be accepted, that's the semantic of the check done on top
of the driver. So what driver did fail with an argument error ? this need
to be fixed.
  This looks okay in the QEmu driver from what I saw
  xenHypervisorListDomains seems to need fixing.
  xenDaemonListDomains need to be fixed too.
  xenDaemonListDefinedDomains need to be fixed too.
  The OpenVZ driver looks okay
  The LXC driver looks okay
  The remote driver seems okay but it's harder for me to assert it works 
  through the full chain.

Patch for the Xen parts attached.
Could you try to investigate and find more precisely under what conditions
you got the 'max = 0' returning an invalid arg error, we need to find this
fix it not work around it at the binding level IMHO,

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard  | virtualization library  http://libvirt.org/
[EMAIL PROTECTED]  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/
Index: src/xen_internal.c
===
RCS file: /data/cvs/libxen/src/xen_internal.c,v
retrieving revision 1.121
diff -u -p -r1.121 xen_internal.c
--- src/xen_internal.c  6 Jun 2008 11:09:57 -   1.121
+++ src/xen_internal.c  17 Jun 2008 09:48:17 -
@@ -2554,9 +2554,12 @@ xenHypervisorListDomains(virConnectPtr c
 
 priv = (xenUnifiedPrivatePtr) conn-privateData;
 if (priv-handle  0 ||
-(ids == NULL) || (maxids  1))
+(ids == NULL) || (maxids  0))
 return (-1);
 
+if (maxids == 0)
+return(0);
+
 if (!(XEN_GETDOMAININFOLIST_ALLOC(dominfos, maxids))) {
 virXenError(conn, VIR_ERR_NO_MEMORY, allocating %d domain info,
 maxids);
Index: src/xend_internal.c
===
RCS file: /data/cvs/libxen/src/xend_internal.c,v
retrieving revision 1.198
diff -u -p -r1.198 xend_internal.c
--- src/xend_internal.c 9 Jun 2008 12:16:03 -   1.198
+++ src/xend_internal.c 17 Jun 2008 09:48:17 -
@@ -3330,7 +3330,10 @@ xenDaemonListDomains(virConnectPtr conn,
 struct sexpr *_for_i, *node;
 long id;
 
-if ((ids == NULL) || (maxids = 0))
+if (maxids == 0)
+return(0);
+
+if ((ids == NULL) || (maxids  0))
 goto error;
 root = sexpr_get(conn, /xend/domain);
 if (root == NULL)
@@ -4219,8 +4222,11 @@ int xenDaemonListDefinedDomains(virConne
 if (priv-xendConfigVersion  3)
 return(-1);
 
-if ((names == NULL) || (maxnames = 0))
+if ((names == NULL) || (maxnames  0))
 goto error;
+if (maxnames == 0)
+return(0);
+
 root = sexpr_get(conn, /xend/domain?state=halted);
 if (root == NULL)
 goto error;
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH]: fix ruby-libvirt bindings when virConnectNum* returns 0

2008-06-17 Thread Daniel Veillard
On Tue, Jun 17, 2008 at 11:16:31AM +0200, Chris Lalancette wrote:
 Attached is a simple patch to fix a problem I ran into when using the
 ruby-libvirt bindings with libvirt 0.4.3.  Basically, if you call any of the
 virConnectList* functions with a max of 0, it returns Invalid Arg.  To get
 around this, modify the ruby-libvirt bindings to return an empty list if we 
 get
 num == 0 when calling the corresponding virConnectNumOf* function.  This 
 should
 solve: https://bugzilla.redhat.com/show_bug.cgi?id=451666

  Hum, looking at the bugzilla it's for storage that the problem was raised.
It's a bit annoying, the main domain and network functions accept a max = 0
The documentation of virConnectListDefinedStoragePools doesn't prevent
maxnames = 0

  Actually the check done in the function is 
if ((names == NULL) || (maxnames  0))

i.e. it allows 0, and pass it to the underlying driver. One solution
would be to just return 0 there, as attached in this patch, the other
solution would be to check which underlying storage driver failed and
fix it,

Daniel


-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard  | virtualization library  http://libvirt.org/
[EMAIL PROTECTED]  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/
Index: src/libvirt.c
===
RCS file: /data/cvs/libxen/src/libvirt.c,v
retrieving revision 1.146
diff -u -p -r1.146 libvirt.c
--- src/libvirt.c   10 Jun 2008 10:43:28 -  1.146
+++ src/libvirt.c   17 Jun 2008 09:59:00 -
@@ -4023,6 +4023,9 @@ virConnectListStoragePools(virConnectPt
 return (-1);
 }
 
+if (maxnames == 0)
+return (0);
+
 if (conn-storageDriver  conn-storageDriver-listPools)
 return conn-storageDriver-listPools (conn, names, maxnames);
 
@@ -4087,6 +4090,9 @@ virConnectListDefinedStoragePools(virCon
 return (-1);
 }
 
+if (maxnames == 0)
+return(0);
+
 if (conn-storageDriver  conn-storageDriver-listDefinedPools)
 return conn-storageDriver-listDefinedPools (conn, names, maxnames);
 
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list