rust-fatfs icon indicating copy to clipboard operation
rust-fatfs copied to clipboard

Unmounting without dropping self

Open MabezDev opened this issue 6 years ago • 8 comments

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 avatar Aug 19 '19 21:08 MabezDev

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.

rafalh avatar Aug 21 '19 13:08 rafalh

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?

MabezDev avatar Aug 21 '19 13:08 MabezDev

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 avatar Aug 24 '19 13:08 rafalh

@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?

rafalh avatar Aug 24 '19 14:08 rafalh

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 avatar Aug 24 '19 15:08 MabezDev

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!

MabezDev avatar Aug 24 '19 16:08 MabezDev

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.

rafalh avatar Aug 24 '19 20:08 rafalh

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.

MabezDev avatar Aug 24 '19 23:08 MabezDev