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 Manager #50

Closed
4 of 5 tasks
Maxhy opened this issue Aug 30, 2015 · 15 comments
Closed
4 of 5 tasks

Mount Manager #50

Maxhy opened this issue Aug 30, 2015 · 15 comments

Comments

@Maxhy
Copy link
Member

Maxhy commented Aug 30, 2015

Mount Manager is no longer an option on current Windows OS.
This issue to discuss/tracks changes on mountmanager branch to support it correctly on Dokan.

  • PnP support: this would be great of course but doesn't seem mandatory. Non-pnp device can notify mount manager with IOCTL_MOUNTMGR_VOLUME_ARRIVAL_NOTIFICATION (see DokanSendVolumeArrivalNotification). Also, PnP should be an option on DokanOptions for users who doesn't want their File System to be PnP.
  • Volume Links: the way Volume Links (??\Volume{x} and \DosDevice\M) are created is wrong. Mount Manager take care about the first one following IOCTL_MOUNTMGR_VOLUME_ARRIVAL_NOTIFICATION and the second one should be created with IOCTL_MOUNTMGR_CREATE_POINT. Ideally the driver shouldn't have to be aware of volume link names.
  • IoRegisterFileSystem: For details, see previous discussion on Enumerate Alternative Streams #48.
  • Recycle Bin: mounting correctly the volume will require to support high level feature like Recycle Bin (currently end up to a corrupted recycled bin each time the volume is mounted).
  • Remove dokan mount service
@Maxhy
Copy link
Member Author

Maxhy commented Aug 30, 2015

About Volume Links on current branch state.
I successfully created drive letter with SetVolumeMountPoint (IOCTL_MOUNTMGR_CREATE_POINT is behind) which make it visible on mountvol DOS utility with the associated ??\Volume{x} link. But diskpart, fltmc or diskmgmt.msc can't still list it. Moreover FileSpy result to a timeout when attaching to the drive (probably related to fltmc error). Current Dokan code is wrong because it still allocate a custom ??\Volume{x} in addition to the one created by Mount Manager, this could be the reason.

Moreover Mount Manager keep links permanent after a reboot which can result to a current drive letter use error if the computer/dokanmounter was improperly shutdown. DokanLibrary should be able to recover from it and reuse the letter.

@Maxhy
Copy link
Member Author

Maxhy commented Jan 8, 2016

