Hey, On Wed, Nov 14, 2012 at 03:45:49PM +0100, Michal Privoznik wrote:
On 14.11.2012 15:16, Christophe Fergeau wrote:
This method returns a version number, which can be 0, so we cannot check the return value for 0 to know if an error happened. Test if the GError is set instead to detect errors. --- examples/conn-test.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/examples/conn-test.c b/examples/conn-test.c index b98d115..3e0e148 100644 --- a/examples/conn-test.c +++ b/examples/conn-test.c @@ -47,7 +47,8 @@ do_connection_open(GObject *source,
g_print("Hypervisor name: %s\n", hv_name);
- if (!(hv_version = gvir_connection_get_version(conn, &err))) { + hv_version = gvir_connection_get_version(conn, &err); + if (err != NULL) { g_error("%s", err->message); }
ACK
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