Merge lp:~justin-fathomdb/nova/libvirt-getversion-whackamole into lp:~hudson-openstack/nova/trunk

[justinsb]

Detect if user is running the default Lucid version of libvirt, and give a nicer error message

Check both libvirt connection object (Maverick) and libvirt namespace (Lucid) for getVersion function

Earlier versions of the python libvirt binding had getVersion in the libvirt namespace, not on the connection object. Check both.

[Jay Pipes (jaypipes)]

lgtm.

review: Approve

[justinsb (justin-fathomdb)]

Though this patch let me get further, I think hit this error when launching an image " virConnect instance has no attribute 'nwfilterDefineXML'"

Paging Soren...

Soren: Is it a lost cause trying to support Lucid's libvirt? If so, there's no point merging this patch (but I could change it to give a friendly error message if an old libvirt is installed!)

[Vish Ishaya (vishvananda)]

I think it is a lost cause. That is why we have a backported libvirt in the ppa.

Vish

On Mar 23, 2011, at 10:01 AM, justinsb wrote:

> Though this patch let me get further, I think hit this error when launching an image " virConnect instance has no attribute 'nwfilterDefineXML'"
>
> Paging Soren...
>
> Soren: Is it a lost cause trying to support Lucid's libvirt? If so, there's no point merging this patch (but I could change it to give a friendly error message if an old libvirt is installed!)
> --
> https://code.launchpad.net/~justin-fathomdb/nova/libvirt-getversion-whackamole/+merge/54475
> You are subscribed to branch lp:nova.

[justinsb (justin-fathomdb)]

OK... changed this so that it gives a nicer error message if getVersion isn't available on the connection object, along with a note on where to find it in case anyone needs it in future...

[Soren Hansen (soren)]

> I think it is a lost cause.

+1

[Josh Kearney (jk0)]

Looks good to me. I don't see any harm in making an error message more friendly.

review: Approve

[Jesse Andrews (anotherjesse)]

Not sure if it is a lost cause - since it makes it easier to run on other platforms that we don't have a PPA for.

review: Approve

=== modified file 'nova/virt/libvirt_conn.py'
--- nova/virt/libvirt_conn.py	2011-03-23 05:29:32 +0000
+++ nova/virt/libvirt_conn.py	2011-03-23 20:33:34 +0000
@@ -981,7 +981,18 @@
 
         """
 
-        return self._conn.getVersion()
+        # NOTE(justinsb): getVersion moved between libvirt versions
+        # Trying to do be compatible with older versions is a lost cause
+        # But ... we can at least give the user a nice message
+        method = getattr(self._conn, 'getVersion', None)
+        if method is None:
+            raise exception.Error(_("libvirt version is too old"
+                                    " (does not support getVersion)"))
+            # NOTE(justinsb): If we wanted to get the version, we could:
+            # method = getattr(libvirt, 'getVersion', None)
+            # NOTE(justinsb): This would then rely on a proper version check
+
+        return method()
 
     def get_cpu_info(self):
         """Get cpuinfo information.