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

[David Matlack]

On Wed, Apr 1, 2026 at 4:46 PM Raghavendra Rao Ananta
<rananta@xxxxxxxxxx> wrote:
>
> On Tue, Mar 31, 2026 at 5:11 PM David Matlack <dmatlack@xxxxxxxxxx> wrote:
> >
> > On Tue, Mar 31, 2026 at 4:53 PM Raghavendra Rao Ananta
> > <rananta@xxxxxxxxxx> wrote:
> >
> > > > > +FIXTURE_SETUP(vfio_pci_sriov_uapi_test)
> > > > > +{
> > > > > +     char *vf_driver;
> > > > > +     int nr_vfs;
> > > > > +
> > > > > +     nr_vfs = sysfs_sriov_totalvfs_get(pf_bdf);
> > > > > +     if (nr_vfs <= 0)
> > > > > +             SKIP(return, "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)
> > > > > +             SKIP(return, "SR-IOV already configured for the PF: %s\n", pf_bdf);
> > > > > +
> > > >
> > > > If there is a test failure (ASSERT) then fixture cleanup will not run
> > > > leaving SR-IOV enabled and all subsequent tests will skip.
> > > >
> > > > Since this test is specific to SR-IOV and user is intentionally passing
> > > > a device to test, I think we can just reset the VFs to 0 before
> > > > proceeding instead of skipping.
> > > >
> > > The idea was to eliminate the assumption that if 'sriov_numvfs'
> > > returns > 0, someone else might be using it. This is based on David's
> > > suggestion: https://lore.kernel.org/all/aQzcQ0fJd-aCRThS@xxxxxxxxxx/ .
> > > Moreover, the device might be in a partial state. So, even if the next
> > > test proceeds, it could fail due to other reasons.
> > >
> > > But I see your point about cleanup on failure. I think this is a
> > > common issue across all tests. Was the expectation that we rely on
> > > cleanup.sh to take care of all the cleanups, regardless of test
> > > pass/failure?
> >
> > I think we rely on the fact that test cases run in sub-processes and
> > so if there is a failure in fixture setup then the child process exits
> > and all vfio and iommufd fds are cleaned up. So the device is still
> > "clean" for the next test.
> >
> > However if we are modifying system state like the number of VFs
> > enabled on a device, I think Vipin is right we need to clean up after
> > ourselves if the error occurs in fixture setup.
>
> In that case, since the FIXTURE_TEARDOWN() is not invoked for a
> failure in FIXTURE_SETUP(), the test must install its own cleaning
> handler. Using conventional error handling with goto labels might also
> be tricky because the sysfs lib contains ASSERTs. Even if we do reset
> sriov_nrvfs and exit, since the failure is in FIXTURE_SETUP(), it's
> highly likely that it's going to fail again.
>
> Since we need the VF creation only once, shall we simply do this setup
> in main()?
>
> int main()
> {
>   pf_bdf = vfio_selftests_get_bdf(&argc, argv);
>   vf_bdf = setup_vf(pf_bdf); // Do all the sysfs stuff here
>   ret = test_harness_run(argc, argv);
>   destroy_vf();
>
>   return ret;
> }
>
> Another possible way is to expose the VF from setup.sh itself and pass
> it as an arg to the test.
>
> WDYT?

Creating and destroying VFs in main() outside of the test harness
sounds like a good solution to me.