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

Unmounting without dropping self #34

Open
MabezDev opened this issue Aug 19, 2019 · 8 comments
Open

Unmounting without dropping self #34

MabezDev opened this issue Aug 19, 2019 · 8 comments

Comments

@MabezDev
Copy link

MabezDev commented Aug 19, 2019

Currently dropping the fs also drops the inner disk. I think its fairly reasonable to expect to be able remount a filesystem, would you consider adding a non consuming unmount?

Quick demo of what I was thinking:

let fs = FileSystem<_>::new(inner);
// ... some file operations
fs.unmount().unwrap() // was the unmount sucessfull

fs.root_dir().open_file("test.txt").unwrap() // now fails with filesystem not mounted

I suspect a mount function would also have to be added too.

Any suggestions alternative suggestions are welcome.

@MabezDev MabezDev changed the title Return disk when unmounting Unmounting without dropping self Aug 19, 2019
@rafalh
Copy link
Owner

rafalh commented Aug 21, 2019

What about returning inner disk object from unmount() function so you can use it to recreate the FileSystem object when mounting again?
I don't like idea of having FileSystem object in unmounted state. It would unnecessary complicate every function using this struct with additional ifs.
With the proposed solution (returning inner from unmount()) you would be able to write a wrapper working like what you proposed (mount()/unmount()) without complicating existing code.

@MabezDev
Copy link
Author

That could definitely work, the main issue I have though, is that my filesystem is statically allocated. AFAIK you can't drop statics in Rust, so when I try and unmount I get can't move out of self errors.

Looking at the src of unmount, it looks like it just flushed the fsinfo sector and clears the dirty bit, what about exposing that as a public api?

@rafalh
Copy link
Owner

rafalh commented Aug 24, 2019

Exposing API for flushing fsinfo sector and clearing dirty bit would be possible but it wouldn't be enough. What if someone tries to use filesystem when unmounted? What about mounting it again? It should be reinitialized when remounting.
If returning disk from unmount resolves your issue I'm going to implement it and release in next major version.

@rafalh
Copy link
Owner

rafalh commented Aug 24, 2019

@MabezDev Is your disk statically allocated too? Couldn't you just use a statically allocated Option so when unmounted it would be set to empty?

@MabezDev
Copy link
Author

Both are static, the disk isn't the issue, like you said I can wrap it in an option. The problem is the filesystem (dropped on unmount), I really want to avoid wrapping that in an option as it may make it a bit awkward to access, but it might be my only option. I think returning the disk when unmounting is a good change overall even if it doesn't solve my issue directly.

@MabezDev
Copy link
Author

Yeah I've decided wrapping it in an option makes the most sense. I am still using unsafe to magically create my disk again after dropping so if its possible to return the disk on unmount, that would be great :). Thanks for your help!

@rafalh
Copy link
Owner

rafalh commented Aug 24, 2019

I was thinking about returning something similar to std::io::IntoInnerError. What do you think? What worries me that if flush fails you get the wrapping (filesystem) object back (see BufWriter) and there is no way to get the inner disk object... maybe it would be better to always return inner disk object together with error.

@MabezDev
Copy link
Author

What worries me that if flush fails you get the wrapping (filesystem) object back

If the filesystem fails to unmount, it leaves it in a weird state (can you really rely on writing to a fs that failed to unmount?) so I don't think its a good idea to return the filesystem from an unmount call. So I think returning the inner disk along with the result of the unmount seems like the best idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants