public inbox for kvm@vger.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

* [KVM-AUTOTEST PATCH 2/7] KVM test: VM.get_address(): fix handling of multiple NICs
  2010-10-24 11:01 [KVM-AUTOTEST PATCH 1/7] KVM test: simplify MAC address management Michael Goldish
@ 2010-10-24 11:01 ` 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
  0 siblings, 1 reply; 9+ messages in thread
From: Michael Goldish @ 2010-10-24 11:01 UTC (permalink / raw)
  To: autotest, kvm

Use VM.get_mac_address() instead of kvm_utils.get_mac_ip_pair_from_dict().

Signed-off-by: Michael Goldish <mgoldish@redhat.com>
---
 client/tests/kvm/kvm_vm.py |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index 51e7cfa..f3e803b 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -930,11 +930,7 @@ class VM:
                                   "%s" % mac)
                     return None
                 # Make sure the IP address is assigned to this guest
-                nic_dicts = [kvm_utils.get_sub_dict(self.params, nic)
-                             for nic in nics]
-                macs = [kvm_utils.get_mac_ip_pair_from_dict(dict)[0]
-                        for dict in nic_dicts]
-                macs.append(mac)
+                macs = [self.get_mac_address(i) for i in range(len(nics))]
                 if not kvm_utils.verify_ip_address_ownership(ip, macs):
                     logging.debug("Could not verify MAC-IP address mapping: "
                                   "%s ---> %s" % (mac, ip))
-- 
1.5.5.6

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [KVM-AUTOTEST PATCH 3/7] [RFC] KVM test: remove all code related to the old MAC address pool method
  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   ` 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-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
  0 siblings, 2 replies; 9+ messages in thread
From: Michael Goldish @ 2010-10-24 11:01 UTC (permalink / raw)
  To: autotest, kvm

This patch removes all code related to the old manual method
(address_pools.cfg).

Note that now running in TAP mode requires an external DHCP server that accepts
*any* MAC address, because MAC addresses are randomly generated and cannot be
manually configured.

Signed-off-by: Michael Goldish <mgoldish@redhat.com>
---
 client/tests/kvm/kvm_utils.py                      |  159 --------------------
 client/tests/kvm/kvm_vm.py                         |   34 ++---
 client/tests/kvm/tests/physical_resources_check.py |   11 +-
 client/tests/kvm/tests/stress_boot.py              |    3 -
 client/tests/kvm/tests_base.cfg.sample             |    2 -
 5 files changed, 16 insertions(+), 193 deletions(-)

diff --git a/client/tests/kvm/kvm_utils.py b/client/tests/kvm/kvm_utils.py
index 778637d..f749f8d 100644
--- a/client/tests/kvm/kvm_utils.py
+++ b/client/tests/kvm/kvm_utils.py
@@ -203,165 +203,6 @@ def get_mac_address(vm_instance, nic_index):
     return mac
 
 
