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
Attachment:
pgpbcK04QFnhU.pgp
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list