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