[libvirt] [PATCH]: fix ruby-libvirt bindings when virConnectNum* returns 0
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 <clalance@redhat.com>
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/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
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/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
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
On Tue, 2008-06-17 at 11:16 +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
Signed-off-by: Chris Lalancette <clalance@redhat.com>
I committed the patch with a couple more places to shortcircuit added. Making a release and pushing an update will likely have to wait till next week. David
participants (3)
-
Chris Lalancette -
Daniel Veillard -
David Lutterkort