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

fpga: move GetAPIVersion call out of NewPort and NewFME #497

Merged
merged 1 commit into from
Nov 11, 2020

Conversation

bart0sh
Copy link
Member

@bart0sh bart0sh commented Nov 10, 2020

This call is implemented by calling ioctl, which raises
"open /dev/intel-fpga-port.X: operation not permitted" when
called inside unprivileged container.

This breaks FPGA plugin.

Calling this API from fpga_tool is still OK, so
moving calls there should fix the issue.

@bart0sh bart0sh added bug Something isn't working fpga FPGA device plugin related issue labels Nov 10, 2020
@bart0sh bart0sh requested review from rojkov, kad and mythi November 10, 2020 14:21
@bart0sh bart0sh force-pushed the PR0094-move-GetAPIVersion-out-of-NewPort branch 2 times, most recently from 8a2b966 to 0b6b247 Compare November 10, 2020 14:34
@codecov-io
Copy link

codecov-io commented Nov 10, 2020

Codecov Report

Merging #497 (fbccd20) into master (047f333) will increase coverage by 0.29%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #497      +/-   ##
==========================================
+ Coverage   56.41%   56.70%   +0.29%     
==========================================
  Files          30       30              
  Lines        2035     2028       -7     
==========================================
+ Hits         1148     1150       +2     
+ Misses        816      807       -9     
  Partials       71       71              
Impacted Files Coverage Δ
pkg/fpga/dfl_linux.go 0.00% <ø> (ø)
pkg/fpga/intel_fpga_linux.go 0.00% <ø> (ø)
pkg/fpgacontroller/fpgacontroller.go 100.00% <0.00%> (ø)
pkg/deviceplugin/server.go 84.05% <0.00%> (+0.47%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 047f333...2c73e2a. Read the comment docs.

@bart0sh bart0sh force-pushed the PR0094-move-GetAPIVersion-out-of-NewPort branch from 0b6b247 to fbccd20 Compare November 10, 2020 14:38
This call is implemented by calling ioctl, which raises
"open /dev/intel-fpga-port.X: operation not permitted" error
when called inside unprivileged container.

This breaks FPGA plugin.

Calling this API from fpga_tool is still OK, so
moving calls there should fix the issue.

Signed-off-by: Ed Bartosh <eduard.bartosh@intel.com>
@bart0sh bart0sh force-pushed the PR0094-move-GetAPIVersion-out-of-NewPort branch from fbccd20 to 2c73e2a Compare November 10, 2020 14:44
Copy link
Contributor

@rojkov rojkov left a comment

Choose a reason for hiding this comment

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

Looks great! Added nitpicks.

cmd/fpga_tool/fpga_tool.go Show resolved Hide resolved
cmd/fpga_tool/fpga_tool.go Show resolved Hide resolved
cmd/fpga_tool/fpga_tool.go Show resolved Hide resolved
Copy link
Contributor

@rojkov rojkov left a comment

Choose a reason for hiding this comment

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

LGTM

@bart0sh
Copy link
Member Author

bart0sh commented Nov 11, 2020

@mythi @kad would you mind to review [&merge] ?

Copy link
Member

@kad kad left a comment

Choose a reason for hiding this comment

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

lgtm

@kad kad merged commit 75355c9 into intel:master Nov 11, 2020
@bart0sh bart0sh deleted the PR0094-move-GetAPIVersion-out-of-NewPort branch June 13, 2021 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fpga FPGA device plugin related issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants