Merge lp:~niedbalski/uvtool/fixes-lp-1428674 into lp:~uvtool-dev/uvtool/trunk

[Jorge Niedbalski]

Proposed fix for #1428674

[lp:~niedbalski/uvtool/fixes-lp-1428674]

94. By Jorge Niedbalski on 2015-03-05

Fixed file size

[Robie Basak (racb)]

This doesn't fix "uvt-kvm wait", which runs inotify on the default.leases file. Commented in the bug, as I think that we need to discuss the general approach.

A couple of comments on this MP specifically:

Why 1 instead of 0 for the file size? I presume you need it because you added another commit changing it, but I'd like to see a comment in the code explaining why.

1, 2, 3 and 0 as arguments to _filter_mac should be named constants or carry an explanation of what they are some other way.

review: Needs Fixing

[Robie Basak (racb)]

I think this has since been resolved in a different way.

Unmerged revisions

94. By Jorge Niedbalski on 2015-03-05

Fixed file size

93. By Jorge Niedbalski on 2015-03-05

[niedbalski, r=] Fixes bugs LP #1428674 and LP #1420142
 - Since libvirt v1.2.11 though, libvirt adds --leasefile-ro which stops it
   updating the dnsmasq lease file and makes libvirt store the leases itself.
 - This patch uses /proc/net/arp as a fallback in case no leasefile exist.

=== modified file 'uvtool/libvirt/__init__.py'
--- uvtool/libvirt/__init__.py	2014-01-23 17:21:41 +0000
+++ uvtool/libvirt/__init__.py	2015-03-05 16:25:47 +0000
@@ -31,6 +31,7 @@
 from lxml.builder import E
 
 LIBVIRT_DNSMASQ_LEASE_FILE = '/var/lib/libvirt/dnsmasq/default.leases'
+PROCFS_ARP = '/proc/net/arp'
 
 
 def get_libvirt_pool_object(libvirt_conn, pool_name):
@@ -238,9 +239,19 @@
 
 def mac_to_ip(mac):
     canonical_mac = mac.lower()
-    with codecs.open(LIBVIRT_DNSMASQ_LEASE_FILE, 'r') as f:
-        for line in f:
-            fields = line.split()
-            if len(fields) > 1 and fields[1].lower() == canonical_mac:
-                return fields[2]
+
+    def _filter_mac(path, mac, mac_f, ip_f):
+        with codecs.open(path, 'r') as ff:
+            for line in ff:
+                fields = line.split()
+                if len(fields) > 1 and fields[mac_f].lower() == mac:
+                    return fields[ip_f]
         return None
+
+    if os.path.exists(LIBVIRT_DNSMASQ_LEASE_FILE) and \
+       os.path.getsize(LIBVIRT_DNSMASQ_LEASE_FILE) > 1:
+        return _filter_mac(LIBVIRT_DNSMASQ_LEASE_FILE, canonical_mac, 1, 2)
+    elif os.path.exists(PROCFS_ARP):
+        return _filter_mac(PROCFS_ARP, canonical_mac, 3, 0)
+
+    return None