vmm: fix test_epoll_stdin_event#1406
Conversation
dianpopa
force-pushed
the
little_fixes_4
branch
from
5b7a0c4 to
6d2997b
Compare
| // If this unit test is run without a terminal attached (i.e ssh without pseudo terminal, | ||
| // request, jailer with `--deamonize` flag on) EPOLL_CTL_ADD would return EPERM | ||
| // on STDIN_FILENO. | ||
| // Here we are testing that if we receive EPERM, we are no longer asserting | ||
| // against `epoll_context.dispatch_table[epoll_context.stdin_index as usize]` holding any value. |
There was a problem hiding this comment.
Considering these restrictions, is this test ever going to be run by the CI?
There was a problem hiding this comment.
Yes, the test is run by the CI both on x86_64 and aarch64.
The x86_64 hits the first branch of the match (i.e Ok) while aarch64 hits the EPERM branch.
The other solution to this problem would be to run the tests with ssh -t (which allocates a pseudo terminal by force) but i did not choose that path because it contravenes with the --daemonize flag where we want firecracker to redirect all three standard I/O file descriptors to dev/null.
There was a problem hiding this comment.
This is not a new one, it is also described at https://github.com/firecracker-microvm/firecracker/blob/master/vmm/src/lib.rs#L211.
There was a problem hiding this comment.
I see. My worry was that this test can silently fail and hide away problems with our code.
dianpopa
force-pushed
the
little_fixes_4
branch
2 times, most recently
from
7a4019a to
d5c0346
Compare
This unit tests makes sure that calling epoll_ctl_add on STDIN_FILENO succeeds. However, if the unit tests are run through ssh without a pseudo terminal, adding STDIN_FILENO would return EPERM. Adjust unit test to not panic in case of EPERM. Signed-off-by: Diana Popa <[email protected]>
dianpopa
force-pushed
the
little_fixes_4
branch
from
d5c0346 to
91ba014
Compare
6 participants
Reason for This PR
Fixes #1381
Description of Changes
This unit tests makes sure that calling
epoll_ctl_add on STDIN_FILENO succeeds.
However, if the unit tests are run through
ssh without a pseudo terminal, adding STDIN_FILENO
would return EPERM. Adjust unit test to not panic
in case of EPERM.
License Acceptance
By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license.
PR Checklist
[Author TODO: Meet these criteria. Where there are two options, keep one.][Reviewer TODO: Verify that these criteria are met. Request changes if not]git commit -s).unsafecode has been added.