[libvirt] [PATCH] Avoid virGetVersion failure on specific driver support configurations

virGetVersion itself doesn't take a virConnectPtr, but in order to obtain the hypervisor version against which libvirt was compiled it is used in combination with virConnectGetType like this: hvType = virConnectGetType(conn) virGetVersion(&libVer, hvType, &typeVer) When virConnectGetType is called on a remote connection then the remote driver returns the type of the underlying driver on the server side, for example QEMU. Then virGetVersion compares hvType to a set of strings that depend on configure options and returns LIBVIR_VERSION_NUMBER in most cases. Now this fails in case libvirt on the client side is just compiled with the remote driver enabled only and the server side has the actual driver such as the QEMU driver. It just happens to work when the actual driver is enabled on client and server side. But that's not always true. I noticed this on FreeBSD: freebsd# virsh -c qemu+tcp://192.168.178.22/system version Compiled against library: libvir 0.9.2 error: failed to get the library version error: this function is not supported by the connection driver: virGetVersion This is not FreeBSD specific, happens on Windows as well due to the similar driver support configuration. The problem is that virConnectGetType returns QEMU, but virGetVersion on the client side only accepts Remote as hvType due to all other drivers being disabled on the client side. Daniel P. Berrange suggested to get rid of all the conditional code in virGetVersion, ignoring the hvType and always setting typeVer to LIBVIR_VERSION_NUMBER. virConnectGetVersion is supposed to be used to obtain the hypervisor version. --- src/libvirt.c | 86 ++++++-------------------------------------------------- 1 files changed, 10 insertions(+), 76 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index cbe1926..5a96625 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -894,21 +894,24 @@ int virStateActive(void) { /** * virGetVersion: * @libVer: return value for the library version (OUT) - * @type: the type of connection/driver looked at, if @typeVer is not NULL + * @type: the type of the connection driver, ignored, just pass NULL * @typeVer: optional return value for the version of the hypervisor (OUT) * * Provides information on up to two versions: @libVer is the version of the * library and will always be set unless an error occurs, in which case an - * error code will be returned. If @typeVer is not NULL it will be set to the - * version of the hypervisor @type against which the library was compiled. - * If @type is NULL, "Xen" is assumed, if @type is unknown or not - * available, an error code will be returned and @typeVer will be 0. + * error code will be returned. If @typeVer is not NULL it should have been be + * set to the version of the hypervisor @type against which the library was + * compiled. Due to a design problem this doesn't work as intented. Therefore, + * this function is only usefull to obtain the local library version number via + * @libVer. To get the version of the running hypervisor use the + * virConnectGetVersion function instead. To get the libvirt library version + * used by a connection use the virConnectGetLibVersion instead. * * Returns -1 in case of failure, 0 otherwise, and values for @libVer and * @typeVer have the format major * 1,000,000 + minor * 1,000 + release. */ int -virGetVersion(unsigned long *libVer, const char *type, +virGetVersion(unsigned long *libVer, const char *type ATTRIBUTE_UNUSED, unsigned long *typeVer) { VIR_DEBUG("libVir=%p, type=%s, typeVer=%p", libVer, type, typeVer); @@ -921,78 +924,9 @@ virGetVersion(unsigned long *libVer, const char *type, goto error; *libVer = LIBVIR_VERSION_NUMBER; - if (typeVer != NULL) { - if (type == NULL) - type = "Xen"; - -/* FIXME: Add _proper_ type version handling for loadable driver modules... */ -#ifdef WITH_DRIVER_MODULES + if (typeVer != NULL) *typeVer = LIBVIR_VERSION_NUMBER; -#else - *typeVer = 0; -# if WITH_XEN - if (STRCASEEQ(type, "Xen")) - *typeVer = xenUnifiedVersion(); -# endif -# if WITH_TEST - if (STRCASEEQ(type, "Test")) - *typeVer = LIBVIR_VERSION_NUMBER; -# endif -# if WITH_QEMU - if (STRCASEEQ(type, "QEMU")) - *typeVer = LIBVIR_VERSION_NUMBER; -# endif -# if WITH_LXC - if (STRCASEEQ(type, "LXC")) - *typeVer = LIBVIR_VERSION_NUMBER; -# endif -# if WITH_LIBXL - if (STRCASEEQ(type, "xenlight")) - *typeVer = LIBVIR_VERSION_NUMBER; -# endif -# if WITH_PHYP - if (STRCASEEQ(type, "phyp")) - *typeVer = LIBVIR_VERSION_NUMBER; -# endif -# if WITH_OPENVZ - if (STRCASEEQ(type, "OpenVZ")) - *typeVer = LIBVIR_VERSION_NUMBER; -# endif -# if WITH_VMWARE - if (STRCASEEQ(type, "VMware")) - *typeVer = LIBVIR_VERSION_NUMBER; -# endif -# if WITH_VBOX - if (STRCASEEQ(type, "VBox")) - *typeVer = LIBVIR_VERSION_NUMBER; -# endif -# if WITH_UML - if (STRCASEEQ(type, "UML")) - *typeVer = LIBVIR_VERSION_NUMBER; -# endif -# if WITH_ONE - if (STRCASEEQ(type, "ONE")) - *typeVer = LIBVIR_VERSION_NUMBER; -# endif -# if WITH_ESX - if (STRCASEEQ(type, "ESX")) - *typeVer = LIBVIR_VERSION_NUMBER; -# endif -# if WITH_XENAPI - if (STRCASEEQ(type, "XenAPI")) - *typeVer = LIBVIR_VERSION_NUMBER; -# endif -# if WITH_REMOTE - if (STRCASEEQ(type, "Remote")) - *typeVer = remoteVersion(); -# endif - if (*typeVer == 0) { - virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); - goto error; - } -#endif /* WITH_DRIVER_MODULES */ - } return 0; error: -- 1.7.0.4

