ROX-30299: Track io uring operations - #1331
Conversation
Assisted-by: Claude Code (claude-opus-4-6)
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1331 +/- ##
==========================================
- Coverage 35.42% 35.04% -0.38%
==========================================
Files 22 22
Lines 3241 3276 +35
Branches 3241 3276 +35
==========================================
Hits 1148 1148
- Misses 2088 2123 +35
Partials 5 5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Molter73
left a comment
Molter73
left a comment
There was a problem hiding this comment.
I would suggest we put the new executables in a Containerfile and run them from there, this should make it easier to port the test to other systems in the future. There is a tests/containers directory already for storing container related files for testing.
| io_uring_cqe_seen(&ring, cqe); | ||
|
|
||
| /* Write content via io_uring */ | ||
| sqe = io_uring_get_sqe(&ring); |
There was a problem hiding this comment.
Why do we need to get the submission queue on every step? I would expect we could do that just at the start.
| io_uring_submit(&ring); | ||
| ret = io_uring_wait_cqe(&ring, &cqe); | ||
| if (ret < 0) { | ||
| fprintf(stderr, "wait openat: %s\n", strerror(-ret)); | ||
| goto err; | ||
| } | ||
| if (cqe->res < 0) { | ||
| fprintf(stderr, "openat: %s\n", strerror(-cqe->res)); | ||
| io_uring_cqe_seen(&ring, cqe); | ||
| goto err; | ||
| } |
There was a problem hiding this comment.
After the prep step, all sections in the program seem to follow this patter of submit -> wait cqe -> check ret -> check res -> mark cqe as seen, could we somehow wrap this in a function to simplify it?
| io_uring_queue_exit(&ring); | ||
| return 0; | ||
|
|
||
| err: | ||
| io_uring_queue_exit(&ring); | ||
| return 3; |
There was a problem hiding this comment.
Define int result = 3 at the top of this function and this can be reduced to:
| io_uring_queue_exit(&ring); | |
| return 0; | |
| err: | |
| io_uring_queue_exit(&ring); | |
| return 3; | |
| result = 0; | |
| err: | |
| io_uring_queue_exit(&ring); | |
| return result; |
There was a problem hiding this comment.
Is this needed because some systems might not have liburing + devel installed? If so, as my original comment stated, we probably want to containerize the binary and just run that.
There was a problem hiding this comment.
This test seems to only pass if the write done via io_uring is not detected by fact, CI is failing due to cc not being found AFAICT. Is this test failing on your system because fact does see the event?
3 participants
Description
Adds a test for modifying a file with
io_uring. I initially thought that it was possible to modify a file usingio_uringwithout triggering an open event in Fact. This test seems to show that is not the case and that it is indeed not possible to useio_uringto modify a file in a way that does not trigger a Fact event. Two binary files are added in this PR,io_uring_write.candio_uring_write_raw.c. Onlyio_uring_write_raw.cis currently used. If this were to be merged, this would be cleaned up.Even though tracking
io_uringoperation may not be needed, having this test might still be beneficial.Checklist
Automated testing
If any of these don't apply, please comment below.
Testing Performed
The following command was run
It had the following output
Neither the
openoropenatsyscalls are used, but Fact did report the file being open for writing.Here is the test failure when running locally.