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

[Raghavendra Rao Ananta]

On Tue, May 5, 2026 at 11:52 AM Vipin Sharma <vipinsh@xxxxxxxxxx> wrote:
>
> On Mon, May 04, 2026 at 10:51:41AM -0700, Raghavendra Rao Ananta wrote:
> > On Mon, Apr 13, 2026 at 11:08 AM Vipin Sharma <vipinsh@xxxxxxxxxx> wrote:
> > >
> > > On Thu, Apr 02, 2026 at 05:30:59PM +0000, Raghavendra Rao Ananta wrote:
> > > > +TEST_F(vfio_pci_sriov_uapi_test, override_token)
> > > > +{
> > > > +     struct vfio_pci_device *pf;
> > > > +     struct vfio_pci_device *vf;
> > > > +     struct iommu *iommu;
> > > > +     int ret;
> > > > +
> > > > +     iommu = iommu_init(variant->iommu_mode);
> > > > +
> > > > +     pf = device_init(pf_bdf, iommu, UUID_2, &ret);
> > >
> > > I am assuming because of this, you cannot move device_init and
> > > device_cleanup calls to FIXTURE_SETUP and FIXTURE_TEARDOWN respectively.
> > >
> > > Can we just start this test with device_cleanup(), then do init with
> > > UUID_2? This will allow to reduce the code in all of the tests by moving
> > > things to corresponding setup and teardown functions. WDYT?
> > >
> > Yes it was intentionally kept this way for the 'override_token' test.
> > Also see the previous 'pf_early_close' test that performs a premature
> > cleanup of the PF. To accommodate these (and any future TEST_F()s we
> > may want to add) based on your suggestion, we'd have to create
> > special/conditional statements across the tests and I'd like to avoid
> > that if possible. The current setup clearly shows what each test
> > does/requires.
> >
>
> I think if you make device_cleanup() handle already cleaned up device as
> no-op this should avoid any special handling.
>

Having a conditional check to avoid doing the cleanup only to satisfy
the 'pf_early_close' test was the special handling that I was
referring to.
No other test requires this.

> iommu_init() and device_init() with UUID_1 will be used by all three
> tests in FIXTURE_SETUP(). Their corresponding cleanup will be in
> FIXTURE_TEARDOWN().
>

'override_token' first initializes the PF with UUID_2 as the token and
then overrides to UUID_1.
To easily satisfy all the tests, ASSERT_COND_VF_CREATION() was built
on the assumption that when the VF is verified, the pf token will
always be UUID_1.

Anyway, I tried moving the PF setup into FIXTURE_SETUP() and the PF/VF
cleanups into FIXTURE_CLEANUP(), and I adjusted
ASSERT_COND_VF_CREATION() to accommodate any PF token for comparison.
I'll push these changes to v8 soon. Let me know what you think.

> If we are running a test harness, and one test failed without clearing
> up VFs then all the following tests will pay penalty this way. As this
> is a selftest code, device assigned to it is for testing not for some
> production work at the same time, I am not able to see why it will be a
> unsafe.
>
> My recommendation will be to reset and not skip, I don't think there are
> any practical risks here. I will leave it to you to make a final
> decision on this.

Not just selftests, it's possible that the VF devices were created for
use by actual vfio users. Hence, I'd like to avoid that assumption and
skip touching the devices altogether.

Thank you.
Raghavendra