[libvirt] [PATCH] Add virConnectGetLibvirtVersion API
Hi all, The attached patch adds a new API call for retrieving the libvirt version used by a connection: virConnectGetLibvirtVersion. Without this, there is currently no way to determine the libvirt version of a remote qemu connection for example. This info can be useful for feature detection/enabling/disabling. As an example, virt-install may want to use the AC97 sound device as the default sound model for new VMs. However, this was only added to libvirt's white list in 0.6.0, while the other models have been available since 0.4.3. If installing a remote guest, virt-install will want to ensure that the remote libvirtd is >= 0.6.0. Granted, the remote version could have backported the AC97 patch and virt-install would be incorrect, but better to be overly restrictive than to blindly specify AC97 and have guest creation bomb out. The 'correct' way to handle the above issue would be some combination of dropping internal whitelists from libvirt and generating them from info reported by the hypervisor, and advertising the available values in the capabilities XML. However I think this API addition makes things more manageable with little downside until a proper solution is implemented. Thanks, Cole
On Mon, Nov 02, 2009 at 03:52:28PM -0500, Cole Robinson wrote:
Hi all,
The attached patch adds a new API call for retrieving the libvirt version used by a connection: virConnectGetLibvirtVersion. Without this, there is currently no way to determine the libvirt version of a remote qemu connection for example. This info can be useful for feature detection/enabling/disabling.
As an example, virt-install may want to use the AC97 sound device as the default sound model for new VMs. However, this was only added to libvirt's white list in 0.6.0, while the other models have been available since 0.4.3. If installing a remote guest, virt-install will want to ensure that the remote libvirtd is >= 0.6.0. Granted, the remote version could have backported the AC97 patch and virt-install would be incorrect, but better to be overly restrictive than to blindly specify AC97 and have guest creation bomb out.
The 'correct' way to handle the above issue would be some combination of dropping internal whitelists from libvirt and generating them from info reported by the hypervisor, and advertising the available values in the capabilities XML. However I think this API addition makes things more manageable with little downside until a proper solution is implemented. [...] /** + * virConnectGetLibvirtVersion: + * @conn: pointer to the hypervisor connection + * @libVer: returns the libvirt library version used on the connection (OUT) + * + * Provides @libVer, which is the version of the libvirt on the @conn host. + * + * Returns -1 in case of failure, 0 otherwise, and values for @libVer have + * the format major * 1,000,000 + minor * 1,000 + release. + */ +int +virConnectGetLibvirtVersion(virConnectPtr conn, unsigned long *libVer)
I have no problem in general with this addition, but I wonder if it should not be improved. The idea with virGetVersion() was that in a single call we would get both the version of the library used and the version of the underlying hypervisor. This somehow failed because it assumed the type of hypervisor had to be provided by the caller (while it's really based on the connection URI) and the fact that hypervisor versions aren't really exposed. Only the protocol version can be exported that way in practice. I was wondering if we should not extend virConnectGetLibvirtVersion() with an extra argument for the hypervisor version. But in restrospect I don't think it's possible, for example when using QEmu/KVM there may be different versions of the emulator present and any versioning should probably be provided from the <capabilities><guest> possibly as an hypervisor/version info instead. So as is I'm fine with the patch, I understand the need. I would possibly increase the description a bit: * Provides @libVer, which is the version of libvirt used by the * daemon running on the @conn host the point about the daemon version is to insist on the decoupling between local version vs. remote version, and also by the fact that even if @host had a libvirt upgrade, if the daemon wasn't restarted it will still report the old version because that's what it is using. ACK from me, might be good to get a review from Dan too :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Mon, Nov 02, 2009 at 03:52:28PM -0500, Cole Robinson wrote:
Hi all,
The attached patch adds a new API call for retrieving the libvirt version used by a connection: virConnectGetLibvirtVersion. Without this, there is currently no way to determine the libvirt version of a remote qemu connection for example. This info can be useful for feature detection/enabling/disabling.
As an example, virt-install may want to use the AC97 sound device as the default sound model for new VMs. However, this was only added to libvirt's white list in 0.6.0, while the other models have been available since 0.4.3. If installing a remote guest, virt-install will want to ensure that the remote libvirtd is >= 0.6.0. Granted, the remote version could have backported the AC97 patch and virt-install would be incorrect, but better to be overly restrictive than to blindly specify AC97 and have guest creation bomb out.
The 'correct' way to handle the above issue would be some combination of dropping internal whitelists from libvirt and generating them from info reported by the hypervisor, and advertising the available values in the capabilities XML. However I think this API addition makes things more manageable with little downside until a proper solution is implemented.
Even as a simple debugging aid for bug reporting, it would be justified to add this extra API call to get the remote version.
commit 59871ddf8956a96a1148769c05ada6e763d91080 Author: Cole Robinson <crobinso@redhat.com> Date: Mon Nov 2 15:34:46 2009 -0500
Add virConnectGetLibvirtVersion
There is currently no way to determine the libvirt version of a remote libvirtd we are connected to. This is a useful piece of data to enable feature detection.
I think I'd prefer a name of either virConnectGetLibVersion virConnectGetAPIVersion
diff --git a/src/libvirt.c b/src/libvirt.c index 5ddf27a..85d7008 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -1437,6 +1437,48 @@ error: }
/** + * virConnectGetLibvirtVersion: + * @conn: pointer to the hypervisor connection + * @libVer: returns the libvirt library version used on the connection (OUT) + * + * Provides @libVer, which is the version of the libvirt on the @conn host. + * + * Returns -1 in case of failure, 0 otherwise, and values for @libVer have + * the format major * 1,000,000 + minor * 1,000 + release. + */ +int +virConnectGetLibvirtVersion(virConnectPtr conn, unsigned long *libVer) +{ + DEBUG("conn=%p, libVir=%p", conn, libVer); + + virResetLastError(); + + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__); + return -1; + } + + if (libVer == NULL) { + virLibConnError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + + if (conn->driver->libvirtVersion) { + int ret = conn->driver->libvirtVersion(conn, libVer); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + /* Copy to connection error object for back compatability */ + virSetConnError(conn); + return -1; +}
Wouldn't it be better here to fallback to *libVer = LIBVIR_VERSION_NUMBER; in the case of 'conn->driver->libvirtVersion' being NULL. That would mean you'd only need to implemnet this driver method for the remote driver, letting all the others fallback to this generic case as they do with virGetVersion()
diff --git a/src/util/util.c b/src/util/util.c index 853d3a0..e03a64e 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -1801,6 +1801,14 @@ int virDiskNameToIndex(const char *name) { return idx; }
+int virGetLibvirtVersion(virConnectPtr conn ATTRIBUTE_UNUSED, + unsigned long *lib_ver) +{ + *lib_ver = LIBVIR_VERSION_NUMBER; + + return 0; +} +
And thus avoiding need for this method. Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On 11/12/2009 06:49 AM, Daniel P. Berrange wrote:
On Mon, Nov 02, 2009 at 03:52:28PM -0500, Cole Robinson wrote:
Hi all,
The attached patch adds a new API call for retrieving the libvirt version used by a connection: virConnectGetLibvirtVersion. Without this, there is currently no way to determine the libvirt version of a remote qemu connection for example. This info can be useful for feature detection/enabling/disabling.
As an example, virt-install may want to use the AC97 sound device as the default sound model for new VMs. However, this was only added to libvirt's white list in 0.6.0, while the other models have been available since 0.4.3. If installing a remote guest, virt-install will want to ensure that the remote libvirtd is >= 0.6.0. Granted, the remote version could have backported the AC97 patch and virt-install would be incorrect, but better to be overly restrictive than to blindly specify AC97 and have guest creation bomb out.
The 'correct' way to handle the above issue would be some combination of dropping internal whitelists from libvirt and generating them from info reported by the hypervisor, and advertising the available values in the capabilities XML. However I think this API addition makes things more manageable with little downside until a proper solution is implemented.
Even as a simple debugging aid for bug reporting, it would be justified to add this extra API call to get the remote version.
commit 59871ddf8956a96a1148769c05ada6e763d91080 Author: Cole Robinson <crobinso@redhat.com> Date: Mon Nov 2 15:34:46 2009 -0500
Add virConnectGetLibvirtVersion
There is currently no way to determine the libvirt version of a remote libvirtd we are connected to. This is a useful piece of data to enable feature detection.
I think I'd prefer a name of either
virConnectGetLibVersion virConnectGetAPIVersion
I'll go with GetLibVersion
diff --git a/src/libvirt.c b/src/libvirt.c index 5ddf27a..85d7008 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -1437,6 +1437,48 @@ error: }
/** + * virConnectGetLibvirtVersion: + * @conn: pointer to the hypervisor connection + * @libVer: returns the libvirt library version used on the connection (OUT) + * + * Provides @libVer, which is the version of the libvirt on the @conn host. + * + * Returns -1 in case of failure, 0 otherwise, and values for @libVer have + * the format major * 1,000,000 + minor * 1,000 + release. + */ +int +virConnectGetLibvirtVersion(virConnectPtr conn, unsigned long *libVer) +{ + DEBUG("conn=%p, libVir=%p", conn, libVer); + + virResetLastError(); + + if (!VIR_IS_CONNECT(conn)) { + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__); + return -1; + } + + if (libVer == NULL) { + virLibConnError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__); + goto error; + } + + if (conn->driver->libvirtVersion) { + int ret = conn->driver->libvirtVersion(conn, libVer); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + /* Copy to connection error object for back compatability */ + virSetConnError(conn); + return -1; +}
Wouldn't it be better here to fallback to
*libVer = LIBVIR_VERSION_NUMBER;
in the case of 'conn->driver->libvirtVersion' being NULL. That would mean you'd only need to implemnet this driver method for the remote driver, letting all the others fallback to this generic case as they do with virGetVersion()
I did that at first, but thought it was kind of hacky. I figured it would be easier for driver writers to plug the util.c function into their driver table, rather than ask 'How do I implement this?' only to end up at libvirt.c and realize it's implemented for them. I suppose a comment would clear that up, so I'll reinstate that behavior. Thanks, Cole
On 11/02/2009 03:52 PM, Cole Robinson wrote:
Hi all,
The attached patch adds a new API call for retrieving the libvirt version used by a connection: virConnectGetLibvirtVersion. Without this, there is currently no way to determine the libvirt version of a remote qemu connection for example. This info can be useful for feature detection/enabling/disabling.
As an example, virt-install may want to use the AC97 sound device as the default sound model for new VMs. However, this was only added to libvirt's white list in 0.6.0, while the other models have been available since 0.4.3. If installing a remote guest, virt-install will want to ensure that the remote libvirtd is >= 0.6.0. Granted, the remote version could have backported the AC97 patch and virt-install would be incorrect, but better to be overly restrictive than to blindly specify AC97 and have guest creation bomb out.
The 'correct' way to handle the above issue would be some combination of dropping internal whitelists from libvirt and generating them from info reported by the hypervisor, and advertising the available values in the capabilities XML. However I think this API addition makes things more manageable with little downside until a proper solution is implemented.
Thanks, Cole
I've just pushed this with the recommended tweaks. Thanks, Cole
participants (3)
-
Cole Robinson -
Daniel P. Berrange -
Daniel Veillard