fs: rework FS interface to be handle based#5143
Merged
MichaelEischer merged 10 commits intorestic:masterfrom Nov 30, 2024
Merged
fs: rework FS interface to be handle based#5143MichaelEischer merged 10 commits intorestic:masterfrom
MichaelEischer merged 10 commits intorestic:masterfrom
Conversation
11 tasks
70a1ae8 to
82aac15
Compare
3 tasks
82aac15 to
282b232
Compare
Use O_DIRECTORY to prevent opening any other than a directory in readdirnames.
The actual implementation still relies on file paths, but with the abstraction layer in place, an FS implementation can ensure atomic file accesses in the future.
282b232 to
7ec9c1b
Compare
7ec9c1b to
3457f8f
Compare
The test did not test the case that the type of a file changed unexpectedly.
3457f8f to
b51bf0c
Compare
2 tasks
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR change? What problem does it solve?
This PR is the first step towards better handling disappearing files in the archiver. Currently, the archiver returns errors if a file disappears between calling Lstat on it and reading the remaining attributes / file content. This case can for example occur if a program concurrently renames a temporary file to its final filename.
Besides using snapshots at the filesystem level (not to confuse with snapshots created by restic) the only clean alternative is to open a file handle and use it throughout the archiver.
This PR prepares the FS interface to internally implement the File interface using file handles. The actual conversion to file handles will happen in a follow-up PR (it turned out to be far, far more complex than I've originally anticipated).
The main changes of the FS and File interfaces are the following:
FS.OpenFile(name string, flag int, metadataOnly bool) (File, error): setting metadataOnly creates a metadata only file handle, whereas not setting it opens the file normally. This distinction is necessary as the filesystem APIs of for example Linux require setting O_PATH to open a file handle for a symlink. Both handle variants are implemented using the File interface as properly separating them significantly complicates testing using mocks. For now, metadata handles are no real handles, but instead internally store the filepath and then just issue normal filesystem operations.File.MakeReadable() errorconverts a metadata handle into a regular file handle. For a file-handle-based File implementation this will atomically allow upgrading a metadata to a regular handle.File.ToNode(ignoreXattrListError bool) (*restic.Node, error): replacesFS.NodeFromFileInfo. The main change is that the FileInfo and filepath passing is handled internally. The integration with the File interface also allows the internal handle to be used.The flow in the archiver then looks as follows:
MakeReadableto upgrade the handleA file handle based implementation of the
Fileinterface will be able to use the same handle during the whole backup flow and therefore ensure that the file cannot disappear in the middle of the backup (-> implementation will happen in a follow-up PR).Besides that the PR adds
O_DIRECTORYor a strict restic.Node.Type check before reading the directory content. There are also lots of cleanups to simplify the code, see the individual commits for more details.Was the change previously discussed in an issue or on the forum?
Part of #5021
Prerequisite to fix the xattr part of #3098
Checklist
[ ] I have added documentation for relevant changes (in the manual).[ ] There's a new file inNo user visible changes expected.changelog/unreleased/that describes the changes for our users (see template).