On 07/04/2011 09:57 AM, Cleber Rosa wrote:
> On 07/04/2011 03:45 AM, Prasad Joshi wrote:
>> On Mon, Jul 4, 2011 at 7:32 AM, Cleber Rosa<[email protected]> wrote:
>>> Building and loading KVM modules that are not shipped with the Linux
>>> kernel
>>> is discouraged these days.
>>>
>>> This patch allows for loading the KVM modules only on the default
>>> system
>>> location, which should be the modules built/shipped with the running
>>> kernel.
>>>
>>> Signed-off-by: Cleber Rosa<[email protected]>
>>> ---
>>> client/tests/kvm/build.cfg.sample | 20 +----
>>> client/virt/kvm_installer.py | 172
>>> ++-----------------------------------
>>> 2 files changed, 11 insertions(+), 181 deletions(-)
>>>
>>> diff --git a/client/tests/kvm/build.cfg.sample
>>> b/client/tests/kvm/build.cfg.sample
>>> index e60c4bc..0bccd65 100644
>>> --- a/client/tests/kvm/build.cfg.sample
>>> +++ b/client/tests/kvm/build.cfg.sample
>>> @@ -34,19 +34,8 @@ variants:
>>> - git:
>>> install_mode = git
>>> ## Install KVM from git repositories.
>>> - ## If you provide only "git_repo" and
>>> "user_git_repo", the
>>> - ## build test will assume it will perform all build
>>> from the
>>> - ## userspace dir, building modules trough
>>> - ## make -C kernel LINUX=%s sync. As of today
>>> (07-13-2009)
>>> - ## upstream needs 3 git repos:
>>> - ## * git_repo (linux sources)
>>> - ## * user_git_repo (qemu sources)
>>> - ## * kmod_repo" to build KVM userspace + kernel
>>> modules.
>>> - # git_repo =
>>> git://git.kernel.org/pub/scm/linux/kernel/git/avi/kvm.git
>>> - # kernel_branch = kernel_branch_name
>>> - # kernel_lbranch = kernel_lbranch_name
>>> - # kernel_commit = kernel_commit_name
>>> - # kernel_patches = ['http://foo.com/patch1',
>>> 'http://foo.com/patch2']
>>> + ## This now only concerns the userspace component,
>>> and it assumes
>>> + ## the kernel modules are shipped with the linux
>>> kernel.
>>> user_git_repo =
>>> git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git
>>> # user_branch = user_branch_name
>>> # user_lbranch = user_lbranch_name
>>> @@ -54,11 +43,6 @@ variants:
>>> # user_patches = ['http://foo.com/patch1',
>>> 'http://foo.com/patch2']
>>> ## This is how you pass extra params to the
>>> userspace configure script
>>> # extra_configure_options = '--enable-spice
>>> --disable-werror'
>>> - # kmod_repo =
>>> git://git.kernel.org/pub/scm/virt/kvm/kvm-kmod.git
>>> - # kmod_branch = kmod_branch_name
>>> - # kmod_lbranch = kmod_lbranch_name
>>> - # kmod_commit = kmod_commit_name
>>> - # kmod_patches = ['http://foo.com/patch1',
>>> 'http://foo.com/patch2']
>>> # spice_repo =
>>> git://anongit.freedesktop.org/spice/spice
>>> # spice_branch = master
>>> # spice_lbranch = master
>>> diff --git a/client/virt/kvm_installer.py
>>> b/client/virt/kvm_installer.py
>>> index e79c9fc..98bde12 100644
>>> --- a/client/virt/kvm_installer.py
>>> +++ b/client/virt/kvm_installer.py
>>> @@ -15,68 +15,6 @@ def kill_qemu_processes():
>>> utils.system("fuser -k /dev/kvm", ignore_status=True)
>>>
>>>
>>> -def _unload_kvm_modules(mod_list):
>>> - logging.info("Unloading previously loaded KVM modules")
>>> - for module in reversed(mod_list):
>>> - utils.unload_module(module)
>>> -
>>> -
>>> -def _load_kvm_modules(mod_list, module_dir=None, load_stock=False):
>>> - """
>>> - Just load the KVM modules, without killing Qemu or unloading
>>> previous
>>> - modules.
>>> -
>>> - Load modules present on any sub directory of module_dir.
>>> Function will walk
>>> - through module_dir until it finds the modules.
>>> -
>>> - @param module_dir: Directory where the KVM modules are located.
>>> - @param load_stock: Whether we are going to load system kernel
>>> modules.
>>> - @param extra_modules: List of extra modules to load.
>>> - """
>>> - if module_dir:
>>> - logging.info("Loading the built KVM modules...")
>>> - kvm_module_path = None
>>> - kvm_vendor_module_path = None
>>> - abort = False
>>> -
>>> - list_modules = ['%s.ko' % (m) for m in mod_list]
>>> -
>>> - list_module_paths = []
>>> - for folder, subdirs, files in os.walk(module_dir):
>>> - for module in list_modules:
>>> - if module in files:
>>> - module_path = os.path.join(folder, module)
>>> - list_module_paths.append(module_path)
>>> -
>>> - # We might need to arrange the modules in the correct order
>>> - # to avoid module load problems
>>> - list_modules_load = []
>>> - for module in list_modules:
>>> - for module_path in list_module_paths:
>>> - if os.path.basename(module_path) == module:
>>> - list_modules_load.append(module_path)
>>> -
>>> - if len(list_module_paths) != len(list_modules):
>>> - logging.error("KVM modules not found. If you don't want
>>> to use the "
>>> - "modules built by this test, make sure
>>> the option "
>>> - "load_modules: 'no' is marked on the test
>>> control "
>>> - "file.")
>>> - raise error.TestError("The modules %s were requested to
>>> be loaded, "
>>> - "but the only modules found were
>>> %s" %
>>> - (list_modules, list_module_paths))
>>> -
>>> - for module_path in list_modules_load:
>>> - try:
>>> - utils.system("insmod %s" % module_path)
>>> - except Exception, e:
>>> - raise error.TestFail("Failed to load KVM modules:
>>> %s" % e)
>>> -
>>> - if load_stock:
>>> - logging.info("Loading current system KVM modules...")
>>> - for module in mod_list:
>>> - utils.system("modprobe %s" % module)
>>> -
>>> -
>>> def create_symlinks(test_bindir, prefix=None, bin_list=None,
>>> unittest=None):
>>> """
>>> Create symbolic links for the appropriate qemu and qemu-img
>>> commands on
>>> @@ -143,8 +81,6 @@ class KvmNotInstalled(KvmInstallException):
>>>
>>>
>>> class BaseInstaller(object):
>>> - # default value for load_stock argument
>>> - load_stock_modules = True
>>> def __init__(self, mode=None):
>>> self.install_mode = mode
>>> self._full_module_list = None
>>> @@ -254,7 +190,9 @@ class BaseInstaller(object):
>>>
>>> May be overridden by subclasses.
>>> """
>>> - _load_kvm_modules(mod_list,
>>> load_stock=self.load_stock_modules)
>>> + logging.info("Loading KVM modules")
>>> + for module in mod_list:
>>> + utils.system("modprobe %s" % module)
>> Why not use the function utils.load_module() to load the modules?
>
> Well, there might be a historic reason for that.
>
> Based on another comment, in another piece of code
> (client/tests/kvm/tests/module_probe.py), I believe the reason is that
> utils.load_module() does "too much": checks if the module is loaded by
> running lsmod, etc.
>
Actually the comment is about utils.unload_module(), but still is the
only hint that I can find.
> As this discussion is not really the point of my proposed change, I
> chose to keep things as they were.
>
>>>
>>> def load_modules(self, mod_list=None):
>>> @@ -269,7 +207,9 @@ class BaseInstaller(object):
>>> """
>>> if mod_list is None:
>>> mod_list = self.full_module_list()
>>> - _unload_kvm_modules(mod_list)
>>> + logging.info("Unloading previously loaded KVM modules")
>>> + for module in reversed(mod_list):
>>> + utils.unload_module(module)
>>>
>>>
>>> def unload_modules(self, mod_list=None):
>>> @@ -297,7 +237,6 @@ class YumInstaller(BaseInstaller):
>>> """
>>> Class that uses yum to install and remove packages.
>>> """
>>> - load_stock_modules = True
>>> def set_install_params(self, test, params):
>>> super(YumInstaller, self).set_install_params(test, params)
>>> # Checking if all required dependencies are available
>>> @@ -357,7 +296,6 @@ class KojiInstaller(YumInstaller):
>>> It uses yum to install and remove packages. Packages are specified
>>> according to the syntax defined in the PkgSpec class.
>>> """
>>> - load_stock_modules = True
>>> def set_install_params(self, test, params):
>>> """
>>> Gets parameters and initializes the package downloader.
>>> @@ -450,8 +388,6 @@ class SourceDirInstaller(BaseInstaller):
>>> super(SourceDirInstaller, self).set_install_params(test,
>>> params)
>>>
>>> self.mod_install_dir = os.path.join(self.prefix, 'modules')
>>> - self.installed_kmods = False # it will be set to True in
>>> case we
>>> - # installed our own modules
>>>
>>> srcdir = params.get("srcdir", None)
>>> self.path_to_roms = params.get("path_to_rom_images", None)
>>> @@ -494,51 +430,11 @@ class SourceDirInstaller(BaseInstaller):
>>> utils.system(step)
>>>
>>>
>>> - def _install_kmods_old_userspace(self, userspace_path):
>>> - """
>>> - Run the module install command.
>>> -
>>> - This is for the "old userspace" code, that contained a
>>> 'kernel' subdirectory
>>> - with the kmod build code.
>>> -
>>> - The code would be much simpler if we could specify the
>>> module install
>>> - path as parameter to the toplevel Makefile. As we can't do
>>> that and
>>> - the module install code doesn't use --prefix, we have to call
>>> - 'make -C kernel install' directly, setting the module
>>> directory
>>> - parameters.
>>> -
>>> - If the userspace tree doens't have a 'kernel' subdirectory,
>>> the
>>> - module install step will be skipped.
>>> -
>>> - @param userspace_path: the path the kvm-userspace directory
>>> - """
>>> - kdir = os.path.join(userspace_path, 'kernel')
>>> - if os.path.isdir(kdir):
>>> - os.chdir(kdir)
>>> - # INSTALLDIR is the target dir for the modules
>>> - # ORIGMODDIR is the dir where the old modules will be
>>> removed. we
>>> - # don't want to mess with the system
>>> modules, so set it
>>> - # to a non-existing directory
>>> - utils.system('make install INSTALLDIR=%s
>>> ORIGMODDIR=/tmp/no-old-modules' % (self.mod_install_dir))
>>> - self.installed_kmods = True
>>> -
>>> -
>>> - def _install_kmods(self, kmod_path):
>>> - """Run the module install command for the kmod-kvm repository
>>> -
>>> - @param kmod_path: the path to the kmod-kvm.git working copy
>>> - """
>>> - os.chdir(kmod_path)
>>> - utils.system('make modules_install DESTDIR=%s' %
>>> (self.mod_install_dir))
>>> - self.installed_kmods = True
>>> -
>>> -
>>> def _install(self):
>>> os.chdir(self.srcdir)
>>> logging.info("Installing KVM userspace")
>>> if self.repo_type == 1:
>>> utils.system("make -C qemu install")
>>> - self._install_kmods_old_userspace(self.srcdir)
>>> elif self.repo_type == 2:
>>> utils.system("make install")
>>> if self.path_to_roms:
>>> @@ -549,17 +445,6 @@ class SourceDirInstaller(BaseInstaller):
>>> unittest=self.unittest_prefix)
>>>
>>>
>>> - def _load_modules(self, mod_list):
>>> - # load the installed KVM modules in case we installed them
>>> - # ourselves. Otherwise, just load the system modules.
>>> - if self.installed_kmods:
>>> - logging.info("Loading installed KVM modules")
>>> - _load_kvm_modules(mod_list,
>>> module_dir=self.mod_install_dir)
>>> - else:
>>> - logging.info("Loading stock KVM modules")
>>> - _load_kvm_modules(mod_list, load_stock=True)
>>> -
>>> -
>>> def install(self):
>>> self._build()
>>> self._install()
>>> @@ -611,19 +496,6 @@ class GitInstaller(SourceDirInstaller):
>>> cfg =
>>> 'PKG_CONFIG_PATH="%s/lib/pkgconfig:%s/share/pkgconfig" ./configure' % (
>>> self.prefix, self.prefix)
>>>
>>> - self.kernel = GitRepo(installer=self, prefix='kernel',
>>> - repo_param='git_repo', srcdir='kvm')
>>> - self.kmod = GitRepo(installer=self, prefix='kmod',
>>> srcdir="kvm_kmod")
>>> - if params.get('kernel_git_repo'):
>>> - cfg += ' --kerneldir=%s' % self.host_kernel_srcdir
>>> - self.kernel.build_steps = [cfg,
>>> - 'make clean',
>>> - 'make -C kernel LINUX=%s sync' %
>>> self.kernel.srcdir]
>>> - self.kmod.build_steps=[cfg,
>>> - 'make clean',
>>> - 'make sync LINUX=%s' %
>>> self.kernel.srcdir,
>>> - 'make']
>>> -
>>> self.spice_protocol = GitRepo(installer=self,
>>> prefix='spice_protocol',
>>> srcdir='spice-protocol',
>>> build_steps= ['./autogen.sh',
>>> @@ -657,19 +529,12 @@ class GitInstaller(SourceDirInstaller):
>>> logging.error(message)
>>> raise error.TestError(message)
>>>
>>> - for repo in [self.userspace, self.kernel, self.kmod,
>>> self.spice_protocol, self.spice]:
>>> + for repo in [self.userspace, self.spice_protocol, self.spice]:
>>> if not repo.repo:
>>> continue
>>> repo.fetch_and_patch()
>>>
>>> def _build(self):
>>> - if self.kmod.repo:
>>> - logging.info('Building KVM modules')
>>> - self.kmod.build()
>>> - elif self.kernel.repo:
>>> - logging.info('Building KVM modules')
>>> - self.kernel.build()
>>> -
>>> if self.spice_protocol.repo:
>>> logging.info('Building Spice-protocol')
>>> self.spice_protocol.build()
>>> @@ -683,25 +548,8 @@ class GitInstaller(SourceDirInstaller):
>>>
>>>
>>> def _install(self):
>>> - if self.kernel:
>>> - os.chdir(self.userspace.srcdir)
>>> - # the kernel module install with --prefix doesn't work,
>>> and DESTDIR
>>> - # wouldn't work for the userspace stuff, so we clear
>>> WANT_MODULE:
>>> - utils.system('make install WANT_MODULE=')
>>> - # and install the old-style-kmod modules manually:
>>> - self._install_kmods_old_userspace(self.userspace.srcdir)
>>> - elif self.kmod:
>>> - # if we have a kmod repository, it is easier:
>>> - # 1) install userspace:
>>> - os.chdir(self.userspace_srcdir)
>>> - utils.system('make install')
>>> - # 2) install kmod:
>>> - self._install_kmods(self.kmod_srcdir)
>>> - else:
>>> - # if we don't have kmod sources, we just install
>>> - # userspace:
>>> - os.chdir(self.userspace.srcdir)
>>> - utils.system('make install')
>>> + os.chdir(self.userspace.srcdir)
>>> + utils.system('make install')
>>>
>>> if self.path_to_roms:
>>> install_roms(self.path_to_roms, self.prefix)
>>> @@ -721,8 +569,6 @@ class GitInstaller(SourceDirInstaller):
>>>
>>>
>>> class PreInstalledKvm(BaseInstaller):
>>> - # load_modules() will use the stock modules:
>>> - load_stock_modules = True
>>> def install(self):
>>> logging.info("Expecting KVM to be already installed. Doing
>>> nothing")
>>>
>>> --
>>> 1.7.4.4
>>>
>>> _______________________________________________
>>> Autotest mailing list
>>> [email protected]
>>> http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
>>>
>
_______________________________________________
Autotest mailing list
[email protected]
http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
