Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rename PCIAddress.Slot to PCIAddress.Device #252

Merged
merged 1 commit into from
May 25, 2021
Merged

Conversation

jaypipes
Copy link
Owner

As noted by @pearsonk in #220, the PCIAddress.Slot field was a bit of a
misnomer, since the more common representation of a PCIAddress is
Bus:Device.Function, not Bus:Slot.Function. Actual PCI slot information
may be retrieved from DMI, but what we were referring to was not the
actual PCI slot information but rather the PCI address device component.

Fixes Issue #220

@@ -40,7 +40,7 @@ func getDeviceModaliasPath(ctx *context.Context, address string) string {
}
return filepath.Join(
paths.SysBusPciDevices,
pciAddr.Domain+":"+pciAddr.Bus+":"+pciAddr.Slot+"."+pciAddr.Function,
pciAddr.Domain+":"+pciAddr.Bus+":"+pciAddr.Device+"."+pciAddr.Function,
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should just use pciAddr.String() here - perhaps in a later patch

Copy link
Owner Author

Choose a reason for hiding this comment

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

++ fixed.

Copy link
Collaborator

@ffromani ffromani left a comment

Choose a reason for hiding this comment

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

minor suggestion inside, otherwise lgtm

As noted by @pearsonk in #220, the PCIAddress.Slot field was a bit of a
misnomer, since the more common representation of a PCIAddress is
Bus:Device.Function, not Bus:Slot.Function. Actual PCI slot information
may be retrieved from DMI, but what we were referring to was not the
actual PCI slot information but rather the PCI address device component.

Fixes Issue #220
@ffromani
Copy link
Collaborator

Looks great to me :)

@ffromani ffromani merged commit 718127c into master May 25, 2021
@jaypipes jaypipes deleted the pci-slot-to-device branch August 29, 2021 16:02
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