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

[Konstantin Khorenko <khorenko@virtuozzo.com>]

> 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