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

[Raghavendra Rao Ananta]

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?

Thank you.
Raghavendra