[Devel] [PATCH vz10] selftests: pci_endpoint: skip when the test device is absent

[Devel] [PATCH vz10] selftests: pci_endpoint: skip when the test device is absent

[Eva Kurchatova]

Every fixture opens /dev/pci-endpoint-test.0 without checking, so on a
machine that has no PCI endpoint test device each case fails on the
first ioctl against fd -1 rather than saying the hardware is not there.

Check for the device once before handing over to the harness. Doing it
there rather than in each of the four fixtures keeps it to a single skip.

https://virtuozzo.atlassian.net/browse/VSTOR-142446
Feature: fix selftests
Signed-off-by: Eva Kurchatova <eva.kurchatova@virtuozzo.com>
---
 .../selftests/pci_endpoint/pci_endpoint_test.c      | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/pci_endpoint/pci_endpoint_test.c b/tools/testing/selftests/pci_endpoint/pci_endpoint_test.c
index da0db0e7c969..640c74fd5b78 100644
--- a/tools/testing/selftests/pci_endpoint/pci_endpoint_test.c
+++ b/tools/testing/selftests/pci_endpoint/pci_endpoint_test.c
@@ -257,4 +257,15 @@ TEST_F(pcie_ep_doorbell, DOORBELL_TEST)
 	pci_ep_ioctl(PCITEST_DOORBELL, 0);
 	EXPECT_FALSE(ret) TH_LOG("Test failed for Doorbell\n");
 }
-TEST_HARNESS_MAIN
+static bool test_device_available(void)
+{
+	return access(test_device, F_OK) == 0;
+}
+
+int main(int argc, char **argv)
+{
+	if (!test_device_available())
+		ksft_exit_skip("no PCI endpoint test device %s\n", test_device);
+
+	return test_harness_run(argc, argv);
+}
-- 
2.55.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH vz10] selftests: pci_endpoint: skip when the test device is absent

[Konstantin Khorenko]

> Every fixture opens /dev/pci-endpoint-test.0 without checking, so on a
> machine that has no PCI endpoint test device each case fails on the
> first ioctl against fd -1 rather than saying the hardware is not there.
> 

The commit message describes a different bug than the one that exists:

      Every fixture opens /dev/pci-endpoint-test.0 without checking, so on a
      machine that has no PCI endpoint test device each case fails on the
      first ioctl against fd -1

Every fixture does check:

      FIXTURE_SETUP(pci_ep_bar)
      {
              self->fd = open(test_device, O_RDWR);

              ASSERT_NE(-1, self->fd) TH_LOG("Can't open PCI Endpoint Test device");
      }

at pci_endpoint_test.c:43 (pci_ep_bar), :82 (pci_ep_basic), :153
(pci_ep_data_transfer) and :232 (pcie_ep_doorbell).

No ioctl is ever reached - what fails is the assertion in the fixture setup
("Test terminated by assertion" in the output above).

Please reword: each of the 17 cases fails on the fixture setup assertion.
Otherwise the next reader goes looking for an ioctl(-1, ...) bug that is not
there. The "four fixtures" part is correct.

> Check for the device once before handing over to the harness. Doing it
> there rather than in each of the four fixtures keeps it to a single skip.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-142446
> Feature: fix selftests
> Signed-off-by: Eva Kurchatova <eva.kurchatova@virtuozzo.com>
>
> diff --git a/tools/testing/selftests/pci_endpoint/pci_endpoint_test.c b/tools/testing/selftests/pci_endpoint/pci_endpoint_test.c
> index da0db0e7c9693..640c74fd5b780 100644
> --- a/tools/testing/selftests/pci_endpoint/pci_endpoint_test.c
> +++ b/tools/testing/selftests/pci_endpoint/pci_endpoint_test.c
> @@ -257,4 +257,15 @@ TEST_F(pcie_ep_doorbell, DOORBELL_TEST)
>  	pci_ep_ioctl(PCITEST_DOORBELL, 0);
>  	EXPECT_FALSE(ret) TH_LOG("Test failed for Doorbell\n");
>  }
> -TEST_HARNESS_MAIN

An empty line to be here.

> +static bool test_device_available(void)
> +{
> +	return access(test_device, F_OK) == 0;
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	if (!test_device_available())
> +		ksft_exit_skip("no PCI endpoint test device %s\n", test_device);
> +
> +	return test_harness_run(argc, argv);
> +}

The check runs before test_harness_run(), while argv parsing happens
inside it (test_harness_argv_check(), kselftest_harness.h:1119).

So on a machine without the device, "./pci_endpoint_test -l" (list tests) and
"-h" (usage) now print the skip message instead of their own output. Please
limit the skip to the plain invocation, which is the one kselftest uses:

      int main(int argc, char **argv)
      {
              /*
               * Only the plain invocation - the one kselftest uses - turns into a
               * skip, so that -l and -h keep working without the hardware.
               */
              if (argc == 1 && !test_device_available())
                      ksft_exit_skip("no PCI endpoint test device %s\n", test_device);

              return test_harness_run(argc, argv);
      }

-- 
Konstantin Khorenko <khorenko@virtuozzo.com>
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel