localfs: Amend Readdir compliance - #101
Conversation
`DirentSize` helps calculate the `count` parameter for `File.Readdir`. `ReadDir` performs a full read on a `File`. Both work fine as-is but the API should not be considered stable yet.
The count parameter for `Readdir` is a byte count (as in POSIX `getdents`), not an integral count (as in Go). This patch treats the value as such, and does estimates and conversions from 9P -> Go to perform the operations within the expectations of the 9P standard. Excess entries read during operation, are retained on the FID, to allow subsequent calls to resume reading the directory with the appropriate offset. Rewinding the directory is also supported with offset `0`. Works but is not considered stable yet.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #101 +/- ##
==========================================
+ Coverage 58.50% 61.94% +3.43%
==========================================
Files 37 37
Lines 4779 3939 -840
==========================================
- Hits 2796 2440 -356
+ Misses 1776 1283 -493
- Partials 207 216 +9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| const ( | ||
| qidSize = 13 // qid[13] | ||
| offsetSize = 8 // offset[8] | ||
| typeSize = 1 // type[1] | ||
| sringHeaderSize = 2 // length prefix for name[s] | ||
| minDirentSize = qidSize + offsetSize + | ||
| typeSize + sringHeaderSize | ||
| minimumNameLen = 1 | ||
| ) |
There was a problem hiding this comment.
These constants overlap with the ones in p9's direntSize. This should be cleaned up somehow.
Maybe we expose a constant p9.MinimumEntry that callers can reference in addition to just p9.DirentSize? Maybe some other way?
|
|
||
| func (l *Local) getChunkSize(offset uint64, count uint32) (int, error) { | ||
| if offset == 0 { | ||
| return 1, nil // "Peek" is a common enough request. |
There was a problem hiding this comment.
I doubt my own comment here. Real heuristics to back|disprove this would be better.
How common is it for programmers to request a single entry from a directory?
And is this typically implemented as a special fast-case for any file systems?
Would it be better to just use a larger arbitrary chunk size somewhere in the range of 32-256 entries for the first chunk request?
1 participant
Resolves: #100
Seems to work correctly, but this might sit around for a while as I continue to test and look at it again.
Feel free to also test and review it while it's still a draft.
Eventually, I'll switch it over to final, and set a merge deadline afterwards.
This also adds 2 new exposed method:
DirentSizeandReadDir.We'll have to decide if these are worth adding to the public API, and what they should be named + how they should work.
ReadDiris probably sensible, but there's a remark in there about how I normally implement these in calling code that might be a good pattern for this as well.This part of the PR could probably be separated out while we retain the fixes to
Local.Readdir.