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

[David Matlack]

On Wed, Apr 1, 2026 at 5:17 PM Raghavendra Rao Ananta
<rananta@xxxxxxxxxx> wrote:
>
> On Wed, Apr 1, 2026 at 4:51 PM David Matlack <dmatlack@xxxxxxxxxx> wrote:
> >
> > 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.
>
> Gah.. thinking this through, since we use ASSERTs in sysfs setup, we'd
> still have problems with 'goto label' like error handling. If it's not
> getting too complicated, I can set up an exit handler (say via
> exitat()) and destroy VFs there.

That sounds reasonable. Doing the setup in a child process would be
another viable approach.