Re: [Qemu-devel] [PATCH 2/6] tests: Add network filter tests to the check-qtest-s390x list

[Cleber Rosa]

On 08/18/2017 03:54 AM, Cornelia Huck wrote:
> On Thu, 17 Aug 2017 16:02:00 +0200
> Thomas Huth <address@hidden> wrote:
> 
>> On 17.08.2017 10:41, Cornelia Huck wrote:
>>> On Thu, 17 Aug 2017 08:25:09 +0200
>>> Thomas Huth <address@hidden> wrote:
>>>   
>>>> With some small modifications, we can also use the the netfilter,
>>>> the fiter-mirror and the filter-redirector tests on s390x.  
>>>
>>> s/fiter/filter/  
>>
>> OK ... could you please fix that when picking up the patch (in case I do
>> not have to resend)?
> 
> Sure.
> 
>>
>>>>
>>>> Signed-off-by: Thomas Huth <address@hidden>
>>>> ---
>>>>  tests/Makefile.include         |  3 +++
>>>>  tests/test-filter-mirror.c     |  9 +++++++--
>>>>  tests/test-filter-redirector.c | 22 ++++++++++++++++------
>>>>  tests/test-netfilter.c         | 11 ++++++++++-
>>>>  4 files changed, 36 insertions(+), 9 deletions(-)
>>>>  
>>>   
>>>> diff --git a/tests/test-filter-mirror.c b/tests/test-filter-mirror.c
>>>> index a1d5865..d569d27 100644
>>>> --- a/tests/test-filter-mirror.c
>>>> +++ b/tests/test-filter-mirror.c
>>>> @@ -25,6 +25,11 @@ static void test_mirror(void)
>>>>      char *recv_buf;
>>>>      uint32_t size = sizeof(send_buf);
>>>>      size = htonl(size);
>>>> +    const char *devstr = "e1000";
>>>> +
>>>> +    if (g_str_equal(qtest_get_arch(), "s390x")) {
>>>> +        devstr = "virtio-net-ccw";
>>>> +    }  
>>>
>>> I'm wondering if we could unify selection of the network device
>>> somehow. There's probably two cases:
>>> - Test a specific device. This obviously needs to be decided
>>>   individually.
>>> - Just use a functional network device. For s390x, this will be
>>>   virtio-net-ccw; for other architectures, this test uses e1000, while
>>>   one of the tests below uses rtl8139 (why?). A helper for that may be
>>>   useful.  
>>
>> Maybe ... OTOH, this likely increases also test coverage if we do not
>> use the same PCI NIC in all the tests...?
> 
> It just looks like a bit of unneeded churn to me.
> 
> Re coverage: Do we have a very simple test that we can run for all kind
> of NICs? This would give some reliable testing for various devices
> instead of having to rely on people picking different devices for their
> tests...
> 

The ideal approach here would be to have a predictable default network
(or any other kind of) device selection, per-target.  Then, no one would
wonder if rtl8139 was chosen on purpose for this specific test.

Besides the default predictable selection, the framework should provide
a way for test *runners* to switch to another device at test run-time.
This is the kind of thing that can improve the coverage a lot, at a very
low development cost.

-- 
Cleber Rosa
[ Sr Software Engineer - Virtualization Team - Red Hat ]
[ Avocado Test Framework - avocado-framework.github.io ]
[  7ABB 96EB 8B46 B94D 5E0F  E9BB 657E 8D33 A5F2 09F3  ]

signature.asc
Description: OpenPGP digital signature