-def mac_str_to_int(addr):
-    """
-    Convert MAC address string to integer.
-
-    @param addr: String representing the MAC address.
-    """
-    return sum(int(s, 16) * 256 ** i
-               for i, s in enumerate(reversed(addr.split(":"))))
-
-
-def mac_int_to_str(addr):
-    """
-    Convert MAC address integer to string.
-
-    @param addr: Integer representing the MAC address.
-    """
-    return ":".join("%02x" % (addr >> 8 * i & 0xFF)
-                    for i in reversed(range(6)))
-
-
-def ip_str_to_int(addr):
-    """
-    Convert IP address string to integer.
-
-    @param addr: String representing the IP address.
-    """
-    return sum(int(s) * 256 ** i
-               for i, s in enumerate(reversed(addr.split("."))))
-
-
-def ip_int_to_str(addr):
-    """
-    Convert IP address integer to string.
-
-    @param addr: Integer representing the IP address.
-    """
-    return ".".join(str(addr >> 8 * i & 0xFF)
-                    for i in reversed(range(4)))
-
-
-def offset_mac(base, offset):
-    """
-    Add offset to a given MAC address.
-
-    @param base: String representing a MAC address.
-    @param offset: Offset to add to base (integer)
-    @return: A string representing the offset MAC address.
-    """
-    return mac_int_to_str(mac_str_to_int(base) + offset)
-
-
-def offset_ip(base, offset):
-    """
-    Add offset to a given IP address.
-
-    @param base: String representing an IP address.
-    @param offset: Offset to add to base (integer)
-    @return: A string representing the offset IP address.
-    """
-    return ip_int_to_str(ip_str_to_int(base) + offset)
-
-
-def get_mac_ip_pair_from_dict(dict):
-    """
-    Fetch a MAC-IP address pair from dict and return it.
-
-    The parameters in dict are expected to conform to a certain syntax.
-    Typical usage may be:
-
-    address_ranges = r1 r2 r3
-
-    address_range_base_mac_r1 = 55:44:33:22:11:00
-    address_range_base_ip_r1 = 10.0.0.0
-    address_range_size_r1 = 16
-
-    address_range_base_mac_r2 = 55:44:33:22:11:40
-    address_range_base_ip_r2 = 10.0.0.60
-    address_range_size_r2 = 25
-
-    address_range_base_mac_r3 = 55:44:33:22:12:10
-    address_range_base_ip_r3 = 10.0.1.20
-    address_range_size_r3 = 230
-
-    address_index = 0
-
-    All parameters except address_index specify a MAC-IP address pool.  The
-    pool consists of several MAC-IP address ranges.
-    address_index specified the index of the desired MAC-IP pair from the pool.
-
-    @param dict: The dictionary from which to fetch the addresses.
-    """
-    index = int(dict.get("address_index", 0))
-    for mac_range_name in get_sub_dict_names(dict, "address_ranges"):
-        mac_range_params = get_sub_dict(dict, mac_range_name)
-        mac_base = mac_range_params.get("address_range_base_mac")
-        ip_base = mac_range_params.get("address_range_base_ip")
-        size = int(mac_range_params.get("address_range_size", 1))
-        if index < size:
-            return (mac_base and offset_mac(mac_base, index),
-                    ip_base and offset_ip(ip_base, index))
-        index -= size
-    return (None, None)
-
-
-def get_sub_pool(dict, piece, num_pieces):
-    """
-    Split a MAC-IP pool and return a single requested piece.
-
-    For example, get_sub_pool(dict, 0, 3) will split the pool in 3 pieces and
-    return a dict representing the first piece.
-
-    @param dict: A dict that contains pool parameters.
-    @param piece: The index of the requested piece.  Should range from 0 to
-        num_pieces - 1.
-    @param num_pieces: The total number of pieces.
-    @return: A copy of dict, modified to describe the requested sub-pool.
-    """
-    range_dicts = [get_sub_dict(dict, name) for name in
-                   get_sub_dict_names(dict, "address_ranges")]
-    if not range_dicts:
-        return dict
-    ranges = [[d.get("address_range_base_mac"),
-               d.get("address_range_base_ip"),
-               int(d.get("address_range_size", 1))] for d in range_dicts]
-    total_size = sum(r[2] for r in ranges)
-    base = total_size * piece / num_pieces
-    size = total_size * (piece + 1) / num_pieces - base
-
-    # Find base of current sub-pool
-    for i in range(len(ranges)):
-        r = ranges[i]
-        if base < r[2]:
-            r[0] = r[0] and offset_mac(r[0], base)
-            r[1] = r[1] and offset_ip(r[1], base)
-            r[2] -= base
-            break
-        base -= r[2]
-
-    # Collect ranges up to end of current sub-pool
-    new_ranges = []
-    for i in range(i, len(ranges)):
-        r = ranges[i]
-        new_ranges.append(r)
-        if size <= r[2]:
-            r[2] = size
-            break
-        size -= r[2]
-
-    # Write new dict
-    new_dict = dict.copy()
-    new_dict["address_ranges"] = " ".join("r%d" % i for i in
-                                          range(len(new_ranges)))
-    for i in range(len(new_ranges)):
-        new_dict["address_range_base_mac_r%d" % i] = new_ranges[i][0]
-        new_dict["address_range_base_ip_r%d" % i] = new_ranges[i][1]
-        new_dict["address_range_size_r%d" % i] = new_ranges[i][2]
-    return new_dict
-
-
 def verify_ip_address_ownership(ip, macs, timeout=10.0):
     """
     Use arping and the ARP cache to make sure a given IP address belongs to one
diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index f3e803b..2db916f 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -920,21 +920,18 @@ class VM:
                 logging.debug("MAC address unavailable")
                 return None
             mac = mac.lower()
-            ip = None
-
-            if not ip or nic_params.get("always_use_tcpdump") == "yes":
-                # Get the IP address from the cache
-                ip = self.address_cache.get(mac)
-                if not ip:
-                    logging.debug("Could not find IP address for MAC address: "
-                                  "%s" % mac)
-                    return None
-                # Make sure the IP address is assigned to this guest
-                macs = [self.get_mac_address(i) for i in range(len(nics))]
-                if not kvm_utils.verify_ip_address_ownership(ip, macs):
-                    logging.debug("Could not verify MAC-IP address mapping: "
-                                  "%s ---> %s" % (mac, ip))
-                    return None
+            # Get the IP address from the cache
+            ip = self.address_cache.get(mac)
+            if not ip:
+                logging.debug("Could not find IP address for MAC address: %s" %
+                              mac)
+                return None
+            # Make sure the IP address is assigned to this guest
+            macs = [self.get_mac_address(i) for i in range(len(nics))]
+            if not kvm_utils.verify_ip_address_ownership(ip, macs):
+                logging.debug("Could not verify MAC-IP address mapping: "
+                              "%s ---> %s" % (mac, ip))
+                return None
             return ip
         else:
             return "localhost"
@@ -985,12 +982,7 @@ class VM:
 
         @param nic_index: Index of the NIC
         """
-        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 kvm_utils.get_mac_address(self.instance, nic_index)
+        return kvm_utils.get_mac_address(self.instance, nic_index)
 
 
     def free_mac_address(self, nic_index=0):
diff --git a/client/tests/kvm/tests/physical_resources_check.py b/client/tests/kvm/tests/physical_resources_check.py
index 6c8e154..682c7b2 100644
--- a/client/tests/kvm/tests/physical_resources_check.py
+++ b/client/tests/kvm/tests/physical_resources_check.py
@@ -123,14 +123,9 @@ def run_physical_resources_check(test, params, env):
     found_mac_addresses = re.findall("macaddr=(\S+)", o)
     logging.debug("Found MAC adresses: %s" % found_mac_addresses)
 
-    nic_index = 0
-    for nic_name in kvm_utils.get_sub_dict_names(params, "nics"):
-        nic_params = kvm_utils.get_sub_dict(params, nic_name)
-        if "address_index" in nic_params:
-            mac, ip = kvm_utils.get_mac_ip_pair_from_dict(nic_params)
-        else:
-            mac = vm.get_mac_address(nic_index)
-            nic_index += 1
+    num_nics = len(kvm_utils.get_sub_dict_names(params, "nics"))
+    for nic_index in range(num_nics):
+        mac = vm.get_mac_address(nic_index)
         if not string.lower(mac) in found_mac_addresses:
             n_fail += 1
             logging.error("MAC address mismatch:")
diff --git a/client/tests/kvm/tests/stress_boot.py b/client/tests/kvm/tests/stress_boot.py
index 0d3ed07..b7916b4 100644
--- a/client/tests/kvm/tests/stress_boot.py
+++ b/client/tests/kvm/tests/stress_boot.py
@@ -28,7 +28,6 @@ def run_stress_boot(tests, params, env):
 
     num = 2
     sessions = [session]
-    address_index = int(params.get("clone_address_index_base", 10))
 
     # boot the VMs
     while num <= int(params.get("max_vms")):
@@ -36,7 +35,6 @@ def run_stress_boot(tests, params, env):
             # clone vm according to the first one
             vm_name = "vm" + str(num)
             vm_params = vm.get_params().copy()
