dokan-dev / dokany

User mode file system library for windows with FUSE Wrapper
http://dokan-dev.github.io
5.21k stars 661 forks source link

Call for better Mirror example for application to manage file context #1043

Closed kice closed 2 years ago

kice commented 2 years ago

Looks like all operations have requests FileName as parameter. However it suggests that we could store application specific information to DOKAN_FILE_INFO.Context.

I checked some issues and looked at the PDOKAN_OPERATIONS Struct Reference, but it looks like it does not say we have to reopen for ReadFile and WriteFile. Only thing I found useful regarding this was here:

https://github.com/dokan-dev/dokan-dotnet/blob/f30f724babf864863700293b2bcb2edef06bc365/sample/DokanNetMirror/Mirror.cs#L282

So only ReadFile/WriteFile were affected, other operations will be fine for memory mapped file operations? If so, please add this kind of information to the documentation.

Why not add an optional flush callback?

Liryna commented 2 years ago

Hi @kice ,

This case is documented here https://dokan-dev.github.io/dokany-doc/html/ and handled by samples.

Maybe we could document it directly in the callback documentation. Would you be willing to open a pull request for this ?

Fyi there is another issue open for this case https://github.com/dokan-dev/dokany/issues/1016

kice commented 2 years ago

Btw, I just noticed that it will close the handle after re-open the file object. Why not keep the new handle back to context?

https://github.com/dokan-dev/dokany/blob/cea3ea61a27b5eb5ce6ad6686febfe97bea3886d/samples/dokan_mirror/mirror.c#L615-L618

After a file object was successfully created, will dokan call back PDOKAN_OPERATIONS.Cleanup only once during whole filetime? Or just to avoid some magic behavior from the old source code?

I would like to see store a struct in mirror DOKAN_FILE_INFO.Context, like storing not only the file handle, but also the real file path and a operations counter to app file context. This should give a very good example to life time management and thread-safety.

Liryna commented 2 years ago

Exact PDOKAN_OPERATIONS.Cleanup will only be called once by the Windows Kernel.

The mirror sample is only storing the HANDLE. It has no use for a struct and the life time of both would be handled the same.

Doc was improved https://github.com/dokan-dev/dokany/commit/6bbbbc711b2b12b5f2dfb91f6761d90bf01bc038

kice commented 2 years ago

So far I what understand for mirror if using sturct

  1. ZwCreateFile: allocate memory for context, store handle
  2. Some operations
  3. maybe Cleanup if it is Memory Mapped File: close handle, DO NOT free the memory
  4. ReadFile/WriteFile/GetFileInformation. If Cleanup was called, we need to reopen file and make new Context, and MUST release the reopen object before return (commit change to dirty data)
  5. Cleanup: close handle, DO NOT free the memory
  6. CloseFile: release memory

For mem mapped file, we should always return the last-written-to-disk data to read operation, and any write operation must be committed to actual file; for read and write, more or less like we have transferred the internal file object to the kernel as soon as use request to map the file object.

Can we say this way, for mem mapped file, we are also dealing with "coherence", and mirror example choose to ignore the dirty data in the memory, which is the correct way to do?

Some other thought as well, if Cleanup is called upfront, I may easily assume that we are committing the final dirty data to the actual file, and it should be the last operation for that file object. However I does not understand why read would be the last operation, or why Cleanup would call before read (async operation?).

And may I suggest that it is a good idea that check DokanFileInfo->PagingIo before reopen the file object in ReadFile/WriteFile/GetFileInformation, to make sure it is mmap.

Liryna commented 2 years ago

This looks to be exact and follow the documentation. Let me know if you find any issue in the project and I will be glad to look into it.