Skip to content

fs: rework FS interface to be handle based#5143

Merged
MichaelEischer merged 10 commits intorestic:masterfrom
MichaelEischer:fs-handle-interface
Nov 30, 2024
Merged

fs: rework FS interface to be handle based#5143
MichaelEischer merged 10 commits intorestic:masterfrom
MichaelEischer:fs-handle-interface

Conversation

@MichaelEischer
Copy link
Copy Markdown
Member

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() error converts 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): replaces FS.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:

  • Open metadata handle and decide whether a file/directory should be backed up
  • Use MakeReadable to upgrade the handle
  • Read file / directory contents and metadata

A file handle based implementation of the File interface 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_DIRECTORY or 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 tests for all code changes.
  • [ ] I have added documentation for relevant changes (in the manual).
  • [ ] There's a new file in changelog/unreleased/ that describes the changes for our users (see template). No user visible changes expected.
  • I'm done! This pull request is ready for review.

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.
Copy link
Copy Markdown
Member Author

@MichaelEischer MichaelEischer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@MichaelEischer MichaelEischer merged commit 8642049 into restic:master Nov 30, 2024
@MichaelEischer MichaelEischer deleted the fs-handle-interface branch November 30, 2024 14:29
@MichaelEischer MichaelEischer mentioned this pull request Nov 30, 2024
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant