-
Notifications
You must be signed in to change notification settings - Fork 201
pci: add parent device address #431
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
Conversation
| ID: d.ProgrammingInterface.ID, | ||
| Name: d.ProgrammingInterface.Name, | ||
| }, | ||
| IOMMUGroup: d.IOMMUGroup, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I forgot add this in #430 so I am trying to sneakily add it here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved this into a separate commit
f809ed8 to
f931ac1
Compare
f931ac1 to
1ee51ca
Compare
ffromani
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine adding the parent address. Quick review, I'm probably overly cautious over backward compatibility
| // The PCI address of the parent device | ||
| ParentAddress string `json:"parent_address"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm contemplating if we should add this at the end of the struct for abundant (excessive?) backward compatibility concerns. But OTOH we mostly (99%) serialize back/from YAML/JSON and clients just recompile, so it's probably OK
pkg/pci/pci_linux.go
Outdated
| if pciaddr.FromString(parentAddr) != nil { | ||
| return parentAddr | ||
| } | ||
|
|
||
| return "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I prefer to keep the happy path on the left:
if pciaddr.FromString(parentAddr) == nil {
return ""
}
return parentAddr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay; I fixed it.
1ee51ca to
3ba2663
Compare
the parent device address is important, too (like IOMMU group), when using pci-passthrough as the both devices should be passed through together Signed-off-by: Christoph Ostarek <christoph@zededa.com>
Signed-off-by: Christoph Ostarek <christoph@zededa.com>
3ba2663 to
330084d
Compare
jaypipes
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all looks good to me, thank you @christoph-zededa :)
the parent device address is important, too (like IOMMU group), when using pci-passthrough as the both devices should be passed through together