-            vm_params["address_index"] = str(address_index)
             curr_vm = vm.clone(vm_name, vm_params)
             kvm_utils.env_register_vm(env, vm_name, curr_vm)
             logging.info("Booting guest #%d" % num)
@@ -56,7 +54,6 @@ def run_stress_boot(tests, params, env):
                 if se.get_command_status(params.get("alive_test_cmd")) != 0:
                     raise error.TestFail("Session #%d is not responsive" % i)
             num += 1
-            address_index += 1
 
         except (error.TestFail, OSError):
             for se in sessions:
diff --git a/client/tests/kvm/tests_base.cfg.sample b/client/tests/kvm/tests_base.cfg.sample
index 769d750..5bca544 100644
--- a/client/tests/kvm/tests_base.cfg.sample
+++ b/client/tests/kvm/tests_base.cfg.sample
@@ -54,7 +54,6 @@ guest_port_remote_shell = 22
 nic_mode = user
 #nic_mode = tap
 nic_script = scripts/qemu-ifup
-#address_index = 0
 run_tcpdump = yes
 
 # Misc
@@ -274,7 +273,6 @@ variants:
         type = stress_boot
         max_vms = 5    
         alive_test_cmd = uname -a
-        clone_address_index_base = 10
         login_timeout = 240
         kill_vm = yes
         kill_vm_vm1 = no
-- 
1.5.5.6

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [KVM-AUTOTEST PATCH 4/7] KVM test: get_ifname(): use self.instance instead of self.vnc_port
  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     ` 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-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
  1 sibling, 1 reply; 9+ messages in thread
From: Michael Goldish @ 2010-10-24 11:01 UTC (permalink / raw)
  To: autotest, kvm

The vnc_port attribute is only unique among VMs that use a VNC display.  Other
VMs don't bother to look for a free VNC port and don't occupy one, so vnc_port
can't be considered unique.  The last 11 characters of self.instance make up a
fairly unique string which can be used instead.  (The whole string can't be
used because it exceeds 15 characters.)

Signed-off-by: Michael Goldish <mgoldish@redhat.com>
---
 client/tests/kvm/kvm_vm.py |    8 ++------
 1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index 2db916f..a5c110c 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -959,10 +959,7 @@ class VM:
 
     def get_ifname(self, nic_index=0):
         """
-        Return the ifname of tap device for the guest nic.
-
-        The vnc_port is unique for each VM, nic_index is unique for each nic
-        of one VM, it can avoid repeated ifname.
+        Return the ifname of a tap device associated with a NIC.
 
         @param nic_index: Index of the NIC
         """
@@ -972,8 +969,7 @@ class VM:
         if nic_params.get("nic_ifname"):
             return nic_params.get("nic_ifname")
         else:
-            return "%s_%s_%s" % (nic_params.get("nic_model"),
-                                 nic_index, self.vnc_port)
+            return "t%d-%s" % (nic_index, self.instance[-11:])
 
 
     def get_mac_address(self, nic_index=0):
-- 
1.5.5.6

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [KVM-AUTOTEST PATCH 5/7] KVM test: kvm_monitor.py: replace MonitorSendError with MonitorSocketError
  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       ` Michael Goldish
  2010-10-24 11:01         ` [KVM-AUTOTEST PATCH 6/7] KVM test: refactor migration code Michael Goldish
  0 siblings, 1 reply; 9+ messages in thread
From: Michael Goldish @ 2010-10-24 11:01 UTC (permalink / raw)
  To: autotest, kvm; +Cc: Michael Goldish

- Replace MonitorSendError with MonitorSocketError.
- Embed socket.error messages in MonitorSocketError messages.
- Catch exceptions raised while receiving data (in addition to those raised
  while sending data).

Signed-off-by: Michael Goldish <mgoldish@redhat.com>
---
 client/tests/kvm/kvm_monitor.py |   35 ++++++++++++++++++++---------------
 1 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/client/tests/kvm/kvm_monitor.py b/client/tests/kvm/kvm_monitor.py
index 7047850..e0365cd 100644
--- a/client/tests/kvm/kvm_monitor.py
+++ b/client/tests/kvm/kvm_monitor.py
@@ -21,7 +21,7 @@ class MonitorConnectError(MonitorError):
     pass
 
 
-class MonitorSendError(MonitorError):
+class MonitorSocketError(MonitorError):
     pass
 
 
@@ -111,7 +111,11 @@ class Monitor:
     def _recvall(self):
         s = ""
         while self._data_available():
-            data = self._socket.recv(1024)
+            try:
+                data = self._socket.recv(1024)
+            except socket.error, (errno, msg):
+                raise MonitorSocketError("Could not receive data from monitor "
+                                         "(%s)" % msg)
             if not data:
                 break
             s += data
@@ -164,7 +168,7 @@ class HumanMonitor(Monitor):
         s = ""
         end_time = time.time() + timeout
         while self._data_available(end_time - time.time()):
-            data = self._socket.recv(1024)
+            data = self._recvall()
             if not data:
                 break
             s += data
@@ -182,7 +186,7 @@ class HumanMonitor(Monitor):
 
         @param cmd: Command to send
         @raise MonitorLockError: Raised if the lock cannot be acquired
-        @raise MonitorSendError: Raised if the command cannot be sent
+        @raise MonitorSocketError: Raised if a socket error occurs
         """
         if not self._acquire_lock(20):
             raise MonitorLockError("Could not acquire exclusive lock to send "
@@ -191,9 +195,9 @@ class HumanMonitor(Monitor):
         try:
             try:
                 self._socket.sendall(cmd + "\n")
-            except socket.error:
-                raise MonitorSendError("Could not send monitor command '%s'" %
-                                       cmd)
+            except socket.error, (errno, msg):
+                raise MonitorSocketError("Could not send monitor command '%s' "
+                                         "(%s)" % (cmd, msg))
 
         finally:
             self._lock.release()
@@ -209,7 +213,7 @@ class HumanMonitor(Monitor):
         @param timeout: Time duration to wait for the (qemu) prompt to return
         @return: Output received from the monitor
         @raise MonitorLockError: Raised if the lock cannot be acquired
-        @raise MonitorSendError: Raised if the command cannot be sent
+        @raise MonitorSocketError: Raised if a socket error occurs
         @raise MonitorProtocolError: Raised if the (qemu) prompt cannot be
                 found after sending the command
         """
@@ -465,12 +469,13 @@ class QMPMonitor(Monitor):
         Send raw data without waiting for response.
 
         @param data: Data to send
-        @raise MonitorSendError: Raised if the data cannot be sent
+        @raise MonitorSocketError: Raised if a socket error occurs
         """
         try:
             self._socket.sendall(data)
-        except socket.error:
-            raise MonitorSendError("Could not send data: %r" % data)
+        except socket.error, (errno, msg):
+            raise MonitorSocketError("Could not send data: %r (%s)" %
+                                     (data, msg))
 
 
     def _get_response(self, id=None, timeout=20):
@@ -505,7 +510,7 @@ class QMPMonitor(Monitor):
         @param timeout: Time duration to wait for response
         @return: The response received
         @raise MonitorLockError: Raised if the lock cannot be acquired
-        @raise MonitorSendError: Raised if the command cannot be sent
+        @raise MonitorSocketError: Raised if a socket error occurs
         @raise MonitorProtocolError: Raised if no response is received
         @raise QMPCmdError: Raised if the response is an error message
                 (the exception's args are (cmd, args, data) where data is the
@@ -547,7 +552,7 @@ class QMPMonitor(Monitor):
         @param timeout: Time duration to wait for response
         @return: The response received
         @raise MonitorLockError: Raised if the lock cannot be acquired
-        @raise MonitorSendError: Raised if the command cannot be sent
+        @raise MonitorSocketError: Raised if a socket error occurs
         @raise MonitorProtocolError: Raised if no response is received
         """
         if not self._acquire_lock(20):
@@ -578,7 +583,7 @@ class QMPMonitor(Monitor):
         @param timeout: Time duration to wait for response
         @return: The response received
         @raise MonitorLockError: Raised if the lock cannot be acquired
-        @raise MonitorSendError: Raised if the command cannot be sent
+        @raise MonitorSocketError: Raised if a socket error occurs
         @raise MonitorProtocolError: Raised if no response is received
         """
         return self.cmd_raw(json.dumps(obj) + "\n")
@@ -597,7 +602,7 @@ class QMPMonitor(Monitor):
         @param timeout: Time duration to wait for response
         @return: The response received
         @raise MonitorLockError: Raised if the lock cannot be acquired
-        @raise MonitorSendError: Raised if the command cannot be sent
+        @raise MonitorSocketError: Raised if a socket error occurs
         @raise MonitorProtocolError: Raised if no response is received
         """
         return self.cmd_obj(self._build_cmd(cmd, args, id), timeout)
-- 
1.5.5.6


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [KVM-AUTOTEST PATCH 6/7] KVM test: refactor migration code
  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         ` 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
  0 siblings, 1 reply; 9+ messages in thread
From: Michael Goldish @ 2010-10-24 11:01 UTC (permalink / raw)
  To: autotest, kvm; +Cc: Michael Goldish

- Refactor migration code so that the '-incoming ...' strings are hardcoded
  only in a single location (kvm_test_utils.py).
- Wrap the removal of the gzipped state file in a finally: clause.
- Get rid of the 'for_migration' and 'extra_params' parameters of VM.create()
  and introduce 'migration_mode' and 'migration_exec_cmd'
- Remove unix socket files in VM.destroy() because they are created by
  VM.create() (as opposed to gzipped state files which are created in
  kvm_test_utils.migrate())

Signed-off-by: Michael Goldish <mgoldish@redhat.com>
---
 client/tests/kvm/kvm_preprocessing.py |   10 +--
 client/tests/kvm/kvm_test_utils.py    |  115 ++++++++++++++++-----------------
 client/tests/kvm/kvm_vm.py            |   44 ++++++------
 3 files changed, 80 insertions(+), 89 deletions(-)

diff --git a/client/tests/kvm/kvm_preprocessing.py b/client/tests/kvm/kvm_preprocessing.py
index e3de0b3..1ddf99b 100644
--- a/client/tests/kvm/kvm_preprocessing.py
+++ b/client/tests/kvm/kvm_preprocessing.py
@@ -59,14 +59,8 @@ def preprocess_vm(test, params, env, name):
         kvm_utils.env_register_vm(env, name, vm)
 
     start_vm = False
-    for_migration = False
 
-    if params.get("start_vm_for_migration") == "yes":
-        logging.debug("'start_vm_for_migration' specified; (re)starting VM "
-                      "with -incoming option...")
-        start_vm = True
-        for_migration = True
-    elif params.get("restart_vm") == "yes":
+    if params.get("restart_vm") == "yes":
         logging.debug("'restart_vm' specified; (re)starting VM...")
         start_vm = True
     elif params.get("start_vm") == "yes":
@@ -81,7 +75,7 @@ def preprocess_vm(test, params, env, name):
 
     if start_vm:
         # Start the VM (or restart it if it's already up)
-        if not vm.create(name, params, test.bindir, for_migration):
+        if not vm.create(name, params, test.bindir):
             raise error.TestError("Could not start VM")
     else:
         # Don't start the VM, just update its params
diff --git a/client/tests/kvm/kvm_test_utils.py b/client/tests/kvm/kvm_test_utils.py
index d9c5a6e..1bb8920 100644
--- a/client/tests/kvm/kvm_test_utils.py
+++ b/client/tests/kvm/kvm_test_utils.py
@@ -167,79 +167,76 @@ def migrate(vm, env=None, mig_timeout=3600, mig_protocol="tcp",
             raise error.TestFail("Timeout expired while waiting for migration "
                                  "to finish")
 
+    dest_vm = vm.clone()
 
-    migration_file = os.path.join("/tmp/",
-                                  mig_protocol + time.strftime("%Y%m%d-%H%M%S"))
-    if mig_protocol == "tcp":
-        mig_extra_params = " -incoming tcp:0:%d"
-    elif mig_protocol == "unix":
-        mig_extra_params = " -incoming unix:%s"
-    elif mig_protocol == "exec":
+    if mig_protocol == "exec":
         # Exec is a little different from other migrate methods - first we
         # ask the monitor the migration, then the vm state is dumped to a
         # compressed file, then we start the dest vm with -incoming pointing
         # to it
-        mig_extra_params = " -incoming \"exec: gzip -c -d %s\"" % migration_file
-        uri = "\"exec:gzip -c > %s\"" % migration_file
-        vm.monitor.cmd("stop")
-        o = vm.monitor.migrate(uri)
-        wait_for_migration()
-
-    # 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, mac_source=vm):
-        raise error.TestError("Could not create dest VM")
-
-    try:
-        if mig_protocol == "tcp":
-            uri = "tcp:localhost:%d" % dest_vm.migration_port
-        elif mig_protocol == "unix":
-            uri = "unix:%s" % dest_vm.migration_file
+        try:
+            exec_file = "/tmp/exec-%s.gz" % kvm_utils.generate_random_string(8)
+            exec_cmd = "gzip -c -d %s" % exec_file
+            uri = '"exec:gzip -c > %s"' % exec_file
+            vm.monitor.cmd("stop")
+            vm.monitor.migrate(uri)
+            wait_for_migration()
 
-        if mig_protocol != "exec":
-            o = vm.monitor.migrate(uri)
+            if not dest_vm.create(migration_mode=mig_protocol,
+                                  migration_exec_cmd=exec_cmd, mac_source=vm):
+                raise error.TestError("Could not create dest VM")
+        finally:
+            logging.debug("Removing migration file %s", exec_file)
+            try:
+                os.remove(exec_file)
+            except OSError:
+                pass
+    else:
+        if not dest_vm.create(migration_mode=mig_protocol, mac_source=vm):
+            raise error.TestError("Could not create dest VM")
+        try:
+            if mig_protocol == "tcp":
+                uri = "tcp:localhost:%d" % dest_vm.migration_port
+            elif mig_protocol == "unix":
+                uri = "unix:%s" % dest_vm.migration_file
+            vm.monitor.migrate(uri)
 
-            if mig_protocol == "tcp" and mig_cancel:
+            if mig_cancel:
                 time.sleep(2)
-                o = vm.monitor.cmd("migrate_cancel")
+                vm.monitor.cmd("migrate_cancel")
                 if not kvm_utils.wait_for(mig_cancelled, 60, 2, 2,
-                                          "Waiting for migration cancel"):
-                    raise error.TestFail("Fail to cancel migration")
+                                          "Waiting for migration "
+                                          "cancellation"):
+                    raise error.TestFail("Failed to cancel migration")
                 dest_vm.destroy(gracefully=False)
                 return vm
+            else:
+                wait_for_migration()
+        except:
+            dest_vm.destroy()
+            raise
+
+    # Report migration status
+    if mig_succeeded():
+        logging.info("Migration finished successfully")
+    elif mig_failed():
+        raise error.TestFail("Migration failed")
+    else:
+        raise error.TestFail("Migration ended with unknown status")
 
-            wait_for_migration()
-
-        # Report migration status
-        if mig_succeeded():
-            logging.info("Migration finished successfully")
-        elif mig_failed():
-            raise error.TestFail("Migration failed")
-        else:
-            raise error.TestFail("Migration ended with unknown status")
-
-        o = dest_vm.monitor.info("status")
-        if "paused" in o:
-            logging.debug("Destination VM is paused, resuming it...")
-            dest_vm.monitor.cmd("cont")
-
-        if os.path.exists(migration_file):
-            logging.debug("Removing migration file %s", migration_file)
-            os.remove(migration_file)
-
-        # Kill the source VM
-        vm.destroy(gracefully=False)
+    if "paused" in dest_vm.monitor.info("status"):
+        logging.debug("Destination VM is paused, resuming it...")
+        dest_vm.monitor.cmd("cont")
 
-        # Replace the source VM with the new cloned VM
-        if env is not None:
-            kvm_utils.env_register_vm(env, vm.name, dest_vm)
+    # Kill the source VM
+    vm.destroy(gracefully=False)
 
-        # Return the new cloned VM
-        return dest_vm
+    # Replace the source VM with the new cloned VM
+    if env is not None:
+        kvm_utils.env_register_vm(env, vm.name, dest_vm)
 
-    except:
-        dest_vm.destroy()
-        raise
+    # Return the new cloned VM
+    return dest_vm
 
 
 def get_time(session, time_command, time_filter_re, time_format):
diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index a5c110c..a860437 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -507,23 +507,20 @@ class VM:
         return qemu_cmd
 
 
-    def create(self, name=None, params=None, root_dir=None,
-               for_migration=False, timeout=5.0, extra_params=None,
-               mac_source=None):
+    def create(self, name=None, params=None, root_dir=None, timeout=5.0,
+               migration_mode=None, migration_exec_cmd=None, mac_source=None):
         """
         Start the VM by running a qemu command.
-        All parameters are optional. The following applies to all parameters
-        but for_migration: If a parameter is not supplied, the corresponding
-        value stored in the class attributes is used, and if it is supplied,
-        it is stored for later use.
+        All parameters are optional. If name, params or root_dir are not
+        supplied, the respective values stored as class attributes are used.
 
         @param name: The name of the object
         @param params: A dict containing VM params
         @param root_dir: Base directory for relative filenames
-        @param for_migration: If True, start the VM with the -incoming
-        option
-        @param extra_params: extra params for qemu command.e.g -incoming option
-        Please use this parameter instead of for_migration.
+        @param migration_mode: If supplied, start VM for incoming migration
+                using this protocol (either 'tcp', 'unix' or 'exec')
+        @param migration_exec_cmd: Command to embed in '-incoming "exec: ..."'
+                (e.g. 'gzip -c -d filename') if migration_mode is 'exec'
         @param mac_source: A VM object from which to copy MAC addresses. If not
                 specified, new addresses will be generated.
         """
