[KVM-AUTOTEST PATCH 6/7] KVM test: refactor migration code

[Michael Goldish <mgoldish@redhat.com>]

- 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