-
Notifications
You must be signed in to change notification settings - Fork 2.9k
pkg/specgen/generate: Fix adding host devices on FreeBSD #27545
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
This was not working when emulating Linux container images on FreeBSD. The code to handle host devices on FreeBSD relies on the container having a devfs mount. Unfortunately, the Linux emulation code which adds this was happening after the host device handling. This changes the logic so that host device management happens after Linux emulation. Signed-off-by: Doug Rabson <[email protected]>
lsm5
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.
LGTM
Thanks Doug Rabson for his review and an additional patch, that's also heading upstream, containers/podman#27545 Approved by: dfr (maintainer) Reviewed by: dfr (maintainer) ChangeLog: containers/podman@v5.6.2...v5.7.0 PR: 291014 Sponsored by: tipi.work
|
the code lgtm, but i wonder if it would be prudent to add a test of any kind to prevent regressions in the future? |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dfr, lsm5, mheon The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Ill override and restart tests for now. If we decide that it should have them, we can remove the label. |
The best test for this would be a FreeBSD-only system test. I do have many of the Podman system tests working on the platform but enabling them for CI in some form was put on hold partly due to the lack of FreeBSD official base images (#19788). We now have official images (https://github.com/orgs/freebsd/packages, https://hub.docker.com/u/freebsd) so perhaps we can revisit that idea. |
|
LGTM then |
This was not working when emulating Linux container images on FreeBSD. The code to handle host devices on FreeBSD relies on the container having a devfs mount. Unfortunately, the Linux emulation code which adds this was happening after the host device handling. This changes the logic so that host device management happens after Linux emulation.
Does this PR introduce a user-facing change?