-
Notifications
You must be signed in to change notification settings - Fork 394
vpp: T7819: do not override driver if it is already done #4857
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
base: current
Are you sure you want to change the base?
Conversation
|
👍 |
Fixes this case:
Traceback (most recent call last):
File "/usr/libexec/vyos/services/vyos-configd", line 156, in run_script
script.apply(c)
File "/usr/libexec/vyos/conf_mode/vpp.py", line 613, in apply
control_host.override_driver(
File "/usr/lib/python3/dist-packages/vyos/vpp/control_host.py", line 138, in override_driver
Path('/sys/module/vfio_pci/drivers/pci:vfio-pci/new_id').write_text(
File "/usr/lib/python3.11/pathlib.py", line 1079, in write_text
with self.open(mode='w', encoding=encoding, errors=errors, newline=newline) as f:
FileExistsError: [Errno 17] File exists
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.
Pull Request Overview
This PR fixes a FileExistsError that occurs when VPP attempts to override a driver that has already been overridden. The fix adds exception handling to gracefully handle the case where the vendor/device ID pair is already registered with vfio-pci.
Key changes:
- Added try-except block around the vfio-pci
new_idwrite operation - FileExistsError is now silently ignored (expected behavior)
- Generic exceptions are caught and logged as warnings
Comments suppressed due to low confidence (1)
python/vyos/vpp/control_host.py:144
- 'except' clause does nothing but pass and there is no explanatory comment.
except FileExistsError:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
CI integration 👍 passed! Details
|
|
Fix works on AWS vm with ENA driver |
|
We have encountered a similar case with something in VPP before, and my previous response still applies here. Whether the suggested solution is appropriate depends on understanding why the drivers are in unexpected states at the moment the function is called. This is what we need to investigate. |
|
@zdc Can you close the PR if the fix is not acceptable? |
Change summary
Do not override already overridden drivers
Fixes this case:
Types of changes
Related Task(s)
Related PR(s)
How to test / Smoketest result
Checklist: