Re: [PATCH v7 8/8] vfio: selftests: Add tests to validate SR-IOV UAPI

[David Matlack]

On Tue, Apr 7, 2026 at 1:51 PM Raghavendra Rao Ananta
<rananta@xxxxxxxxxx> wrote:
>
> On Mon, Apr 6, 2026 at 3:24 PM David Matlack <dmatlack@xxxxxxxxxx> wrote:
> >
> > On 2026-04-02 05:30 PM, Raghavendra Rao Ananta wrote:
> >
> > > +/* clang-format off */
> > > +FIXTURE(vfio_pci_sriov_uapi_test) {};
> > > +/* clang-format on */
> > > +
> > > +FIXTURE_SETUP(vfio_pci_sriov_uapi_test)
> > > +{
> > > +}
> > > +
> > > +FIXTURE_TEARDOWN(vfio_pci_sriov_uapi_test)
> > > +{
> > > +}
> >
> > Please do iommu_init() iommu_cleanup() here to reduce code duplication
> > and as an added bonus you can drop the clang-format comments above.
> >
> I think even Vipin suggested this. Last time I tried this, IIRC got a
> build error that 'variant' was inaccessible. But I guess I just had to
> re-org my code to make it work.
> I'll do this in v8.
>
> > > +static void vf_teardown(void)
> > > +{
> > > +     /* Destroy the VF only when the main/parent process exits. */
> > > +     if (getpid() != main_pid)
> > > +             return;
> >
> > Is this because the child processes created by test_harness_run() to run
> > the TEST_F()s inherit the atexit() call? If so please clarify that in
> > the comment in more detail.
> >
> Yes, that's the reason, and I assumed the comment made that clear.
> I'll re-write it if required.
>
> > > +
> > > +     free(vf_bdf);
> > > +     sysfs_sriov_numvfs_set(pf_bdf, 0);
> > > +}
> > > +
> > > +static void vf_setup(void)
> > > +{
> > > +     char *vf_driver;
> > > +     int nr_vfs;
> > > +
> > > +     nr_vfs = sysfs_sriov_totalvfs_get(pf_bdf);
> > > +     if (nr_vfs <= 0)
> > > +             ksft_exit_skip("SR-IOV may not be supported by the PF: %s\n", pf_bdf);
> > > +
> > > +     nr_vfs = sysfs_sriov_numvfs_get(pf_bdf);
> > > +     if (nr_vfs != 0)
> > > +             ksft_exit_skip("SR-IOV already configured for the PF: %s\n", pf_bdf);
> > > +
> > > +     /* Create only one VF for testing */
> > > +     sysfs_sriov_numvfs_set(pf_bdf, 1);
> > > +
> > > +     /*
> > > +      * Setup an exit handler to destroy the VF in case of failures
> > > +      * during further setup at the end of the test run.
> > > +      */
> > > +     main_pid = getpid();
> > > +     VFIO_ASSERT_EQ(atexit(vf_teardown), 0);
> > > +
> > > +     vf_bdf = sysfs_sriov_vf_bdf_get(pf_bdf, 0);
> > > +
> > > +     /*
> > > +      * The VF inherits the driver from the PF.
> > > +      * Ensure this is 'vfio-pci' before proceeding.
> > > +      */
> > > +     vf_driver = sysfs_driver_get(vf_bdf);
> > > +     VFIO_ASSERT_NE(vf_driver, NULL);
> > > +     VFIO_ASSERT_EQ(strcmp(vf_driver, "vfio-pci"), 0);
> > > +     free(vf_driver);
> > > +
> > > +     printf("Created 1 VF (%s) under the PF: %s\n", vf_bdf, pf_bdf);
> >
> > Do we actually need atexit()? This code can go into main and we can do
> > the VF cleanup before returning from main after test_harness_run()
> > returns. I don't think you even need to check the VF driver. If the VF
> > is not bound to vfio-pci then that will be caught by the test cases when
> > they call device_init(vf_bdf, ...).
> >
> Other than the asserts on 'vf_driver', the calls to
> `sysfs_sriov_vf_bdf_get()` and `sysfs_driver_get()` themselves have a
> potential chance of triggering asserts, although this is unlikely.
> But the bigger reason is that test_harness_run() unconditionally calls
> ksft_exit() at the end, forcing it to be the last function called from
> main(), regardless of the tests' outcome. Hence, the exit handler.
>
> I see atexit() used in some selftests, so I thought it was normal and
> acceptable.

Ah ok if test_harness_run() exits instead of returning then I agree
with using atexit(). Thanks for the explanation!