cephfs: add compatiblity wrappers for Go's io/fs interfaces #1089
Conversation
|
@Mergifyio rebase |
✅ Branch has been successfully rebased |
9f3527f to
50b1f55
Compare
ansiwen
left a comment
There was a problem hiding this comment.
In general LGTM, just some curious remarks.
| // under-documented. They mainly seem to try and enforce "clean" paths. | ||
| // look for them and reject them here because ceph libs won't reject on | ||
| // its own | ||
| if strings.HasPrefix(name, "/") || |
There was a problem hiding this comment.
Not sure how performance critical the Open functions is, but there is certainly room for improvement instead of iterating the string several times with a single RE like
var invalidPathPattern = regexp.MustCompile(`^/|/\.{0,2}/|/\.$`)But as I said, probably premature optimization.
| return nil, &fs.PathError{Op: "open", Path: name, Err: errInvalid} | ||
| } | ||
|
|
||
| d, err := mw.mount.OpenDir(name) |
There was a problem hiding this comment.
So, we can assume this is as efficient as using a stat first to check if it is a directory?
There was a problem hiding this comment.
I think that is a pretty fast operation, but if we find that this limits performance later we can come back and fix it because how we determine to open the "file" is not part of the interface and just up to our implementation.
There was a problem hiding this comment.
It's also a preference of mine to avoid "look before you leap" calls when doing fs ops because of race conditions. It is probably silly in this case, but there's the chance a file could be replaced between a stat and open and this is potentially racy. Using the "system call" directly and handling errors from that call is a habit I have developed when doing FS related work.
50b1f55 to
9e2ad03
Compare
|
This pull request has been removed from the queue for the following reason: Pull request #1089 has been dequeued. The pull request rule doesn't match anymore. The following conditions don't match anymore:
You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it. |
|
@Mergifyio rebase |
✅ Branch has been successfully rebased |
9e2ad03 to
0b62e94
Compare
|
@anoopcs9 do you think you find time to look at this prior to the 15th? Thanks! |
|
@Mergifyio rebase |
Make the Directory objects support multiple calls to Close() by nil'ing out the dir.dir property after closing the ceph object. Then subsequent calls to Directory methods need to check that dir.dir is not nil. Add a wrapper for EBADF to mimic the system behavior of trying to operate on a closed directory FD. Signed-off-by: John Mulligan <jmulligan@redhat.com>
Make calling file Read and Write calls a no-op when called with a zero length byte slice and prevent a panic when trying to get the address of the array for these slices. Signed-off-by: John Mulligan <jmulligan@redhat.com>
Add a MountWrapper type that implements fs.FS, as well as private types that implement fs.File, fs.FileInfo, fs.ReadDirFile, and fs.DirEntry. The new wrapper classes sit between the Go stdlib and the native CephFS focused types we provide. Ideally, some of our types would be directly compatible with the fs interfaces but in many cases our code predates io/fs. I hope we consider the io/fs use case for any new types added to the cephfs subpackage in the future - and possibly make compatible types. But for now I think this is sufficent - especially since the "native" types support things like writing and other ceph focused operations. Signed-off-by: John Mulligan <jmulligan@redhat.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
✅ Branch has been successfully rebased |
0b62e94 to
6222cb5
Compare
Fixes: #1088
I've been meaning to look at this for a long time. Thanks for providing the extra motivation to work on it.
First, fix a few things in the lib that the fstest from Golang digs up.
Then implement fscompat.go a file providing a MountWrapper type that implements fs.FS, as well as private types that implement fs.File, fs.FileInfo, fs.ReadDirFile, and fs.DirEntry.
The new wrapper classes sit between the Go stdlib and the native CephFS
focused types we provide. Ideally, some of our types would be directly
compatible with the fs interfaces but in many cases our code predates
io/fs. I hope we consider the io/fs use case for any new types added to
the cephfs subpackage in the future - and possibly make compatible
types. But for now I think this is sufficent - especially since the
"native" types support things like writing and other ceph focused
operations.
Checklist
//go:build ceph_previewmake api-updateto record new APIsNew or infrequent contributors may want to review the go-ceph Developer's Guide including the section on how we track API Status and the API Stability Plan.
The go-ceph project uses mergify. View the mergify command guide for information on how to interact with mergify. Add a comment with
@Mergifyiorebaseto rebase your PR when github indicates that the PR is out of date with the base branch.