Merge pull request #2584 from PaulYuuu/kvm_module_reload

env_process: Handle the kvm_probe module together

virttest/arch.py

@@ -89,4 +89,9 @@ def get_kvm_module_list():
         host_cpu_type = cpu.get_cpu_vendor_name()
         return ["kvm", "kvm-%s" % host_cpu_type]
     elif ARCH in ('ppc64', 'ppc64le'):
+        # FIXME: Please correct it if anyone still want to use KVM-PR mode
+        return ["kvm", "kvm-hv"]
+    elif ARCH in ('s390', 's390x'):
         return ["kvm"]
+    elif ARCH == "aarch64":
+        return []

virttest/env_process.py

@@ -50,6 +50,7 @@ from virttest import utils_package
 from virttest import utils_qemu
 from virttest import migration
 from virttest import utils_kernel_module
+from virttest import arch
 from virttest.utils_version import VersionInterval
 from virttest.staging import service
 
@@ -81,8 +82,8 @@ preprocess_vm_on_hook = None
 postprocess_vm_on_hook = None
 postprocess_vm_off_hook = None
 
-#: Object to handle kvm module reloads with certain parameters
-KVM_MODULE_HANDLER = None
+#: A list to handle kvm and kvm_probe modules reload with certain parameters
+KVM_MODULE_HANDLERS = []
 
 #: QEMU version regex.  Attempts to extract the simple and extended version
 #: information from the output produced by `qemu -version`
@@ -1171,10 +1172,18 @@ def preprocess(test, params, env):
                     env["cpu_driver"] = cpu_driver[0]
                     params["cpu_driver"] = env.get("cpu_driver")
 
-    kvm_module_params = params.get("kvm_module_parameters", "")
-    force_load = params.get("kvm_module_force_load", "no") == "yes"
-    global KVM_MODULE_HANDLER
-    KVM_MODULE_HANDLER = utils_kernel_module.reload("kvm", force_load, kvm_module_params)
+    global KVM_MODULE_HANDLERS
+    kvm_modules = arch.get_kvm_module_list()
+    for module in reversed(kvm_modules):
+        param_prefix = module if module == "kvm" else "kvm_probe"
+        module_force_load = params.get_boolean("%s_module_force_load"
+                                               % param_prefix)
+        module_parameters = params.get("%s_module_parameters" % param_prefix,
+                                       "")
+        module_handler = utils_kernel_module.reload(module, module_force_load,
+                                                    module_parameters)
+        if module_handler is not None:
+            KVM_MODULE_HANDLERS.append(module_handler)
 
     version_info = {}
     # Get the KVM kernel module version
@@ -1719,8 +1728,8 @@ def postprocess(test, params, env):
             err += "\nTHP cleanup: %s" % str(details).replace('\\n', '\n  ')
             logging.error(details)
 
-    if KVM_MODULE_HANDLER:
-        KVM_MODULE_HANDLER.restore()
+    for kvm_module in KVM_MODULE_HANDLERS:
+        kvm_module.restore()
 
     if params.get("setup_ksm") == "yes":
         try:

virttest/unittests/test_utils_kernel_module.py

@@ -19,6 +19,36 @@ some_module_params = "%s=%s" % (some_module_param, some_module_val)
 getstatusoutput_ok = mock.Mock(return_value=(0, ""))
 
 
+@mock.patch.object(utils_kernel_module.os, 'listdir', return_value=[some_module_param])
+@mock.patch.object(utils_kernel_module, 'open', mock.mock_open(read_data=some_module_val + '\n'))
+@mock.patch.object(utils_kernel_module.process, 'getstatusoutput', getstatusoutput_ok)
+class TestUnloadModule(unittest.TestCase):
+    """
+    Tests the reload_module method
+    """
+
+    def tearDown(self):
+        getstatusoutput_ok.reset_mock()
+
+    def assertUnloaded(self):
+        getstatusoutput_ok.assert_called_once()
+
+    def assertNoUnload(self):
+        getstatusoutput_ok.assert_not_called()
+
+    @mock.patch.object(utils_kernel_module.os.path, 'exists', return_value=True)
+    def test_tc1(self, *mocks):
+        self.handler = utils_kernel_module.KernelModuleHandler(some_module_name)
+        self.handler.unload_module()
+        self.assertUnloaded()
+
+    @mock.patch.object(utils_kernel_module.os.path, 'exists', return_value=False)
+    def test_tc2(self, *mocks):
+        self.handler = utils_kernel_module.KernelModuleHandler(some_module_name)
+        self.handler.unload_module()
+        self.assertNoUnload()
+
+
 @mock.patch.object(utils_kernel_module.os, 'listdir', return_value=[some_module_param])
 @mock.patch.object(utils_kernel_module, 'open', mock.mock_open(read_data=some_module_val + '\n'))
 @mock.patch.object(utils_kernel_module.process, 'getstatusoutput', getstatusoutput_ok)
@@ -132,18 +162,15 @@ class TestRestore(unittest.TestCase):
     def tearDown(self):
         getstatusoutput_ok.reset_mock()
 
-    def assertRestored(self, params):
+    def assertRestored(self, orig_params, cur_params):
         self.assertTrue(getstatusoutput_ok.called)
-        cmd = getstatusoutput_ok.call_args[0][0]
-        self.assertTrue(params in cmd)
+        self.assertEqual(orig_params, cur_params)
 
     def assertNoRestore(self):
-        self.assertFalse(getstatusoutput_ok.called)
+        getstatusoutput_ok.assert_not_called()
 
     def assertUnloaded(self):
-        self.assertTrue(getstatusoutput_ok.called)
-        cmd = getstatusoutput_ok.call_args[0][0]
-        self.assertTrue("modprobe -r" in cmd)
+        getstatusoutput_ok.assert_called_once()
 
     @mock.patch.object(utils_kernel_module.os.path, 'exists', return_value=True)
     def test_tc1(self, *mocks):
@@ -151,7 +178,8 @@ class TestRestore(unittest.TestCase):
         orig_config = self.handler.config_backup
         self.handler.reload_module(True, "key=value")
         self.handler.restore()
-        self.assertRestored(orig_config)
+        cur_config = self.handler.current_config
+        self.assertRestored(orig_config, cur_config)
 
     @mock.patch.object(utils_kernel_module.os.path, 'exists', return_value=True)
     def test_tc1_reload_twice(self, *mocks):
@@ -160,7 +188,8 @@ class TestRestore(unittest.TestCase):
         self.handler.reload_module(True, "key=value")
         self.handler.reload_module(True, "key1=value1")
         self.handler.restore()
-        self.assertRestored(orig_config)
+        cur_config = self.handler.current_config
+        self.assertRestored(orig_config, cur_config)
 
     @mock.patch.object(utils_kernel_module.os.path, 'exists', return_value=True)
     def test_tc2(self, *mocks):

virttest/utils_kernel_module.py

@@ -20,10 +20,42 @@ import os
 import logging
 
 from avocado.utils import process
-from avocado.core import exceptions
 
 
-def reload(module_name, force, params):
+class KernelModuleError(Exception):
+
+    def __init__(self, handler, module_name, reason):
+        self.handler = handler
+        self.module_name = module_name
+        self.reason = reason
+
+    def __str__(self):
+        return "Couldn't %s module %s: %s" % (self.handler, self.module_name,
+                                              self.reason)
+
+
+class KernelModuleUnloadError(KernelModuleError):
+
+    def __init__(self, module_name, reason):
+        super(KernelModuleUnloadError, self).__init__("unload", module_name,
+                                                      reason)
+
+
+class KernelModuleReloadError(KernelModuleError):
+
+    def __init__(self, module_name, reason):
+        super(KernelModuleReloadError, self).__init__("reload", module_name,
+                                                      reason)
+
+
+class KernelModuleRestoreError(KernelModuleError):
+
+    def __init__(self, module_name, reason):
+        super(KernelModuleRestoreError, self).__init__("restore", module_name,
+                                                       reason)
+
+
+def reload(module_name, force, params=""):
     """
     Convenience method that creates a KernelModuleHandler instance
     and reloads the module only if any action will is required.
@@ -48,12 +80,28 @@ class KernelModuleHandler(object):
         """Create kernel module handler"""
 
         self._module_name = module_name
+        self._module_path = os.path.join('/sys/module/',
+                                         self._module_name.replace('-', '_'))
+        self._module_params_path = os.path.join(self._module_path, 'parameters')
+        self._module_holders_path = os.path.join(self._module_path, "holders")
         self._was_loaded = None
-        self._loaded_config = None
         self._config_backup = None
         self._backup_config()
 
-    def reload_module(self, force, params):
+    def unload_module(self):
+        """
+        Unload module and those modules that use it.
+
+        If there are some modules using this module, they are unloaded first.
+        """
+        if os.path.exists(self._module_path):
+            unload_cmd = 'rmmod ' + self._module_name
+            logging.debug("Unloading module: %s", unload_cmd)
+            status, output = process.getstatusoutput(unload_cmd)
+            if status:
+                raise KernelModuleUnloadError(self._module_name, output)
+
+    def reload_module(self, force, params=""):
         """
         Reload module with given parameters.
 
