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

Hey, On Wed, Nov 14, 2012 at 03:45:00PM +0100, Michal Privoznik wrote:

On 14.11.2012 15:16, Christophe Fergeau wrote:

...

g_type_init has been deprecated in glib 2.35, not 2.34. With versions older than 2.35, we have to call it or we'll get a runtime failure. --- libvirt-gconfig/libvirt-gconfig-compat.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libvirt-gconfig/libvirt-gconfig-compat.h b/libvirt-gconfig/libvirt-gconfig-compat.h index 85a420d..3719896 100644 --- a/libvirt-gconfig/libvirt-gconfig-compat.h +++ b/libvirt-gconfig/libvirt-gconfig-compat.h @@ -25,7 +25,7 @@

#include <glib-object.h>

-#if GLIB_CHECK_VERSION(2, 34, 0) +#if GLIB_CHECK_VERSION(2, 35, 0) #define g_type_init() #endif

Wasn't it deprecated in 2.36? From [1]:

g_type_init has been deprecated since version 2.36 ...

2.35 is the development version for 2.36, so it has been deprecated in the 2.35 release cycle, but the doc only mention 2.36 as this is when it will get available. However, 2.36 is not released yet, but people using the 2.35 development version will hit the deprecation warning.

Moreover, this is just a compile time check. Don't we want a runtime one? That is glib_check_version().

The deprecation warning happens at compile time, I don't think there's any runtime warning when calling g_type_init(). Christophe