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

opae.io: Fix error handling when binding the vfio-pci driver fails #3095

Merged
merged 1 commit into from
Feb 8, 2024

Conversation

anandaravuri
Copy link
Contributor

@anandaravuri anandaravuri commented Feb 7, 2024

The 'opae.io init' command can fail to bind the vfio-pci driver when the IOMMU is disabled, leaving the system seemingly unable to bind another driver. The fix is to check if the IOMMU is enabled before even attempting to bind the vfio-pci driver.

@anandaravuri anandaravuri requested a review from a team as a code owner February 7, 2024 00:12
@coveralls
Copy link

coveralls commented Feb 7, 2024

Pull Request Test Coverage Report for Build 7818328836

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 64.508%

Totals Coverage Status
Change from base Build 7788148637: 0.0%
Covered Lines: 15809
Relevant Lines: 24507

💛 - Coveralls

@@ -145,6 +145,10 @@ def vfio_init(pci_addr, new_owner='', force=False, **kwargs):
msg = '(0x{:04x},0x{:04x}) at {}'.format(
int(vid_did[0], 16), int(vid_did[1], 16), pci_addr)

if not iommu_enabled():
LOG.error("IOMMU is disabled. Binding failed.")
raise SystemExit("IOMMU is disabled")
Copy link
Contributor

Choose a reason for hiding this comment

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

The LOG.error message is not exactly correct because the binding "will" fail. How about "Binding to vfio-pci will fail because IOMMU is disabled."

Does the Log.error call and the raise SystemExit result in two messages being printed? We only need error message and a non-zero exit.

@fpgamatt
Copy link
Contributor

fpgamatt commented Feb 7, 2024

Can we improve the commit message to something like the following?

opae.io: Fix error handling when binding the vfio-pci driver fails

The 'opae.io init' command can fail to bind the vfio-pci driver when the IOMMU is disabled, leaving the system seemingly unable to bind another driver. The fix is to check if the IOMMU is enabled before even attempting to bind the vfio-pci driver.

 The 'opae.io init' command can fail to bind the vfio-pci driver when the IOMMU is disabled,
 leaving the system seemingly unable to bind another driver.
 The fix is to check if the IOMMU is enabled before even attempting to bind the vfio-pci driver.

Signed-off-by: anandaravuri <[email protected]>
@anandaravuri anandaravuri changed the title Fix: opae.io init command exit with error message in case of iommu n… opae.io: Fix error handling when binding the vfio-pci driver fails Feb 7, 2024
@anandaravuri anandaravuri merged commit ad5352f into master Feb 8, 2024
31 checks passed
@anandaravuri anandaravuri deleted the aravuri/fix_opae.io_no_iommu branch February 8, 2024 22:16
@anandaravuri anandaravuri restored the aravuri/fix_opae.io_no_iommu branch February 12, 2024 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants