Skip to content

Conversation

@JunAr7112
Copy link
Contributor

No description provided.

@copy-pr-bot
Copy link

copy-pr-bot bot commented Nov 10, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

return fmt.Errorf("error checking for GPU devices on the host: %w", err)
}
for _, device := range GPUDevices {
if device.SriovInfo.PhysicalFunction == nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if device.SriovInfo.PhysicalFunction == nil {
if device.SriovInfo == nil || device.SriovInfo.PhysicalFunction == nil {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this would work as device.SriovInfo is a struct value (with type SriovInfo) and not a pointer, so it cannot be nil.

totalVF := int(device.SriovInfo.PhysicalFunction.TotalVFs)
if totalVF > 0 {
log.Infof("Found GPU device with SR-IOV VFs: %s (TotalVFs: %d)", device.Address, totalVF)
return nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we returning early here?

Copy link
Contributor Author

@JunAr7112 JunAr7112 Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If VFIO is enabled, than we wouldn't need to execute the rest of the function (and we would error by default when looking for mdev devices). That's why we would return early if VF exist. if VFIO is not enabled, there would be devices under mdevBusPath := "/sys/class/mdev_bus" and we would not enter into this for loop. Instead we would follow the earlier verification code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants