Re: [libvirt] [libvirt-glib 2/2] conn-test: Fix gvir_connection_get_version error check

Hey, On Wed, Nov 14, 2012 at 03:45:49PM +0100, Michal Privoznik wrote: Looking a bit more at this, the situation is a bit more complicated. virConnectGetVersion documentation says: "* Returns -1 in case of error, 0 otherwise. if the version can't be * extracted by lack of capacities returns 0 and @hvVer is 0, otherwise * @hvVer value is major * 1,000,000 + minor * 1,000 + release *" virConnectGetVersion returning 0 indicates some kind of error, so it would probably be better for gvir_connection_get_version to set a GError in this case rather than just returning 0. Ie something like the patch below may be more correct: diff --git a/libvirt-gobject/libvirt-gobject-connection.c b/libvirt-gobject/libvirt-gobject-connection.c index 3157a66..500de78 100644 --- a/libvirt-gobject/libvirt-gobject-connection.c +++ b/libvirt-gobject/libvirt-gobject-connection.c @@ -1108,6 +1108,11 @@ gvir_connection_get_version(GVirConnection *conn, if (virConnectGetVersion(priv->conn, &ret) < 0) { gvir_set_error_literal(err, GVIR_CONNECTION_ERROR, 0, "Unable to get hypervisor version"); + ret = 0; + } else if (ret == 0) { + /* From virConnectGetVersion doc: "if the version can't be + * extracted by lack of capacities returns 0 and @hvVer is 0" */ + g_set_error_literal(err, GVIR_CONNECTION_ERROR, 0, "XXXX"); } cleanup: As a side note, I get virConnectGetVersion to return 0 by running libvirtd 1.0.0 as a user, and then running virsh version with virsh 0.10.2. It seems to me that in this case virConnectGetVersion should either work or return -1, but not 0. I had a quick look at the code, but stopped when I saw there were remote calls involved... Any thoughts on all of this? Christophe