[et-mgmt-tools] [PATCH] Check the making domain's mac address

[Tatsuro Enokura]

Hi, Hugh

Hugh Brock wrote:
>>>>> The virt-install command can specify the making domain's vnif
>>>>> MAC address. The MAC address must be unique on the system,
>>>>> but the virt-install command doesn't check that
>>>>> the MAC address is unique among the running domains and host.
>>>>>
>>>>> The attached patch resolve this issue in the following way:
>>>>>
>>>>>   1) Get the running Domain's vnif MAC address.
>>>>>   2) Get the host's NIC MAC address.
>>>>>   3) Check the making domain's MAC address with 1) and 2) 's data.
>>
>>> Actually, you really don't need to build up a list of mac addresses and
>>> iterate over them. A much easier way is simply to get the XML for a
>>> domain and then use an xpath expression something like
>>>
>>>     if 
>>> ctx.xpathEval("count(/domain/devices/interface/mac/@address='%s')" %
>>> macaddr) > 0:
>>>     # handle the case where the macaddr conflicts with an existing 
>>> domain
>>>
>>> If you can rewrite the patch along these lines I'll be happy to take it.
>>
>> Thank you for your suggestion.
>> I rewrite the patch.
> 
> I'm reviewing this now. I'm trying to work out if there's a way to do it 
> without repeating so much code... :)...

Thank you for viewing my code.
I rewrite the patch that the repeating code become the method.

Thanks,
Tatsuro Enokura

--------------------------------------------------------------------
diff -r 057e8c1b54df virtinst/Guest.py
--- a/virtinst/Guest.py Wed Mar 14 16:11:43 2007 -0400
+++ b/virtinst/Guest.py Thu Mar 15 15:07:26 2007 +0900
@@ -15,6 +15,7 @@ import os, os.path
 import os, os.path
 import stat, sys, time
 import re
+import libxml2

 import libvirt

@@ -148,9 +149,32 @@ class VirtualNetworkInterface:
         self.macaddr = macaddr
         self.bridge = bridge

-    def setup(self):
+    def setup(self, conn):
+        # get Running Domains
+        ids = conn.listDomainsID();
+        vms = []
+        for id in ids:
+            vm = conn.lookupByID(id)
+            vms.append(vm)
+
+        # get the Host's NIC MACaddress
+        hostdevs = util.get_host_network_devices()
+
+        # check conflict MAC address
         if self.macaddr is None:
-            self.macaddr = util.randomMAC()
+            while 1:
+                self.macaddr = util.randomMAC()
+                if self.countMACaddr(vms) > 0:
+                    continue
+                else:
+                    break
+        else:
+            if self.countMACaddr(vms) > 0:
+                raise RuntimeError, "The MAC address you entered is
already in use by another guest!"
+            for (dummy, dummy, dummy, dummy, host_macaddr) in hostdevs:
+                if self.macaddr.upper() == host_macaddr.upper():
+                    raise RuntimeError, "The MAC address you entered is
conflict with the physical NIC."
+
         if not self.bridge:
             self.bridge = util.default_bridge()

@@ -160,6 +184,30 @@ class VirtualNetworkInterface:
                 "      <mac address='%(mac)s'/>\n" + \
                 "    </interface>\n") % \
                 { "bridge": self.bridge, "mac": self.macaddr }
+
+    def countMACaddr(self, vms):
+        count = 0
+        for vm in vms:
+            doc = None
+            try:
+                doc = libxml2.parseDoc(vm.XMLDesc(0))
+            except:
+                continue
+            ctx = doc.xpathNewContext()
+            try:
+                try:
+                    count +=
ctx.xpathEval("count(/domain/devices/interface/mac[@address='%s'])"
+                                           % self.macaddr.upper())
+                    count +=
ctx.xpathEval("count(/domain/devices/interface/mac[@address='%s'])"
+                                           % self.macaddr.lower())
+                except:
+                    continue
+            finally:
+                if ctx is not None:
+                    ctx.xpathFreeContext()
+                if doc is not None:
+                    doc.freeDoc()
+        return count

 # Back compat class to avoid ABI break
 class XenNetworkInterface(VirtualNetworkInterface):
@@ -429,7 +477,7 @@ class Guest(object):
         for disk in self.disks:
             disk.setup(progresscb)
         for nic in self.nics:
-            nic.setup()
+            nic.setup(self.conn)

     def _get_disk_xml(self, install = True):
         """Get the disk config in the libvirt XML format"""
diff -r 057e8c1b54df virtinst/util.py
--- a/virtinst/util.py  Wed Mar 14 16:11:43 2007 -0400
+++ b/virtinst/util.py  Thu Mar 15 14:05:00 2007 +0900
@@ -123,3 +123,22 @@ def uuidFromString(s):
 def uuidFromString(s):
     s = s.replace('-', '')
     return [ int(s[i : i + 2], 16) for i in range(0, 32, 2) ]
+
+# the following function quotes from python2.5/uuid.py
+def get_host_network_devices():
+    device = []
+    for dir in ['', '/sbin/', '/usr/sbin']:
+        executable = os.path.join(dir, "ifconfig")
+        if not os.path.exists(executable):
+            continue
+        try:
+            cmd = 'LC_ALL=C %s -a 2>/dev/null' % (executable)
+            pipe = os.popen(cmd)
+        except IOError:
+            continue
+        for line in pipe:
+            words = line.lower().split()
+            for i in range(len(words)):
+                if words[i] == "hwaddr":
+                    device.append(words)
+    return device
--------------------------------------------------------------------