fix: directIO read size can exceeds buffer size#68
fix: directIO read size can exceeds buffer size#68shuoranliu wants to merge 1 commit intojacobsa:masterfrom
Conversation
stapelberg
left a comment
There was a problem hiding this comment.
Thanks for your PR!
Out of curiosity, can you elaborate on in which situation you encountered a panic?
| // Start fusermount, passing it a buffer in which to write stderr. | ||
| var stderr bytes.Buffer | ||
|
|
||
| cfg.Options["max_read"] = strconv.Itoa(buffer.MaxReadSize) |
There was a problem hiding this comment.
I think you’ll need to ensure cfg.Options is non-nil here, see the travis test failure.
There was a problem hiding this comment.
Thanks, I've made a modification. To reproduce the issue, just try to invoke a read system call with the size of 512Kbytes which is larger than MaxReadSize, and with O_DIRECT flag.
There was a problem hiding this comment.
Thanks for fixing this! Would it be possible to add a unit test, too, which exercises this behavior? It should fail before your fix commit, and pass afterwards, so that we don’t accidentally re-introduce this bug at some point down the road.
There was a problem hiding this comment.
Is there a guide on how to write ci test cases? I believe it is better to write a test case from user space file system. BTW, I have no idea why ci failed again.
There was a problem hiding this comment.
Not sure I understand your question—there are no “ci test cases”, the CI just runs the go tests of this project :)
This particular failure might have been a flake (sometimes the CI is overloaded), so I have restarted the run.
Since the package uses a fixed-size memory pool for out messages, it is important to tell fuse kernel module what the maximum read size is. Otherwise, a large read will cause a panic. Right now, max readahead size can only affect buffered read. The kernel might still send a large directIO read in spite of the max readahead size. We need to specify "max_read" option during mount. Signed-off-by: Shuoran Liu <shuoranliu@gmail.com>
01da9b6 to
1f74c2f
Compare
Since the package uses a fixed-size memory pool for out messages,
it is important to tell fuse kernel module what the maximum read size is.
Otherwise, a large read will cause a panic.
Right now, max readahead size can only affect buffered read. The kernel
might still send a large directIO read in spite of the max readahead size.
We need to specify "max_read" option during mount.
Signed-off-by: Shuoran Liu shuoranliu@gmail.com