[libvirt] [PATCH 2/2] Tests : Make nwfilter testcases robust against optional locking flags.

Martin Kletzander mkletzan at redhat.com
Wed Nov 26 11:09:38 UTC 2014


On Wed, Nov 26, 2014 at 04:11:16PM +0530, Prerna Saxena wrote:
>
>On Wednesday 26 November 2014 04:06 PM, Martin Kletzander wrote:
>> On Wed, Nov 26, 2014 at 03:28:21PM +0530, Prerna Saxena wrote:
>>> Hi Martin,
>>> Thanks for the feedback.
>>>
>>> On Tuesday 25 November 2014 05:57 PM, Martin Kletzander wrote:
>>>> On Tue, Nov 25, 2014 at 05:12:48PM +0530, Prerna Saxena wrote:
>>>>> Commit dc33e6e4a5a5d42 introduces iptables/ebtables to adding locking
>>>>> flags if/when these are available. However, the nwfilter testcases
>>>>> list outputs without taking into account whether locking flags have been passed.
>>>>>
>>>>> This shows up false testcase failures such as :
>>>>> 2) ebiptablesTearOldRules                                            ...
>>>>> Offset 1035
>>>>> Expect [t nat -D PREROUTING -i vnet0 -j libvirt-I-vnet0
>>>>> ebtables -t nat -D POSTROUTING -o vnet0 -j libvirt-O-vnet0
>>>>> ebtables -t nat -L libvirt-I-vnet0
>>>>> ebtables -t nat -L libvirt-O-vnet0
>>>>> ...snip...]
>>>>> Actual [-concurrent -t nat -D PREROUTING -i vnet0 -j libvirt-I-vnet0
>>>>> ebtables --concurrent -t nat -D POSTROUTING -o vnet0 -j libvirt-O-vnet0
>>>>> ebtables --concurrent -t nat -L libvirt-I-vnet0
>>>>> ebtables --concurrent -t nat -L libvirt-O-vnet0
>>>>> ...snip...]
>>>>>
>>>>> This scrubs all reference to locking flags from test results buffer,
>>>>> so that achieved output matches the expected results.
>>>>>
>>>> Instead of parsing and re-creating the string (which also doesn't
>>>> check whether we use the locking flag properly),
>>> The function virtTestClearLockingArgs() merely replaces instances of 'ebtables --concurrent' with 'ebtables'.
>>> (likewise for iptables and ip6tables), if at all found. I didnt find the need for sanity checking in this approach :-)
>>>> it would be way
>>>> better if we could unify the result.
>>> Having said so, I agree with this.
>>>
>>>> From the top of my head, we can either expose the
>>>> virFirewallCheckUpdateLock() as non-static and mock it in tests to
>>>> always set the lock flags to true or we can create new functions that
>>>> will override setting of the flags.
>>>>
>>> The problem is with expected results that are coded for these tests.
>>> On distros that support these flags, the issue would go away if the expected results take into account the locking flags. However, adding a permanent change to the expected args string would break
>>> older distros.
>> Actually, no, I wanted to unconditionally add the parameters there
>> only for tests.
>>
>> Looking at it more closely, this can fail only if you are building as
>> root, is that correct?
>
>Yes, that is correct.
>

I'm testing a patch with different approach, I'll Cc you on that when
it is done.  Let me know whether that works for you then.

>>> So I thought of tweaking the actual results.
>>>
>>> Approach #2:
>>> We could change the expected results to look somewhat like this :
>>> ebtables $FLAGS -t nat -D PREROUTING -i vnet0 -j libvirt-I-vnet0
>>>
>>> And have a script that dynamically replaces $FLAGS with :
>>> * "--concurrent", if locking is supported at compile-time.
>>> * OR, with " ", if locking is not available.
>>>
>>> Ofcourse, not all tests have their expected results in a separate file. Some such as
>>> tests/nwfilterebiptablestest.c have their expected args in the form of char* encoded in the same program. This complicates this approach..
>>>
>>> I am looking forward to community suggestions on how this can best be implemented. Will be happy to rework this patch if needed.
>>>
>>> Regards,
>>>
>>> --
>>> Prerna Saxena
>>>
>>> Linux Technology Centre,
>>> IBM Systems and Technology Lab,
>>> Bangalore, India
>>>
>
>--
>Prerna Saxena
>
>Linux Technology Centre,
>IBM Systems and Technology Lab,
>Bangalore, India
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20141126/74ec3fe2/attachment-0001.sig>


More information about the libvir-list mailing list