mountmanager branch is now stable enough to be tested :).
Anyone interested about it, feedbacks would be great if you can allocate some time for code review (don't take care about git history, just the general diff with master) and tests before the merge.
Special ping to @marinkobabic @accorp @kenjiuno @js69 😄

Thanks!

@Liryna
Copy link
Member

Liryna commented Jan 9, 2016

Beta release added: https://github.com/dokan-dev/dokany/releases/tag/v0.8.1-beta
It is a quick build so I did not changed the version of the binaries.

@Maxhy , I tested on my win10 and I have to kill explorer to see the device :(

@Maxhy
Copy link
Member Author

Maxhy commented Jan 9, 2016

@Liryna ok thanks. Network drive only or even regular virtual drive doesn't show up without explorer kill?

@Liryna
Copy link
Member

Liryna commented Jan 9, 2016

My fault ! I had to clean my register. Everything work (just have to f5 on explorer to see it)

@viciousviper
Copy link

For what it's worth - all (stable) Dokan.NET tests pass on this version of Dokany.
👍

@Liryna
Copy link
Member

Liryna commented Jan 10, 2016

@Maxhy
Issue: Same as what I faced before - Mirror is started but device is not mounted.

1-Play with dokan: mount / unmount a couple of times with mirror.exe (mirror.exe /r C:\Users /l m)
->Reboot
1-Mount with mirror.exe - mirror.exe /r C:\Users /l m
2>m:
2>dir
...work
1- ctrl +c the mirror.exe
2>dir
The system cannot find the path specified. (normal)
1- mirror.exe /r C:\Users /l m
2>dir <- In the same shell as the first dir
The system cannot find the path specified.

Logs of second mount after reboot: http://pastebin.com/ACMqb0uN

I have to clean register by removing all key starting by # in HKLM/SYSTEM/MountedDevices to be able to mount again.

Thank you @viciousviper, so no changes to make to the wrapper 👍

@Maxhy
Copy link
Member Author

Maxhy commented Jan 10, 2016

Ok @Liryna, I believe it is related to https://msdn.microsoft.com/en-us/library/windows/hardware/ff560461%28v=vs.85%29.aspx
If the input to this IOCTL is ("\DosDevices\X:", NULL, NULL) where X is the current drive letter for the volume indicated in the input triple, the mount manager adds a special entry to its database indicating that the client does not require a drive letter. On subsequent reboots, the mount manager will not assign a default drive letter to the volume.

@Maxhy
Copy link
Member Author

Maxhy commented Jan 10, 2016

@kenjiuno, following your #47 (comment) you will be interested to know that latest commit fb32a62 on mountmanager branch implements correctly UNC provider now.
After building latest change, you can use the mirror this way: mirror.exe /r c:\test /l m /n /u \localhost\myunc

Then you will be able to access it through M drive or \\localhost\myunc\.

@Liryna this commit should also fix your issue. Be aware that mountmanager is now an option. To use it you should start the mirror with /g parameter.

@marinkobabic
Copy link
Contributor

Hi Maxhy

  • What happens when DokanOptions->MountPoint and DokanOptions->UNCName is null?
  • Please add more logging information
  • Now you have the possiblility to run DefineDosDevice optionally if somebody wants to have the drive only visible in his Windows session. This means a new option can be made available. In the actual version of dokan if you have two users logged in into the windows, both of them will see the mounted drives.
  • DokanDbgPrintW, DokanDbgPrint, DbgPrint, DbgPrintW - when to use which of them. It's all mixed
  • DokanSendVolumeArrivalNotification should use DokanSendVolumeCreatePoint or just remove the code from DokanSendVolumeArrivalNotification which is part of DokanSendVolumeCreatePoint

In generally it looks very good!

Thanks for the great job 😄

@Maxhy
Copy link
Member Author

Maxhy commented Jan 10, 2016

Thank you for your positive feedback marinkobabic.

  • If DokanOptions->MountPoint and DokanOptions->UNCName is null then you have two cases:
    • you're using Mount Manager, Windows will automatically assign a drive and notify Dokan through IOCTL_MOUNTDEV_LINK_CREATED.
    • you're not using Mount Manager, you will have a working volume but no way to access it directly. That's said, it could be excepted for some use (ok not a lot of people want to do that).
  • I will try to add more logs but feel free to suggest what is missing if you see something
  • Yes, having an option to mount the drive globally or for current user session was on my mind too 😄. But I want to do that after mountmanager branch merge.
  • Dokan debugging functions is a mess from the original code. Some cleanup should be done but this should be a separate discussion.
  • Indeed, I'm gonna to remove the duplicate code on DokanSendVolumeArrivalNotification (test artifacts). Thank you!

@marinkobabic
Copy link
Contributor

Just an additional Comment:
Actually the creating of a device and mounting of the device is in once place. This seems to me somehow wrong.

  • An application can create a device and then execute mount/unmount several times without the delete and recreate the device.
  • One device can be mounted to different mount points at the same time

This are the reasons I would keep this two things in future versions separate in minimum on the driver level. Actually it's not related to the Mount Manager changes you have made so far.

@Maxhy
Copy link
Member Author

Maxhy commented Jan 10, 2016

This has always bothered me too, a new issue should be opened for this. Please feel free to create it, it will be handled when I will get some free time again or if someone is willing to do it through a PR. But I agree with your comment.

@Maxhy
Copy link
Member Author

Maxhy commented Jan 11, 2016

I'm glad to say after 6 months that this is now merged :).

@Maxhy Maxhy closed this as completed Jan 11, 2016
@Liryna
Copy link
Member

Liryna commented Jan 11, 2016

🍺

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

4 participants