Re: [PATCH v3 08/10] rust: pci: add SR-IOV enable and disable tokens

[Danilo Krummrich]

On Wed Sep 30, 2026 at 12:18 PM CEST, Zhi Wang wrote:
> +pub struct SriovEnable<'callback> {

I understand why you chose 'callback, but I think just 'a is good enough.

> +    pdev: &'callback Device<device::CoreInternal<'callback>>,
> +    num_vfs: u32,

Please see patch 1.

> +}
> +
> +impl<'callback> SriovEnable<'callback> {
> +    /// Returns the number of VFs requested for this callback.
> +    pub fn num_vfs(&self) -> u32 {
> +        self.num_vfs
> +    }
> +
> +    /// Enables between one and the requested number of VFs.
> +    ///
> +    /// VF drivers can probe before this method returns, so the PF resources they access must
> +    /// already be initialized. The returned guard disables the VFs if subsequent setup fails.
> +    /// Return it from `sriov_enable()` to leave the VFs enabled on callback success.
> +    pub fn enable(self, num_vfs: u32) -> Result<SriovEnabled<'callback>> {
> +        if num_vfs == 0 || num_vfs > self.num_vfs {
> +            return Err(EINVAL);
> +        }
> +        let num_vfs = c_int::try_from(num_vfs).map_err(|_| EOVERFLOW)?;
> +
> +        // SAFETY: The PF's driver data and registration remain installed throughout this callback.
> +        // The registration owns final VF teardown after the enable guard is disarmed.
> +        if unsafe { (*self.pdev.as_raw()).vf_registration_data_rust }.is_null() {
> +            return Err(ENODEV);
> +        }
> +
> +        self.pdev.enable_sriov(num_vfs)?;
> +        Ok(SriovEnabled {
> +            pdev: Some(self.pdev),
> +            num_vfs,
> +        })
> +    }
> +}
> +
> +/// Enabled VFs awaiting successful completion of `sriov_enable()`.
> +pub struct SriovEnabled<'callback> {
> +    pdev: Option<&'callback Device<device::CoreInternal<'callback>>>,

This doesn't need to be an Option.

> +    num_vfs: c_int,
> +}
> +
> +impl SriovEnabled<'_> {
> +    #[expect(dead_code)]
> +    fn disarm(mut self) -> c_int {
> +        self.pdev = None;
> +        self.num_vfs

Instead of the Option dance, you can use ManuallyDrop to prevent the destructor
from running.

> +    }
> +}
> +
> +impl Drop for SriovEnabled<'_> {
> +    fn drop(&mut self) {
> +        if let Some(pdev) = self.pdev.take() {
> +            pdev.disable_sriov();
> +        }
> +    }
> +}