On 06/06/2011 05:59 AM, Matthias Bolte wrote:
When virConnectGetType is called on a remote connection then the remote driver returns the type of the underlying driver on the server side, for example QEMU. Then virGetVersion compares hvType to a set of strings that depend on configure options and returns LIBVIR_VERSION_NUMBER in most cases. Now this fails in case libvirt on the client side is just compiled with the remote driver enabled only and the server side has the actual driver such as the QEMU driver.
Conditional ACK, with docs fixed:
* Provides information on up to two versions: @libVer is the version of the
We now provide only one version.
* library and will always be set unless an error occurs, in which case an - * error code will be returned. If @typeVer is not NULL it will be set to the - * version of the hypervisor @type against which the library was compiled. - * If @type is NULL, "Xen" is assumed, if @type is unknown or not - * available, an error code will be returned and @typeVer will be 0. + * error code will be returned. If @typeVer is not NULL it should have been be
"been be"
+ * set to the version of the hypervisor @type against which the library was + * compiled. Due to a design problem this doesn't work as intented. Therefore,
intended
+ * this function is only usefull to obtain the local library version number via
useful
+ * @libVer. To get the version of the running hypervisor use the + * virConnectGetVersion function instead. To get the libvirt library version + * used by a connection use the virConnectGetLibVersion instead.
I'm thinking this looks better: * virGetVersion: * @libVer: return value for the library version (OUT) * @type: ignored; pass NULL * @typeVer: pass NULL; for historical purposes duplicates @libVer if * non-NULL * * Provides version information. @libVer is the version of the * library and will always be set unless an error occurs, in which case * an error code will be returned. @typeVer exists for historical * compatibility; if it is not NULL it will duplicate @libVer (it was * originally intended to return hypervisor information based on @type, * but due to the design of remote clients this is not reliable). To * get the version of the running hypervisor use the * virConnectGetVersion function instead. To get the libvirt library * version used by a connection use the virConnectGetLibVersion instead. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

2011/6/6 Eric Blake <eblake@redhat.com>:
On 06/06/2011 05:59 AM, Matthias Bolte wrote:
When virConnectGetType is called on a remote connection then the remote driver returns the type of the underlying driver on the server side, for example QEMU. Then virGetVersion compares hvType to a set of strings that depend on configure options and returns LIBVIR_VERSION_NUMBER in most cases. Now this fails in case libvirt on the client side is just compiled with the remote driver enabled only and the server side has the actual driver such as the QEMU driver.
Conditional ACK, with docs fixed:
* Provides information on up to two versions: @libVer is the version of the
We now provide only one version.
* library and will always be set unless an error occurs, in which case an - * error code will be returned. If @typeVer is not NULL it will be set to the - * version of the hypervisor @type against which the library was compiled. - * If @type is NULL, "Xen" is assumed, if @type is unknown or not - * available, an error code will be returned and @typeVer will be 0. + * error code will be returned. If @typeVer is not NULL it should have been be
"been be"
+ * set to the version of the hypervisor @type against which the library was + * compiled. Due to a design problem this doesn't work as intented. Therefore,
intended
+ * this function is only usefull to obtain the local library version number via
useful
+ * @libVer. To get the version of the running hypervisor use the + * virConnectGetVersion function instead. To get the libvirt library version + * used by a connection use the virConnectGetLibVersion instead.
I'm thinking this looks better:
* virGetVersion: * @libVer: return value for the library version (OUT) * @type: ignored; pass NULL * @typeVer: pass NULL; for historical purposes duplicates @libVer if * non-NULL * * Provides version information. @libVer is the version of the * library and will always be set unless an error occurs, in which case * an error code will be returned. @typeVer exists for historical * compatibility; if it is not NULL it will duplicate @libVer (it was * originally intended to return hypervisor information based on @type, * but due to the design of remote clients this is not reliable). To * get the version of the running hypervisor use the * virConnectGetVersion function instead. To get the libvirt library * version used by a connection use the virConnectGetLibVersion instead.
Yes, much better, thanks :) Pushed with improved documentation. Matthias
participants (2)
-
Eric Blake
-
Matthias Bolte