-
Notifications
You must be signed in to change notification settings - Fork 3k
erts: implement AF_VSOCK #10055
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: master
Are you sure you want to change the base?
erts: implement AF_VSOCK #10055
Conversation
CT Test Results 4 files 202 suites 1h 54m 16s ⏱️ For more details on these failures, see this check. Results for commit 3922639. ♻️ This comment has been updated with latest results. To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass. See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally. Artifacts// Erlang/OTP Github Action Bot |
81fd3c2
to
2ed40be
Compare
And by the way, our dialyzer checks fail with:
|
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.
The implementation looks good overall!
@bmk's comment about the Cid
badmatch must be fixed - it is a bug, visible in the PR check "Run static analysis".
The test suite should jog that formatting function, just to get code coverage and verify that it doesn't crash, e.g by printing the socket address to the test log.
The failed PR check "Create SBOM" can be ignored.
The PR check "CT Test Results" fails on testcase socket_SUITE:reg_s_single_open_and_close_and_count/1
, with {badmatch,{error,eperm}}
, probably when creating a socket in domain vsock
. On my laptop the testcase has no permission problems, so this may be related to the docker installation on GitHub. Nevertheless the question is if socket:is_supported(vsock)
should verify permissions before returning true
, or if the test suite should have an extended test for eperm
in has_support_vsock/0
.
The same test case doesn't verify the number of vsock
sockets, that needs to be added a'la local
sockets.
Many more test cases needs to be added, but that is known already...
thanks for taking a look. I'm kind of lost with the test suite, I'm not really sure where anything should go or how it's meant to be structured. if I wanted to add a unit test for address formatting, where should I put it? |
I will try to come up with a starting direction for you, soon... |
I duplicated one AF_LOCAL testcase to also run for AF_VSOCK. The first 3 diff hunks are the added test case. The rest of the diff should be common code for all/many I hope you can use this commit as a template... |
From #7719, i've rebased it, made some small changes, and started to add tests.