[libvirt] [test-API PATCH 2/6] Added support for Gentoo
Martin Kletzander
mkletzan at redhat.com
Tue Mar 27 07:44:06 UTC 2012
On 03/26/2012 02:23 PM, Peter Krempa wrote:
> On 03/24/2012 06:42 PM, Martin Kletzander wrote:
>> In order to support libvirt-test-API on more distributions, this
>> commit adds support for Gentoo.
>> The file is copy-paste from dist/redhat/env_update.py just modified to
>> make the get_* functions work on Gentoo, some removed.
>
> Probably no one else that uses the test api uses Gentoo, so I'll do a
> review of this patch.
>
>> ---
>> dist/gentoo/env_inspect.py | 98
>> ++++++++++++++++++++++++++++++++++++++++++++
>> 1 files changed, 98 insertions(+), 0 deletions(-)
>> create mode 100644 dist/gentoo/__init__.py
>> create mode 100644 dist/gentoo/env_inspect.py
>>
>> diff --git a/dist/gentoo/__init__.py b/dist/gentoo/__init__.py
>> new file mode 100644
>> index 0000000..e69de29
>> diff --git a/dist/gentoo/env_inspect.py b/dist/gentoo/env_inspect.py
>> new file mode 100644
>> index 0000000..e8fccc0
>> --- /dev/null
>> +++ b/dist/gentoo/env_inspect.py
>> @@ -0,0 +1,98 @@
>> +#!/usr/bin/env python
>> +#
>> +# libvirt-test-API is copyright 2010, 2012 Red Hat, Inc.
>
> s/2010, //
>
We should have 2010, 2012 or 2010-2012 in the file, not just 2010, I guess.
>> +# libvirt-test-API is free software: you can redistribute it and/or
>> modify it
>> +# under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation, either version 2 of the License, or
>> +# (at your option) any later version. This program is distributed in
>> +# the hope that it will be useful, but WITHOUT ANY WARRANTY; without
>> +# even the implied warranties of TITLE, NON-INFRINGEMENT,
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>> +#
>> +# The GPL text is available in the file COPYING that accompanies this
>> +# distribution and at<http://www.gnu.org/licenses>.
>> +#
>> +# Filename: envinspect.py
>
> s/envinspect/env_inspect/
>
>> +# Summary: To generate a callable class for clearing testing environment
>> +# Description: The module match the reference of clearing function
>> +# from each testcase to the corresponding testcase's
>> +# argument in the order of testcase running
>
> A little re-wording would help to understand what's going on here.
>
This and others are just a copy-paste from redhat/env_inspect.py and I
didn't write it but when writing v2, I'll try to rephrase this.
>> +
>> +import os
>> +import portage
>> +
>> +vt_dbapi = portage.db[portage.root]['vartree'].dbapi
>> +
>> +def get_libvirt_ver():
>> + pkg = vt_dbapi.match('app-emulation/libvirt')
>> + if pkg:
>> + return 0, pkg[-1]
>> + else:
>> + return 100, "No libvirt installed"
>> +
>> +def get_libvirt_pyth_ver():
>> + pkgs = vt_dbapi.match('app-emulation/libvirt')
>> + for pkg in pkgs:
>> + if 'python' in vt_dbapi.aux_get(pkg, ['USE'])[0].split():
>> + return 0, '%s[python]' % pkg
>> +
>> + return 100, "USE flag 'python' not enabled for libvirt"
>> +
>> +def get_qemu_kvm_ver():
>> + pkg = vt_dbapi.match('qemu-kvm') or vt_dbapi.match('qemu')
>> + if pkg:
>> + return 0, pkg[-1]
>> + else:
>> + return 100, "No qemu installed"
>
> This function returns codes 0 or 100 ...
>
>> +
>> +def get_kernel_ver():
>> + # on Gentoo, there is no need to check for kernel
>> + return 0, os.uname()[2]
>> +
>> +
>> +class EnvInspect(object):
>> + """to check and collect the testing enviroment infomation
>> + before performing testing
>> + """
>> +
>> + def __init__(self, logger):
>> + self.logger = logger
>> +
>> + def env_checking(self):
>> + flag = 0
>> + result = ""
>> + if get_libvirt_ver()[0] == 100:
>> + result = NOTOK
>> + flag = 1
>> + else:
>> + result = OK
>> + self.logger.info(" %-36s%-6s" % (get_libvirt_ver()[1],
>> result))
>> +
>> + if get_libvirt_pyth_ver()[0] == 100:
>> + result = NOTOK
>> + flag = 1
>> + else:
>> + result = OK
>> + self.logger.info(" %-36s%-6s" %
>> (get_libvirt_pyth_ver()[1], result))
>> +
>> + if get_qemu_kvm_ver()[0] == 150 and flag == 0:
>> + flag = 0
>> + elif get_qemu_kvm_ver()[0] == 150 and flag == 1:
>> + flag = 1
>
> ... so these tests here are not needed. (And are quite strange too ...
> but that's probably a cut&paste leftover)
>
>> + else:
>> + pass
>> + self.logger.info(" %-36s%-6s" % (get_qemu_kvm_ver()[1], OK))
>
> And in any case, the test succeeds, so it's probably ment to be only a
> version check, that is not mandatory. IMO we should make this check
> mandatory for now, as most of the tests use the local hypervisor for
> testing machines and only a few tests actualy deal with remote code.
>
>> +
>> + if get_kernel_ver()[0] == 100:
>> + result = NOTOK
>> + flag = 1
>> + else:
>
> Well, as was said in the comment, on Gentoo the kernel test always
> succeeds, so the return value check is not necessary and can be changed
> to something like:
>
> self.logger.info(" %-36s%-6s" % (get_kernel_ver()[1], OK))
>
> and leave out the condition.
I wanted to completely rewrite this (I don't understand why we use
return codes like 0 and 1 instead of True and False, codes 0 and 100,
then check it for 150, call the functions twice, etc.), however the main
goal was to make it work. Nevertheless I'll rewrite this.
>> + result = OK
>> + self.logger.info(" %-36s%-6s" % (get_kernel_ver()[1],
>> result))
>> +
>> + return flag
>> +
>> +
>> +OK = "ok"
>> +NOTOK = "not ok"
>
> In any case, it works correctly on Gentoo and the tests are faster than
> querying the package states with equery.
>
> Probably worth a v2 where you change the test conditions.
>
> Peter
>
Will do.
Thanks.
Martin
More information about the libvir-list
mailing list