@@ -79,7 +127,7 @@ class KernelModuleHandler(object):
         :param params: parameters to load with, e.g. 'key1=param1 ...'
         """
 
-        current_config = self._get_serialized_config()
+        current_config = self.current_config
         if not force:
             do_not_load = False
             if (current_config and
@@ -97,20 +145,18 @@ class KernelModuleHandler(object):
             if do_not_load:
                 return
 
-        cmds = []
-        if self._was_loaded:
-            # TODO: Handle cases were module cannot be removed
-            cmds.append('modprobe -r --remove-dependencies %s' % self._module_name)
-        cmd = 'modprobe %s %s' % (self._module_name, params)
-        cmds.append(cmd.strip())
-        logging.debug("Loading module: %s", cmds)
-        for cmd in cmds:
-            status, output = process.getstatusoutput(cmd, ignore_status=True)
-            if status:
-                raise exceptions.TestError("Couldn't load module %s: %s" % (
-                    self._module_name, output
-                ))
-        self._loaded_config = params
+        # TODO: Handle cases were module cannot be removed
+        holders = self.module_holders
+        for holder in holders:
+            holder.unload_module()
+        self.unload_module()
+        reload_cmd = 'modprobe %s %s' % (self._module_name, params)
+        logging.debug("Reloading module: %s", reload_cmd)
+        status, output = process.getstatusoutput(reload_cmd.strip())
+        if status:
+            raise KernelModuleReloadError(self._module_name, output)
+        for holder in holders:
+            holder.restore()
 
     def restore(self):
         """
@@ -132,22 +178,22 @@ class KernelModuleHandler(object):
         +-------------------+-+-+-+-+
         """
 
-        if self._loaded_config is not None:
-            cmds = []
+        if self.current_config != self._config_backup:
             # TODO: Handle cases were module cannot be removed
-            cmds.append('modprobe -r --remove-dependencies %s' % self._module_name)
+            holders = self.module_holders
+            for holder in holders:
+                holder.unload_module()
+            self.unload_module()
             if self._was_loaded:
-                cmd = 'modprobe %s %s' % (self._module_name,
-                                          self._config_backup)
-                cmds.append(cmd.strip())
-            logging.debug("Restoring module state: %s", cmds)
-            for cmd in cmds:
-                status, output = process.getstatusoutput(cmd, ignore_status=True)
+                restore_cmd = 'modprobe %s %s' % (self._module_name,
+                                                  self._config_backup)
+                logging.debug("Restoring module state: %s", restore_cmd)
+                status, output = process.getstatusoutput(restore_cmd)
                 if status:
-                    raise exceptions.TestError(
-                        "Couldn't restore module %s. Command '%s'."
-                        " Output '%s'."
-                        % (self._module_name, cmd, output))
+                    raise KernelModuleRestoreError(self._module_name,
+                                                   output)
+            for holder in holders:
+                holder.restore()
 
     def _backup_config(self):
         """
@@ -176,6 +222,12 @@ class KernelModuleHandler(object):
 
         return self._config_backup
 
+    @property
+    def current_config(self):
+        """ Read-only property """
+
+        return self._get_serialized_config()
+
     def _get_serialized_config(self):
         """
         Get current module parameters
@@ -184,13 +236,21 @@ class KernelModuleHandler(object):
          module not loaded
         """
 
-        mod_params_path = '/sys/module/%s/parameters/' % self._module_name
-        if not os.path.exists(mod_params_path):
+        if not os.path.exists(self._module_params_path):
             return None
 
         mod_params = {}
-        params = os.listdir(mod_params_path)
+        params = os.listdir(self._module_params_path)
         for param in params:
-            with open(os.path.join(mod_params_path, param), 'r') as param_file:
+            with open(os.path.join(self._module_params_path,
+                                   param), 'r') as param_file:
                 mod_params[param] = param_file.read().strip()
         return " ".join("%s=%s" % _ for _ in mod_params.items()) if mod_params else ""
+
+    @property
+    def module_holders(self):
+        """Find out which modules use this module."""
+        if os.path.exists(self._module_holders_path):
+            module_used_by = os.listdir(self._module_holders_path)
+            return [KernelModuleHandler(module) for module in module_used_by]
+        return []