@@ -655,17 +652,15 @@ class VM:
             # Make qemu command
             qemu_command = self.make_qemu_command()
 
-            # Enable migration support for VM by adding extra_params.
-            if extra_params is not None:
-                if " -incoming tcp:0:%d" == extra_params:
-                    self.migration_port = kvm_utils.find_free_port(5200, 6000)
-                    qemu_command += extra_params % self.migration_port
-                elif " -incoming unix:%s" == extra_params:
-                    self.migration_file = os.path.join("/tmp/", "unix-" +
-                                          time.strftime("%Y%m%d-%H%M%S"))
-                    qemu_command += extra_params % self.migration_file
-                else:
-                    qemu_command += extra_params
+            # Add migration parameters if required
+            if migration_mode == "tcp":
+                self.migration_port = kvm_utils.find_free_port(5200, 6000)
+                qemu_command += " -incoming tcp:0:%d" % self.migration_port
+            elif migration_mode == "unix":
+                self.migration_file = "/tmp/migration-unix-%s" % self.instance
+                qemu_command += " -incoming unix:%s" % self.migration_file
+            elif migration_mode == "exec":
+                qemu_command += ' -incoming "exec:%s"' % migration_exec_cmd
 
             logging.debug("Running qemu command:\n%s", qemu_command)
             self.process = kvm_subprocess.run_bg(qemu_command, None,
@@ -826,6 +821,11 @@ class VM:
                     os.unlink(f)
                 except OSError:
                     pass
+            if hasattr(self, "migration_file"):
+                try:
+                    os.unlink(self.migration_file)
+                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)
-- 
1.5.5.6


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [KVM-AUTOTEST PATCH 7/7] [RFC] KVM test: migrate_cancel: allow for alternative spellings of 'cancelled'
  2010-10-24 11:01         ` [KVM-AUTOTEST PATCH 6/7] KVM test: refactor migration code Michael Goldish
@ 2010-10-24 11:01           ` Michael Goldish
  2010-10-26 13:08             ` Lucas Meneghel Rodrigues
  0 siblings, 1 reply; 9+ messages in thread
From: Michael Goldish @ 2010-10-24 11:01 UTC (permalink / raw)
  To: autotest, kvm

Is there any chance someone will decide to switch over to the American spelling?

Signed-off-by: Michael Goldish <mgoldish@redhat.com>
---
 client/tests/kvm/kvm_test_utils.py |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/client/tests/kvm/kvm_test_utils.py b/client/tests/kvm/kvm_test_utils.py
index 1bb8920..a3b182b 100644
--- a/client/tests/kvm/kvm_test_utils.py
+++ b/client/tests/kvm/kvm_test_utils.py
@@ -157,9 +157,11 @@ def migrate(vm, env=None, mig_timeout=3600, mig_protocol="tcp",
     def mig_cancelled():
         o = vm.monitor.info("migrate")
         if isinstance(o, str):
-            return "Migration status: cancelled" in o
+            return ("Migration status: cancelled" in o or
+                    "Migration status: canceled" in o)
         else:
-            return o.get("status") == "cancelled"
+            return (o.get("status") == "cancelled" or
+                    o.get("status") == "canceled")
 
     def wait_for_migration():
         if not kvm_utils.wait_for(mig_finished, mig_timeout, 2, 2,
-- 
1.5.5.6

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [KVM-AUTOTEST PATCH 3/7] [RFC] KVM test: remove all code related to the old MAC address pool method
  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-26 12:56     ` Lucas Meneghel Rodrigues
  1 sibling, 0 replies; 9+ messages in thread
From: Lucas Meneghel Rodrigues @ 2010-10-26 12:56 UTC (permalink / raw)
  To: Michael Goldish; +Cc: autotest, kvm

On Sun, 2010-10-24 at 13:01 +0200, Michael Goldish wrote:
> This patch removes all code related to the old manual method
> (address_pools.cfg).
> 
> Note that now running in TAP mode requires an external DHCP server that accepts
> *any* MAC address, because MAC addresses are randomly generated and cannot be
> manually configured.

Yes, this patch looks good. I don't think there are much DHCP servers
that do not accept any MAC address. It might be good to document this
information on the wiki though.

