All of lore.kernel.org
 help / color / mirror / Atom feed
* [KVM-AUTOTEST PATCH 1/7] KVM test: simplify MAC address management
@ 2010-10-24 11:01 Michael Goldish
  2010-10-24 11:01 ` [KVM-AUTOTEST PATCH 2/7] KVM test: VM.get_address(): fix handling of multiple NICs Michael Goldish
  0 siblings, 1 reply; 9+ messages in thread
From: Michael Goldish @ 2010-10-24 11:01 UTC (permalink / raw)
  To: autotest, kvm

- Remove the VM.mac_prefix attribute
- Remove the 'root_dir' parameter from generate_mac_address() and
  free_mac_address()
- Remove the 'prefix' parameter from generate_mac_address()
- Remove the explicit setting and clearing of bits in the most significant
  byte (it is fixed to 0x9A anyway)
- Remove the 'shareable' parameter from set_mac_address() (no longer required)
- Remove the 'preserve_mac' parameter from VM.clone() and replace it with
  'mac_source' in VM.create()
- Remove overly verbose debug messages from free_mac_address()
- Replace VM.free_mac_addresses() with VM.free_mac_address()
- Use _open_mac_pool() and _close_mac_pool() to save code
- Merge _generate_mac_address_prefix() with generate_mac_address_prefix()
- Concentrate all functions that access the MAC address pool in the same module
  (kvm_utils.py)
- Minor style changes

Signed-off-by: Michael Goldish <mgoldish@redhat.com>
---
 client/tests/kvm/kvm_test_utils.py   |    2 +-
 client/tests/kvm/kvm_utils.py        |  185 +++++++++++++++++----------------
 client/tests/kvm/kvm_vm.py           |   93 ++++++------------
 client/tests/kvm/tests/mac_change.py |   11 +--
 4 files changed, 130 insertions(+), 161 deletions(-)

diff --git a/client/tests/kvm/kvm_test_utils.py b/client/tests/kvm/kvm_test_utils.py
index 585e194..d9c5a6e 100644
--- a/client/tests/kvm/kvm_test_utils.py
+++ b/client/tests/kvm/kvm_test_utils.py
@@ -187,7 +187,7 @@ def migrate(vm, env=None, mig_timeout=3600, mig_protocol="tcp",
 
     # Clone the source VM and ask the clone to wait for incoming migration
     dest_vm = vm.clone()
-    if not dest_vm.create(extra_params=mig_extra_params):
+    if not dest_vm.create(extra_params=mig_extra_params, mac_source=vm):
         raise error.TestError("Could not create dest VM")
 
     try:
diff --git a/client/tests/kvm/kvm_utils.py b/client/tests/kvm/kvm_utils.py
index a2b0a3f..778637d 100644
--- a/client/tests/kvm/kvm_utils.py
+++ b/client/tests/kvm/kvm_utils.py
@@ -11,6 +11,17 @@ from autotest_lib.client.common_lib import error, logging_config
 import kvm_subprocess
 
 
+def _lock_file(filename):
+    f = open(filename, "w")
+    fcntl.lockf(f, fcntl.LOCK_EX)
+    return f
+
+
+def _unlock_file(f):
+    fcntl.lockf(f, fcntl.LOCK_UN)
+    f.close()
+
+
 def dump_env(obj, filename):
     """
     Dump KVM test environment to a file.
@@ -83,119 +94,113 @@ def get_sub_dict_names(dict, keyword):
 
 # Functions related to MAC/IP addresses
 
-def _generate_mac_address_prefix():
-    """
-    Generate a MAC address prefix. By convention we will set KVM autotest
-    MAC addresses to start with 0x9a.
-    """
-    r = random.SystemRandom()
-    l = [0x9a, r.randint(0x00, 0x7f), r.randint(0x00, 0x7f),
-         r.randint(0x00, 0xff)]
-    prefix = ':'.join(map(lambda x: "%02x" % x, l)) + ":"
-    return prefix
+def _open_mac_pool(lock_mode):
+    lock_file = open("/tmp/mac_lock", "w+")
+    fcntl.lockf(lock_file, lock_mode)
+    pool = shelve.open("/tmp/address_pool")
+    return pool, lock_file
 
 
-def generate_mac_address_prefix():
+def _close_mac_pool(pool, lock_file):
+    pool.close()
+    fcntl.lockf(lock_file, fcntl.LOCK_UN)
+    lock_file.close()
+
+
+def _generate_mac_address_prefix(mac_pool):
     """
     Generate a random MAC address prefix and add it to the MAC pool dictionary.
     If there's a MAC prefix there already, do not update the MAC pool and just
-    return what's in there.
-    """
-    lock_file = open("/tmp/mac_lock", 'w')
-    fcntl.lockf(lock_file.fileno() ,fcntl.LOCK_EX)
-    mac_pool = shelve.open("/tmp/address_pool", writeback=False)
+    return what's in there. By convention we will set KVM autotest MAC
+    addresses to start with 0x9a.
 
-    if mac_pool.get('prefix'):
-        prefix = mac_pool.get('prefix')
-        logging.debug('Retrieved previously generated MAC prefix for this '
-                      'host: %s', prefix)
+    @param mac_pool: The MAC address pool object.
+    @return: The MAC address prefix.
+    """
+    if "prefix" in mac_pool:
+        prefix = mac_pool["prefix"]
+        logging.debug("Used previously generated MAC address prefix for this "
+                      "host: %s", prefix)
     else:
-        prefix = _generate_mac_address_prefix()
-        mac_pool['prefix'] = prefix
-        logging.debug('Generated MAC address prefix for this host: %s', prefix)
-
-    mac_pool.close()
-    fcntl.lockf(lock_file.fileno(), fcntl.LOCK_UN)
-    lock_file.close()
-
+        r = random.SystemRandom()
+        prefix = "9a:%02x:%02x:%02x:" % (r.randint(0x00, 0xff),
+                                         r.randint(0x00, 0xff),
+                                         r.randint(0x00, 0xff))
+        mac_pool["prefix"] = prefix
+        logging.debug("Generated MAC address prefix for this host: %s", prefix)
     return prefix
 
 
-def generate_mac_address(root_dir, instance_vm, nic_index, prefix=None):
+def generate_mac_address(vm_instance, nic_index):
     """
-    Random generate a MAC address and add it to the MAC pool.
+    Randomly generate a MAC address and add it to the MAC address pool.
 
-    Try to generate macaddress based on the mac address prefix, add it to a
-    dictionary 'address_pool'.
-    key = VM instance + nic index, value = mac address
-    {['20100310-165222-Wt7l:0'] : 'AE:9D:94:6A:9b:f9'}
+    Try to generate a MAC address based on a randomly generated MAC address
+    prefix and add it to a persistent dictionary.
+    key = VM instance + NIC index, value = MAC address
+    e.g. {'20100310-165222-Wt7l:0': '9a:5d:94:6a:9b:f9'}
 
-    @param root_dir: Root dir for kvm.
-    @param instance_vm: Here we use instance of vm.
-    @param nic_index: The index of nic.
-    @param prefix: Prefix of MAC address.
+    @param vm_instance: The instance attribute of a VM.
+    @param nic_index: The index of the NIC.
     @return: MAC address string.
     """
-    if prefix is None:
-        prefix = generate_mac_address_prefix()
-
-    r = random.SystemRandom()
-    lock_file = open("/tmp/mac_lock", 'w')
-    fcntl.lockf(lock_file.fileno() ,fcntl.LOCK_EX)
-    mac_pool = shelve.open("/tmp/address_pool", writeback=False)
-    found = False
-    key = "%s:%s" % (instance_vm, nic_index)
-
-    if mac_pool.get(key):
-        found = True
-        mac = mac_pool.get(key)
-
-    while not found:
-        suffix = "%02x:%02x" % (r.randint(0x00,0xfe),
-                                r.randint(0x00,0xfe))
-        mac = prefix + suffix
-        mac_list = mac.split(":")
-        # Clear multicast bit
-        mac_list[0] = int(mac_list[0],16) & 0xfe
-        # Set local assignment bit (IEEE802)
-        mac_list[0] = mac_list[0] | 0x02
-        mac_list[0] = "%02x" % mac_list[0]
-        mac = ":".join(mac_list)
-        if mac in [mac_pool.get(k) for k in mac_pool.keys()]:
+    mac_pool, lock_file = _open_mac_pool(fcntl.LOCK_EX)
+    key = "%s:%s" % (vm_instance, nic_index)
+    if key in mac_pool:
+        mac = mac_pool[key]
+    else:
+        prefix = _generate_mac_address_prefix(mac_pool)
+        r = random.SystemRandom()
+        while key not in mac_pool:
+            mac = prefix + "%02x:%02x" % (r.randint(0x00, 0xff),
+                                          r.randint(0x00, 0xff))
+            if mac in mac_pool.values():
                 continue
-        mac_pool[key] = mac
-        found = True
-    logging.debug("Generated MAC address for NIC %s: %s ", key, mac)
-
-    mac_pool.close()
-    fcntl.lockf(lock_file.fileno(), fcntl.LOCK_UN)
-    lock_file.close()
+            mac_pool[key] = mac
+            logging.debug("Generated MAC address for NIC %s: %s", key, mac)
+    _close_mac_pool(mac_pool, lock_file)
     return mac
 
 
-def free_mac_address(root_dir, instance_vm, nic_index):
+def free_mac_address(vm_instance, nic_index):
     """
-    Free mac address from address pool
+    Remove a MAC address from the address pool.
 
-    @param root_dir: Root dir for kvm
-    @param instance_vm: Here we use instance attribute of vm
-    @param nic_index: The index of nic
+    @param vm_instance: The instance attribute of a VM.
+    @param nic_index: The index of the NIC.
     """
-    lock_file = open("/tmp/mac_lock", 'w')
-    fcntl.lockf(lock_file.fileno() ,fcntl.LOCK_EX)
-    mac_pool = shelve.open("/tmp/address_pool", writeback=False)
-    key = "%s:%s" % (instance_vm, nic_index)
-    if not mac_pool or (not key in mac_pool.keys()):
-        logging.debug("NIC not present in the MAC pool, not modifying pool")
-        logging.debug("NIC: %s" % key)
-        logging.debug("Contents of MAC pool: %s" % mac_pool)
-    else:
-        logging.debug("Freeing MAC addr for NIC %s: %s", key, mac_pool[key])
-        mac_pool.pop(key)
+    mac_pool, lock_file = _open_mac_pool(fcntl.LOCK_EX)
+    key = "%s:%s" % (vm_instance, nic_index)
+    if key in mac_pool:
+        logging.debug("Freeing MAC address for NIC %s: %s", key, mac_pool[key])
+        del mac_pool[key]
+    _close_mac_pool(mac_pool, lock_file)
 
-    mac_pool.close()
-    fcntl.lockf(lock_file.fileno(), fcntl.LOCK_UN)
-    lock_file.close()
+
+def set_mac_address(vm_instance, nic_index, mac):
+    """
+    Set a MAC address in the pool.
+
+    @param vm_instance: The instance attribute of a VM.
+    @param nic_index: The index of the NIC.
+    """
+    mac_pool, lock_file = _open_mac_pool(fcntl.LOCK_EX)
+    mac_pool["%s:%s" % (vm_instance, nic_index)] = mac
+    _close_mac_pool(mac_pool, lock_file)
+
+
+def get_mac_address(vm_instance, nic_index):
+    """
+    Return a MAC address from the pool.
+
+    @param vm_instance: The instance attribute of a VM.
+    @param nic_index: The index of the NIC.
+    @return: MAC address string.
+    """
+    mac_pool, lock_file = _open_mac_pool(fcntl.LOCK_SH)
+    mac = mac_pool.get("%s:%s" % (vm_instance, nic_index))
+    _close_mac_pool(mac_pool, lock_file)
+    return mac
 
 
 def mac_str_to_int(addr):
diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index 848ff63..51e7cfa 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -109,16 +109,15 @@ class VM:
         self.serial_console = None
         self.redirs = {}
         self.vnc_port = 5900
-        self.uuid = None
         self.monitors = []
         self.pci_assignable = None
+        self.netdev_id = []
+        self.uuid = None
 
         self.name = name
         self.params = params
         self.root_dir = root_dir
         self.address_cache = address_cache
-        self.mac_prefix = params.get('mac_prefix')
-        self.netdev_id = []
 
         # Find a unique identifier for this VM
         while True:
@@ -127,12 +126,8 @@ class VM:
             if not glob.glob("/tmp/*%s" % self.instance):
                 break
 
-        if self.mac_prefix is None:
-            self.mac_prefix = kvm_utils.generate_mac_address_prefix()
-
 
-    def clone(self, name=None, params=None, root_dir=None,
-                    address_cache=None, preserve_mac=True):
+    def clone(self, name=None, params=None, root_dir=None, address_cache=None):
         """
         Return a clone of the VM object with optionally modified parameters.
         The clone is initially not alive and needs to be started using create().
@@ -143,7 +138,6 @@ class VM:
         @param params: Optional new VM creation parameters
         @param root_dir: Optional new base directory for relative filenames
         @param address_cache: A dict that maps MAC addresses to IP addresses
-        @param preserve_mac: Clone mac address or not.
         """
         if name is None:
             name = self.name
@@ -153,20 +147,7 @@ class VM:
             root_dir = self.root_dir
         if address_cache is None:
             address_cache = self.address_cache
-        vm = VM(name, params, root_dir, address_cache)
-        if preserve_mac:
-            vlan = 0
-            for nic_name in kvm_utils.get_sub_dict_names(params, "nics"):
-                nic_params = kvm_utils.get_sub_dict(params, nic_name)
-                vm.set_mac_address(self.get_mac_address(vlan), vlan, True)
-                vlan += 1
-        return vm
-
-
-    def free_mac_addresses(self):
-        nic_num = len(kvm_utils.get_sub_dict_names(self.params, "nics"))
-        for i in range(nic_num):
-            kvm_utils.free_mac_address(self.root_dir, self.instance, i)
+        return VM(name, params, root_dir, address_cache)
 
 
     def make_qemu_command(self, name=None, params=None, root_dir=None):
@@ -423,16 +404,7 @@ class VM:
         for nic_name in kvm_utils.get_sub_dict_names(params, "nics"):
             nic_params = kvm_utils.get_sub_dict(params, nic_name)
             # Handle the '-net nic' part
-            mac = None
-            if "address_index" in nic_params:
-                mac = kvm_utils.get_mac_ip_pair_from_dict(nic_params)[0]
-                self.set_mac_address(mac=mac, nic_index=vlan)
-            else:
-                mac = kvm_utils.generate_mac_address(self.root_dir,
-                                                     self.instance,
-                                                     vlan,
-                                                     self.mac_prefix)
-
+            mac = self.get_mac_address(vlan)
             qemu_cmd += add_nic(help, vlan, nic_params.get("nic_model"), mac,
                                 self.netdev_id[vlan],
                                 nic_params.get("nic_extra_params"))
@@ -536,7 +508,8 @@ class VM:
 
 
     def create(self, name=None, params=None, root_dir=None,
-               for_migration=False, timeout=5.0, extra_params=None):
+               for_migration=False, timeout=5.0, extra_params=None,
+               mac_source=None):
         """
         Start the VM by running a qemu command.
         All parameters are optional. The following applies to all parameters
@@ -551,6 +524,8 @@ class VM:
         option
         @param extra_params: extra params for qemu command.e.g -incoming option
         Please use this parameter instead of for_migration.
+        @param mac_source: A VM object from which to copy MAC addresses. If not
+                specified, new addresses will be generated.
         """
         self.destroy()
 
@@ -625,6 +600,15 @@ class VM:
                 self.uuid = f.read().strip()
                 f.close()
 
+            # Generate or copy MAC addresses for all NICs
+            num_nics = len(kvm_utils.get_sub_dict_names(params, "nics"))
+            for vlan in range(num_nics):
+                mac = mac_source and mac_source.get_mac_address(vlan)
+                if mac:
+                    kvm_utils.set_mac_address(self.instance, vlan, mac)
+                else:
+                    kvm_utils.generate_mac_address(self.instance, vlan)
+
             # Assign a PCI assignable device
             self.pci_assignable = None
             pa_type = params.get("pci_assignable")
@@ -799,14 +783,10 @@ class VM:
                                       "to go down...")
                         if kvm_utils.wait_for(self.is_dead, 60, 1, 1):
                             logging.debug("VM is down, freeing mac address.")
-                            self.free_mac_addresses()
                             return
                     finally:
                         session.close()
 
-            # Free mac addresses
-            self.free_mac_addresses()
-
             if self.monitor:
                 # Try to destroy with a monitor command
                 logging.debug("Trying to kill VM with monitor command...")
@@ -846,6 +826,9 @@ class VM:
                     os.unlink(f)
                 except OSError:
                     pass
+            num_nics = len(kvm_utils.get_sub_dict_names(self.params, "nics"))
+            for vlan in range(num_nics):
+                self.free_mac_address(vlan)
 
 
     @property
@@ -1002,41 +985,25 @@ class VM:
 
     def get_mac_address(self, nic_index=0):
         """
-        Return the macaddr of guest nic.
+        Return the MAC address of a NIC.
 
         @param nic_index: Index of the NIC
         """
-        mac_pool = shelve.open("/tmp/address_pool", writeback=False)
-        key = "%s:%s" % (self.instance, nic_index)
-        if key in mac_pool.keys():
-            return mac_pool[key]
+        nic_name = kvm_utils.get_sub_dict_names(self.params, "nics")[nic_index]
+        nic_params = kvm_utils.get_sub_dict(self.params, nic_name)
+        if nic_params.get("address_index"):
+            return kvm_utils.get_mac_ip_pair_from_dict(nic_params)[0]
         else:
-            return None
+            return kvm_utils.get_mac_address(self.instance, nic_index)
 
 
-    def set_mac_address(self, mac, nic_index=0, shareable=False):
+    def free_mac_address(self, nic_index=0):
         """
-        Set mac address for guest. Note: It just update address pool.
+        Free a NIC's MAC address.
 
-        @param mac: address will set to guest
         @param nic_index: Index of the NIC
-        @param shareable: Where VM can share mac with other VM or not.
         """
-        lock_file = open("/tmp/mac_lock", 'w')
-        fcntl.lockf(lock_file.fileno() ,fcntl.LOCK_EX)
-        mac_pool = shelve.open("/tmp/address_pool", writeback=False)
-        key = "%s:%s" % (self.instance, nic_index)
-
-        if not mac in [mac_pool[i] for i in mac_pool.keys()]:
-            mac_pool[key] = mac
-        else:
-            if shareable:
-                mac_pool[key] = mac
-            else:
-                logging.error("MAC address %s is already in use!", mac)
-        mac_pool.close()
-        fcntl.lockf(lock_file.fileno(), fcntl.LOCK_UN)
-        lock_file.close()
+        kvm_utils.free_mac_address(self.instance, nic_index)
 
 
     def get_pid(self):
diff --git a/client/tests/kvm/tests/mac_change.py b/client/tests/kvm/tests/mac_change.py
index 010c395..c614e15 100644
--- a/client/tests/kvm/tests/mac_change.py
+++ b/client/tests/kvm/tests/mac_change.py
@@ -24,14 +24,11 @@ def run_mac_change(test, params, env):
         raise error.TestFail("Could not log into guest '%s'" % vm.name)
 
     old_mac = vm.get_mac_address(0)
-    new_different = False
-    while not new_different:
-        kvm_utils.free_mac_address(vm.root_dir, vm.instance, 0)
-        new_mac = kvm_utils.generate_mac_address(vm.root_dir,
-                                                 vm.instance,
-                                                 0, vm.mac_prefix)
+    while True:
+        vm.free_mac_address(0)
+        new_mac = kvm_utils.generate_mac_address(vm.instance, 0)
         if old_mac != new_mac:
-            new_different = True
+            break
     logging.info("The initial MAC address is %s", old_mac)
     interface = kvm_test_utils.get_linux_ifname(session, old_mac)
     # Start change MAC address
-- 
1.5.5.6

^ permalink raw reply related	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-10-26 13:08 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-24 11:01 [KVM-AUTOTEST PATCH 1/7] KVM test: simplify MAC address management Michael Goldish
2010-10-24 11:01 ` [KVM-AUTOTEST PATCH 2/7] KVM test: VM.get_address(): fix handling of multiple NICs Michael Goldish
2010-10-24 11:01   ` [KVM-AUTOTEST PATCH 3/7] [RFC] KVM test: remove all code related to the old MAC address pool method Michael Goldish
2010-10-24 11:01     ` [KVM-AUTOTEST PATCH 4/7] KVM test: get_ifname(): use self.instance instead of self.vnc_port Michael Goldish
2010-10-24 11:01       ` [KVM-AUTOTEST PATCH 5/7] KVM test: kvm_monitor.py: replace MonitorSendError with MonitorSocketError Michael Goldish
2010-10-24 11:01         ` [KVM-AUTOTEST PATCH 6/7] KVM test: refactor migration code Michael Goldish
2010-10-24 11:01           ` [KVM-AUTOTEST PATCH 7/7] [RFC] KVM test: migrate_cancel: allow for alternative spellings of 'cancelled' Michael Goldish
2010-10-26 13:08             ` Lucas Meneghel Rodrigues
2010-10-26 12:56     ` [KVM-AUTOTEST PATCH 3/7] [RFC] KVM test: remove all code related to the old MAC address pool method Lucas Meneghel Rodrigues

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.