[libvirt] [TCK] nwfilter tests and libvirt commit 4f209434
Hi, As I've been looking through libvirt-tck tests, I found that commit 4f209434 (in libvirt) changes a condition that the nwfilter/050-apply-verify-host.t relies on. Specifically, the 050-apply-verify-host.t test creates a number of filters with invalid values (such as dscp='64', and protocol='256'). Without commit 4f209434, these invalid values are silently dropped off the end of the rule, as in the following example: #virsh nwfilter-define <xml with illegal value:> <filter name='tck-testcase'> <uuid>5c6d49af-b071-6127-b4ec-6f8ed4b55335</uuid> <rule action='accept' direction='in'> <ip srcipaddr='10.1.2.3' srcipmask='255.255.255.254' dstipaddr='10.1.2.3' dstipmask='255.255.255.128' protocol='255' dscp='64' /> </rule> #virsh nwfilter-dumpxml tck-testcase <rule action='accept' direction='in' priority='500'> <ip srcipaddr='10.1.2.3' srcipmask='31' dstipaddr='10.1.2.3' dstipmask='25' protocol='255'/> </rule> With commit 4f209434 in place, the entire filter is rejected with the following error: 'internal error: dscp has illegal value 64'. As the filter is not created, the testing of that filter by the 050-apply-verify-host test fails. I agree that the change is the right thing to do, but I'm wondering how best to handle the now failing tests. This test (050-apply-verify-host.t) runs ~1200 subtests, and about 10% of those tests fail due to the change. I'm thinking that the illegal options (and only those illegal options) should be removed from the now broken tests, and a few new tests added to specifically test for the failure to add the entire rule when illegal data is passed. Thoughts? BTW - Is anyone else running the full libvirt-tck suite against recent versions of libvirt? There are quite a few issues (such as this one) which should be easily seen... Thanks, Mike
On 03/27/2014 12:22 AM, Mike Latimer wrote:
Hi,
As I've been looking through libvirt-tck tests, I found that commit 4f209434 (in libvirt) changes a condition that the nwfilter/050-apply-verify-host.t relies on.
Specifically, the 050-apply-verify-host.t test creates a number of filters with invalid values (such as dscp='64', and protocol='256'). Without commit 4f209434, these invalid values are silently dropped off the end of the rule, as in the following example:
#virsh nwfilter-define <xml with illegal value:> <filter name='tck-testcase'> <uuid>5c6d49af-b071-6127-b4ec-6f8ed4b55335</uuid> <rule action='accept' direction='in'> <ip srcipaddr='10.1.2.3' srcipmask='255.255.255.254' dstipaddr='10.1.2.3' dstipmask='255.255.255.128' protocol='255' dscp='64' /> </rule>
#virsh nwfilter-dumpxml tck-testcase <rule action='accept' direction='in' priority='500'> <ip srcipaddr='10.1.2.3' srcipmask='31' dstipaddr='10.1.2.3' dstipmask='25' protocol='255'/> </rule>
With commit 4f209434 in place, the entire filter is rejected with the following error: 'internal error: dscp has illegal value 64'. As the filter is not created, the testing of that filter by the 050-apply-verify-host test fails.
I agree that the change is the right thing to do, but I'm wondering how best to handle the now failing tests. This test (050-apply-verify-host.t) runs ~1200 subtests, and about 10% of those tests fail due to the change. I'm thinking that the illegal options (and only those illegal options) should be removed from the now broken tests, and a few new tests added to specifically test for the failure to add the entire rule when illegal data is passed. Thoughts?
I agree.
BTW - Is anyone else running the full libvirt-tck suite against recent versions of libvirt?
I had thought someone had setup a continuous run of libvirt-tck against current upstream, but maybe that's only being done for autotest.
There are quite a few issues (such as this one) which should be easily seen...
On Wed, Mar 26, 2014 at 04:22:04PM -0600, Mike Latimer wrote:
Hi,
As I've been looking through libvirt-tck tests, I found that commit 4f209434 (in libvirt) changes a condition that the nwfilter/050-apply-verify-host.t relies on.
Specifically, the 050-apply-verify-host.t test creates a number of filters with invalid values (such as dscp='64', and protocol='256'). Without commit 4f209434, these invalid values are silently dropped off the end of the rule, as in the following example:
#virsh nwfilter-define <xml with illegal value:> <filter name='tck-testcase'> <uuid>5c6d49af-b071-6127-b4ec-6f8ed4b55335</uuid> <rule action='accept' direction='in'> <ip srcipaddr='10.1.2.3' srcipmask='255.255.255.254' dstipaddr='10.1.2.3' dstipmask='255.255.255.128' protocol='255' dscp='64' /> </rule>
#virsh nwfilter-dumpxml tck-testcase <rule action='accept' direction='in' priority='500'> <ip srcipaddr='10.1.2.3' srcipmask='31' dstipaddr='10.1.2.3' dstipmask='25' protocol='255'/> </rule>
With commit 4f209434 in place, the entire filter is rejected with the following error: 'internal error: dscp has illegal value 64'. As the filter is not created, the testing of that filter by the 050-apply-verify-host test fails.
I agree that the change is the right thing to do, but I'm wondering how best to handle the now failing tests. This test (050-apply-verify-host.t) runs ~1200 subtests, and about 10% of those tests fail due to the change. I'm thinking that the illegal options (and only those illegal options) should be removed from the now broken tests, and a few new tests added to specifically test for the failure to add the entire rule when illegal data is passed. Thoughts?
I've got fixes for this test suite pending that I'll be sending today hopefully. It will be a mixture of fixing the illegal attribute values and/or deleting pointless rules. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (3)
-
Daniel P. Berrange -
Laine Stump -
Mike Latimer