Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mount support on windows without cgo #2862

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Conversation

fgma
Copy link
Contributor

@fgma fgma commented Aug 1, 2020

What does this PR change? What problem does it solve?

This PR adds mount support on window using winfsp and cgofuse. Besides the name implies it it does NOT require cgo on windows and cross compiles without any problems.

Basic functionality is working but code needs cleanup and is lacking documentation and refactoring.

To test it I prepared zip file that contains some bat files to demo the functionality:

mount_demo.zip

Please compile restic for windows and install winfsp. Then unzip the provided zip file. Copy the compiled binary to the mount_demo folder and run demo.bat. This will do the following:

  1. run init.bat: initialize a repository
  2. run backup.bat: create some snapshots from the provided data folder
  3. run mount.bat: mount the created repsoitory as drive r:

If the drive does not show up immediately press F5 to refresh windows explorer.

I hope to get some feedback on how to continue working on this PR.

Was the change discussed in an issue or in the forum before?

#2791

Checklist

  • I have read the Contribution Guidelines
  • I have enabled maintainer edits for this PR
  • I have added tests for all changes in this PR
  • I have added documentation for the changes (in the manual)
  • There's a new file in changelog/unreleased/ that describes the changes for our users (template here)
  • I have run gofmt on the code in all commits
  • All commit messages are formatted in the same style as the other commits in the repo
  • I'm done, this Pull Request is ready for review

@AlexpFr
Copy link

AlexpFr commented Mar 22, 2021

fgma, is it possible for you to update this PR for curent master branch ?
Thanks

@fgma
Copy link
Contributor Author

fgma commented Mar 22, 2021

@AlexpFr I could but the idea was to replace the backend code with a common abstraction layer used also for webdav or other frontends: #2860

@AlexpFr
Copy link

AlexpFr commented Mar 22, 2021

Yes I saw this PR as well but haven't tested it yet.

I am currently using winfsp to mount sshfs network drives and it works really well.
That’s why I’m interested in your work.

What method do you currently use to mount backups on windows? Fuse, webdav or another?

I quickly tried to integrate your PR into the master branch but there have been a lot of commits since then, I think it would be wise to let you review your code (if you find it useful).

@ioncodes

This comment has been minimized.

@rawtaz

This comment has been minimized.

@JsBergbau

This comment has been minimized.

@rawtaz

This comment has been minimized.

@jkiebzak

This comment has been minimized.

@rawtaz

This comment has been minimized.

@jkiebzak
Copy link

jkiebzak commented Sep 21, 2021 via email

@ghost
Copy link

ghost commented May 10, 2022

Replying to helpfully bump this PR.

@fgma
Copy link
Contributor Author

fgma commented May 11, 2022

I'd really like to finish this but I've not received any feedback yet. I've already asked in #2860 because it is related and could share code.

@MichaelEischer
Copy link
Member

MichaelEischer commented Jul 30, 2022

The main reason why there hasn't been any feedback to this PR so far is that it is an absolutely gigantic amount of code. Properly reviewing the whole PR would require at least a full day, most likely more. The other problem is that the PR looks like it needs a substantial rewrite. So I'm afraid that the feedback will be mostly bad news:

As already indicated by the number of added and removed lines of code, there is essentially zero code sharing between the existing fuse implementation and this PR. That is not acceptable, we cannot maintain two completely separate implementations as that will just end in a maintenance nightmare. The only acceptable way to structure the implementation is to separate the fuse code into a data model (shared by both implementations!!!) and presentation code which just transforms that data model into the format required for the particular fuse library. For the snapshot pseudo-directories most of the refactoring work is done by #2913 which separates things into a generic SnapshotDirStructure and the fuse library adaption by SnapshotDir.

To name a few specific problems (the list is probably far from complete):

  • Even though the two implementations are quite different, it feels like the completely different file name structure is overkill. Why not rename fs_node_root_windows.go to root_windows.go and so on?
  • FsNodeFiltered, FsNodeRoot and FsNodeSnapshotsDir can largely be replaced by the unified implementation in mount: Make snapshots dir structure customizable #2913.
  • FsNodeSnapshotDir should share more code with the existing file.go implementation, in particular I have the Read method in mind.
  • The mount function in cmd/restic/cmd_mount.go could also share more code between the fuse implementations.

My impression after scrolling through the code is that the FsNode implementation which uses path []string is an unmitigated disaster. It looks like a considerable amount of the whole code is just busy with constantly reimplementing path walking, which is necessary again and again for every single method and thus is likely broken in some places. An example of the latter is ReadDir and GetAttributesinFsNodeSnapshotsDir, only one of them calls NewFsNodeSnapshotDirFromSnapshot`, such that the behavior differs depending on the order in which these methods are called.

The path walking must be implemented generically and not repeated for each node (and especially not for each method), such that the resulting interface is similar to that of the currently used fuse library. If that would be too inefficient, I'd be willing to settle for one Lookup method per FsNode type, but definitely not one per method.

The other question is the choice of fuse library. Is cgofuse + winfsp the only possible solution for windows? Or is there maybe a CGO-less fuse library which works for all operating systems? The latter would be a much better solution than having two different fuse libraries in use. Especially, as the currently used library is no longer supported for recent macOS versions.

@ei-ke
Copy link

ei-ke commented Aug 11, 2022

If this PR would require that much rewriting maybe it would be better to put this work into WebDAV (#485)? Especially as this could also help Mac users accessing snapshots in an easier way (#3096). Or would cgofuse also solve the Mac issue?

Personally I have nearly no programming knowledge but wanted to dive into Go since a while. But I guess as a first "something" after working through a book or two this might be a bold project...

@fgma
Copy link
Contributor Author

fgma commented Aug 11, 2022

I consider the current state of this PR a tech demo. As I already stated in my initial post:

Basic functionality is working but code needs cleanup and is lacking documentation and refactoring.

To continue working on it I need some specific feedback about the future of restic:

  • Will cgofuse ever be accepted as a dependency?
  • Should there be a common backend shared by fuse/cgofuse/webdav which create some kind of abstract filesystem interface? How should the interface look like?
  • What about the webdav backend? Is it dead or should this be a higher priority for now? I already offered help there multiple times.

@JsBergbau
Copy link
Contributor

What about the webdav backend?

Is it possible to serve owership and permissions with webdav backend? I guess this is not possible. If so Webdav can't be a replacement for fuse mounts.

@MichaelEischer
Copy link
Member

MichaelEischer commented Aug 18, 2022

To continue working on it I need some specific feedback about the future of restic:

  • Will cgofuse ever be accepted as a dependency?
  • Should there be a common backend shared by fuse/cgofuse/webdav which create some kind of abstract filesystem interface? How should the interface look like?
  • What about the webdav backend? Is it dead or should this be a higher priority for now? I already offered help there multiple times.
  • We don't want CGO as a build dependency. For Windows cgofuse would tolerable as it does not actually require CGO. This directly leads to my previous question: are there other alternative libraries which could cover all platforms and not require CGO?
  • That common backend already exists to a large part: SnapshotsDirStructure and the Snapshot/Tree/Node/Repository can already be shared. The remaining easily sharable code of the current fuse implementation seems to largely concentrate on reading file parts and loading a directory (approx. 200 lines of code). Most other code just appears to be busy with converting the existing data model into a format suitable for the fuse library.
  • The only "significant" remaining code duplication after that would be the dir and the SnapshotsDir structs which both represent directories. Both have completely different behavior, so the only duplication are a few lines of code returning the directory attributes and entries. That is small enough to get rid of later on. (If necessary we can let the SnapshotsDirStructure generate fake restic.Nodes and get rid of the little duplication that way) So right now, introducing a new abstraction sounds like overkill to me.
  • Regarding WebDAV: we're still interested in such a mount backend. Right now, my impression is that interest in a windows fuse implementation is somewhat similar to that in WebDAV. So I don't have a strong opinion in either direction. But I haven't looked into the limitations of WebDAV.

@ei-ke
Copy link

ei-ke commented Aug 18, 2022

@JsBergbau
RFC3744 is relating to this.
Further my intention was not to get rid of fuse mounts but rather have a second option for windows/macos.

@MichaelEischer
Copy link
Member

  • We don't want CGO as a build dependency. For Windows cgofuse would tolerable as it does not actually require CGO. This directly leads to my previous question: are there other alternative libraries which could cover all platforms and not require CGO?

I took a look myself and ended up with the following libraries: #3096 (comment) . Of those only cgofuse works on Windows and does not require CGO. If we decided to accept cgofuse, then we'd also get support for several other operating systems for free.

In short, we can stick with cgofuse for windows.

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.

None yet

8 participants