[libvirt] [test-API PATCH 2/6] Added support for Gentoo

[Martin Kletzander]

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