> Signed-off-by: Michael Goldish <mgoldish@redhat.com>
> ---
>  client/tests/kvm/kvm_utils.py                      |  159 --------------------
>  client/tests/kvm/kvm_vm.py                         |   34 ++---
>  client/tests/kvm/tests/physical_resources_check.py |   11 +-
>  client/tests/kvm/tests/stress_boot.py              |    3 -
>  client/tests/kvm/tests_base.cfg.sample             |    2 -
>  5 files changed, 16 insertions(+), 193 deletions(-)
> 
> diff --git a/client/tests/kvm/kvm_utils.py b/client/tests/kvm/kvm_utils.py
> index 778637d..f749f8d 100644
> --- a/client/tests/kvm/kvm_utils.py
> +++ b/client/tests/kvm/kvm_utils.py
> @@ -203,165 +203,6 @@ def get_mac_address(vm_instance, nic_index):
>      return mac
>  
> 
> -def mac_str_to_int(addr):
> -    """
> -    Convert MAC address string to integer.
> -
> -    @param addr: String representing the MAC address.
> -    """
> -    return sum(int(s, 16) * 256 ** i
> -               for i, s in enumerate(reversed(addr.split(":"))))
> -
> -
> -def mac_int_to_str(addr):
> -    """
> -    Convert MAC address integer to string.
> -
> -    @param addr: Integer representing the MAC address.
> -    """
> -    return ":".join("%02x" % (addr >> 8 * i & 0xFF)
> -                    for i in reversed(range(6)))
> -
> -
> -def ip_str_to_int(addr):
> -    """
> -    Convert IP address string to integer.
> -
> -    @param addr: String representing the IP address.
> -    """
> -    return sum(int(s) * 256 ** i
> -               for i, s in enumerate(reversed(addr.split("."))))
> -
> -
> -def ip_int_to_str(addr):
> -    """
> -    Convert IP address integer to string.
> -
> -    @param addr: Integer representing the IP address.
> -    """
> -    return ".".join(str(addr >> 8 * i & 0xFF)
> -                    for i in reversed(range(4)))
> -
> -
> -def offset_mac(base, offset):
> -    """
> -    Add offset to a given MAC address.
> -
> -    @param base: String representing a MAC address.
> -    @param offset: Offset to add to base (integer)
> -    @return: A string representing the offset MAC address.
> -    """
> -    return mac_int_to_str(mac_str_to_int(base) + offset)
> -
> -
> -def offset_ip(base, offset):
> -    """
> -    Add offset to a given IP address.
> -
> -    @param base: String representing an IP address.
> -    @param offset: Offset to add to base (integer)
> -    @return: A string representing the offset IP address.
> -    """
> -    return ip_int_to_str(ip_str_to_int(base) + offset)
> -
> -
> -def get_mac_ip_pair_from_dict(dict):
> -    """
> -    Fetch a MAC-IP address pair from dict and return it.
> -
> -    The parameters in dict are expected to conform to a certain syntax.
> -    Typical usage may be:
> -
> -    address_ranges = r1 r2 r3
> -
> -    address_range_base_mac_r1 = 55:44:33:22:11:00
> -    address_range_base_ip_r1 = 10.0.0.0
> -    address_range_size_r1 = 16
> -
> -    address_range_base_mac_r2 = 55:44:33:22:11:40
> -    address_range_base_ip_r2 = 10.0.0.60
> -    address_range_size_r2 = 25
> -
> -    address_range_base_mac_r3 = 55:44:33:22:12:10
> -    address_range_base_ip_r3 = 10.0.1.20
> -    address_range_size_r3 = 230
> -
> -    address_index = 0
> -
> -    All parameters except address_index specify a MAC-IP address pool.  The
> -    pool consists of several MAC-IP address ranges.
> -    address_index specified the index of the desired MAC-IP pair from the pool.
> -
> -    @param dict: The dictionary from which to fetch the addresses.
> -    """
> -    index = int(dict.get("address_index", 0))
> -    for mac_range_name in get_sub_dict_names(dict, "address_ranges"):
> -        mac_range_params = get_sub_dict(dict, mac_range_name)
> -        mac_base = mac_range_params.get("address_range_base_mac")
> -        ip_base = mac_range_params.get("address_range_base_ip")
> -        size = int(mac_range_params.get("address_range_size", 1))
> -        if index < size:
> -            return (mac_base and offset_mac(mac_base, index),
> -                    ip_base and offset_ip(ip_base, index))
> -        index -= size
> -    return (None, None)
> -
> -
> -def get_sub_pool(dict, piece, num_pieces):
> -    """
> -    Split a MAC-IP pool and return a single requested piece.
> -
> -    For example, get_sub_pool(dict, 0, 3) will split the pool in 3 pieces and
> -    return a dict representing the first piece.
> -
> -    @param dict: A dict that contains pool parameters.
> -    @param piece: The index of the requested piece.  Should range from 0 to
> -        num_pieces - 1.
> -    @param num_pieces: The total number of pieces.
> -    @return: A copy of dict, modified to describe the requested sub-pool.
> -    """
> -    range_dicts = [get_sub_dict(dict, name) for name in
> -                   get_sub_dict_names(dict, "address_ranges")]
> -    if not range_dicts:
> -        return dict
> -    ranges = [[d.get("address_range_base_mac"),
> -               d.get("address_range_base_ip"),
> -               int(d.get("address_range_size", 1))] for d in range_dicts]
> -    total_size = sum(r[2] for r in ranges)
> -    base = total_size * piece / num_pieces
> -    size = total_size * (piece + 1) / num_pieces - base
> -
> -    # Find base of current sub-pool
> -    for i in range(len(ranges)):
> -        r = ranges[i]
> -        if base < r[2]:
> -            r[0] = r[0] and offset_mac(r[0], base)
> -            r[1] = r[1] and offset_ip(r[1], base)
> -            r[2] -= base
> -            break
> -        base -= r[2]
> -
> -    # Collect ranges up to end of current sub-pool
> -    new_ranges = []
> -    for i in range(i, len(ranges)):
> -        r = ranges[i]
> -        new_ranges.append(r)
> -        if size <= r[2]:
> -            r[2] = size
> -            break
> -        size -= r[2]
> -
> -    # Write new dict
> -    new_dict = dict.copy()
> -    new_dict["address_ranges"] = " ".join("r%d" % i for i in
> -                                          range(len(new_ranges)))
> -    for i in range(len(new_ranges)):
> -        new_dict["address_range_base_mac_r%d" % i] = new_ranges[i][0]
> -        new_dict["address_range_base_ip_r%d" % i] = new_ranges[i][1]
> -        new_dict["address_range_size_r%d" % i] = new_ranges[i][2]
> -    return new_dict
> -
> -
>  def verify_ip_address_ownership(ip, macs, timeout=10.0):
>      """
>      Use arping and the ARP cache to make sure a given IP address belongs to one
> diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
> index f3e803b..2db916f 100755
> --- a/client/tests/kvm/kvm_vm.py
> +++ b/client/tests/kvm/kvm_vm.py
> @@ -920,21 +920,18 @@ class VM:
>                  logging.debug("MAC address unavailable")
>                  return None
>              mac = mac.lower()
> -            ip = None
> -
> -            if not ip or nic_params.get("always_use_tcpdump") == "yes":
> -                # Get the IP address from the cache
> -                ip = self.address_cache.get(mac)
> -                if not ip:
> -                    logging.debug("Could not find IP address for MAC address: "
> -                                  "%s" % mac)
> -                    return None
> -                # Make sure the IP address is assigned to this guest
> -                macs = [self.get_mac_address(i) for i in range(len(nics))]
> -                if not kvm_utils.verify_ip_address_ownership(ip, macs):
> -                    logging.debug("Could not verify MAC-IP address mapping: "
> -                                  "%s ---> %s" % (mac, ip))
> -                    return None
> +            # Get the IP address from the cache
> +            ip = self.address_cache.get(mac)
> +            if not ip:
> +                logging.debug("Could not find IP address for MAC address: %s" %
> +                              mac)
> +                return None
> +            # Make sure the IP address is assigned to this guest
> +            macs = [self.get_mac_address(i) for i in range(len(nics))]
> +            if not kvm_utils.verify_ip_address_ownership(ip, macs):
> +                logging.debug("Could not verify MAC-IP address mapping: "
> +                              "%s ---> %s" % (mac, ip))
> +                return None
>              return ip
>          else:
>              return "localhost"
> @@ -985,12 +982,7 @@ class VM:
>  
>          @param nic_index: Index of the NIC
>          """
> -        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 kvm_utils.get_mac_address(self.instance, nic_index)
> +        return kvm_utils.get_mac_address(self.instance, nic_index)
>  
> 
>      def free_mac_address(self, nic_index=0):
> diff --git a/client/tests/kvm/tests/physical_resources_check.py b/client/tests/kvm/tests/physical_resources_check.py
> index 6c8e154..682c7b2 100644
> --- a/client/tests/kvm/tests/physical_resources_check.py
> +++ b/client/tests/kvm/tests/physical_resources_check.py
> @@ -123,14 +123,9 @@ def run_physical_resources_check(test, params, env):
>      found_mac_addresses = re.findall("macaddr=(\S+)", o)
>      logging.debug("Found MAC adresses: %s" % found_mac_addresses)
>  
> -    nic_index = 0
> -    for nic_name in kvm_utils.get_sub_dict_names(params, "nics"):
> -        nic_params = kvm_utils.get_sub_dict(params, nic_name)
> -        if "address_index" in nic_params:
> -            mac, ip = kvm_utils.get_mac_ip_pair_from_dict(nic_params)
> -        else:
> -            mac = vm.get_mac_address(nic_index)
> -            nic_index += 1
> +    num_nics = len(kvm_utils.get_sub_dict_names(params, "nics"))
> +    for nic_index in range(num_nics):
> +        mac = vm.get_mac_address(nic_index)
>          if not string.lower(mac) in found_mac_addresses:
>              n_fail += 1
>              logging.error("MAC address mismatch:")
> diff --git a/client/tests/kvm/tests/stress_boot.py b/client/tests/kvm/tests/stress_boot.py
> index 0d3ed07..b7916b4 100644
> --- a/client/tests/kvm/tests/stress_boot.py
> +++ b/client/tests/kvm/tests/stress_boot.py
> @@ -28,7 +28,6 @@ def run_stress_boot(tests, params, env):
>  
>      num = 2
>      sessions = [session]
> -    address_index = int(params.get("clone_address_index_base", 10))
>  
>      # boot the VMs
>      while num <= int(params.get("max_vms")):
> @@ -36,7 +35,6 @@ def run_stress_boot(tests, params, env):
>              # clone vm according to the first one
>              vm_name = "vm" + str(num)
>              vm_params = vm.get_params().copy()
> -            vm_params["address_index"] = str(address_index)
>              curr_vm = vm.clone(vm_name, vm_params)
>              kvm_utils.env_register_vm(env, vm_name, curr_vm)
>              logging.info("Booting guest #%d" % num)
> @@ -56,7 +54,6 @@ def run_stress_boot(tests, params, env):
>                  if se.get_command_status(params.get("alive_test_cmd")) != 0:
>                      raise error.TestFail("Session #%d is not responsive" % i)
>              num += 1
> -            address_index += 1
>  
>          except (error.TestFail, OSError):
>              for se in sessions:
> diff --git a/client/tests/kvm/tests_base.cfg.sample b/client/tests/kvm/tests_base.cfg.sample
> index 769d750..5bca544 100644
> --- a/client/tests/kvm/tests_base.cfg.sample
> +++ b/client/tests/kvm/tests_base.cfg.sample
> @@ -54,7 +54,6 @@ guest_port_remote_shell = 22
>  nic_mode = user
>  #nic_mode = tap
>  nic_script = scripts/qemu-ifup
> -#address_index = 0
>  run_tcpdump = yes
>  
>  # Misc
> @@ -274,7 +273,6 @@ variants:
>          type = stress_boot
>          max_vms = 5    
>          alive_test_cmd = uname -a
> -        clone_address_index_base = 10
>          login_timeout = 240
>          kill_vm = yes
>          kill_vm_vm1 = no

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [KVM-AUTOTEST PATCH 7/7] [RFC] KVM test: migrate_cancel: allow for alternative spellings of 'cancelled'
  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
  0 siblings, 0 replies; 9+ messages in thread
