Re: [libvirt] [PATCH] Add virConnectGetLibvirtVersion API
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(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/