From: Lucas Meneghel Rodrigues @ 2010-10-26 13:08 UTC (permalink / raw)
  To: Michael Goldish; +Cc: autotest, kvm

On Sun, 2010-10-24 at 13:01 +0200, Michael Goldish wrote:
> Is there any chance someone will decide to switch over to the American spelling?

This is mainly harmless. Even if the switch won't happen, this is
absolutely fine, thanks!

> Signed-off-by: Michael Goldish <mgoldish@redhat.com>
> ---
>  client/tests/kvm/kvm_test_utils.py |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/client/tests/kvm/kvm_test_utils.py b/client/tests/kvm/kvm_test_utils.py
> index 1bb8920..a3b182b 100644
> --- a/client/tests/kvm/kvm_test_utils.py
> +++ b/client/tests/kvm/kvm_test_utils.py
> @@ -157,9 +157,11 @@ def migrate(vm, env=None, mig_timeout=3600, mig_protocol="tcp",
>      def mig_cancelled():
>          o = vm.monitor.info("migrate")
>          if isinstance(o, str):
> -            return "Migration status: cancelled" in o
> +            return ("Migration status: cancelled" in o or
> +                    "Migration status: canceled" in o)
>          else:
> -            return o.get("status") == "cancelled"
> +            return (o.get("status") == "cancelled" or
> +                    o.get("status") == "canceled")
>  
>      def wait_for_migration():
>          if not kvm_utils.wait_for(mig_finished, mig_timeout, 2, 2,

^ permalink raw reply	[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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox