Skip to content

Fix unmount iso/virtual drives after eject #9770

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

Merged
merged 11 commits into from
Sep 12, 2022

Conversation

DavidPerikala
Copy link
Contributor

Resolved / Related Issues
Items resolved / related issues by this PR.

Details of Changes
Add details of changes here.

  • Fixed Unmounting ISO files to remove label after ejecting.
  • I could see that the case for ejecting is same as inserting? How? I guess it should be the same as Removing, right? Or is there a reason for that?
  • I updated and tested it. It works.

Validation
How did you test these changes?

  • Built and ran the app

@gave92
Copy link
Member

gave92 commented Aug 19, 2022

I think it was like that because when you have a "real" CDrom drive (or even a daemon tools-like drive), when you eject a CD you don't want to remove the drive from the list, but just update the label.

@gave92
Copy link
Member

gave92 commented Aug 19, 2022

Plus even if you do this I think the empty drive will still appear in windows explorer.

@DavidPerikala
Copy link
Contributor Author

I think it was like that because when you have a "real" CDrom drive (or even a daemon tools-like drive), when you eject a CD you don't want to remove the drive from the list, but just update the label.

Oh okay. Now I understand.

Plus even if you do this I think the empty drive will still appear in windows explorer.

oops yes. I see it :/
Any hint, how we can proceed here?

@gave92
Copy link
Member

gave92 commented Aug 19, 2022

Uh good question. I guess we could:

  • check if we can invoke the "Eject" verb of the explorer context menu from Files
  • check if dismounting the iso from powershell removes the drive from explorer (e.g. with Dismount-DiskImage -ImagePath "E:\Windows10.iso")

@gave92
Copy link
Member

gave92 commented Aug 23, 2022

Just checked [2]:
Calling e.g. Dismount-DiskImage -ImagePath "E:\Windows10.iso" will also remove the drive from explorer.
Guess this issue is the same as #6439 (for removable drives), we do not Eject the device/ISO correctly.

@gave92
Copy link
Member

gave92 commented Aug 23, 2022

Update: pushed a change to eject using shell. Appears to fix both #6072 and #6439.

@DavidPerikala
Copy link
Contributor Author

Wow. I was looking in a different implementation but great, Thank you @gave92

@gave92
Copy link
Member

gave92 commented Aug 23, 2022

Thanks! Does it work on your end as well then?
I'm not super happy with this solution, I was hoping to accomplish this in UWP without depending on file explorer.
So if you've found a different solution feel free to revert my changes.

@DavidPerikala
Copy link
Contributor Author

Yes I tried and it works.
It's ok. I guess we can use this fix for now?. But I will try an implementation and send a new PR for that it that works.

@DavidPerikala
Copy link
Contributor Author

@gave92 Hold on! It doesn't remove the pendrive. so #6439 is still reproducible. Can you verify once?

@gave92
Copy link
Member

gave92 commented Aug 23, 2022

I can confirm that on my end removing a usb key works (I have only one with which to try though). I'm on windows 10 (if that matters).
Do you get any error message? Do you get the "Safe to remove Hardware" windows notification? Do you see any error logged in "debug_fulltrsut.log" file?

@DavidPerikala
Copy link
Contributor Author

I am on win 11 & So there's safe to remove hardware notification & no error. But the drive is still there on the UI. Once I remove it physically it's removed. Debug log doesn't show any error.

@gave92
Copy link
Member

gave92 commented Aug 23, 2022

  • Does the drive appear in Explorer after you eject it from Files (before you remove it physically)?
  • Does the drive disappear from FIles if you use the code from main (without this PR)?

@DavidPerikala
Copy link
Contributor Author

Does the drive appear in Explorer after you eject it from Files (before you remove it physically)?

No, It's gone from the Explorer. (before removing physically)

Does the drive disappear from FIles if you use the code from main (without this PR)?

No, it doesn't disappear from Files on the main branch

@gave92
Copy link
Member

gave92 commented Aug 23, 2022

No, it doesn't disappear from Files on the main branch

Thanks for confirming. If this PR does not break anything I think we can merge it as it solves at least #6072. We'll leave #6439 open.

@gave92 gave92 assigned yaira2 and unassigned yaira2 Aug 23, 2022
@gave92 gave92 requested a review from d2dyno1 August 23, 2022 20:14
@yaira2 yaira2 requested review from gave92 and d2dyno1 September 12, 2022 03:44
@gave92 gave92 added ready to merge Pull requests that are approved and ready to merge and removed needs - code review labels Sep 12, 2022
@yaira2 yaira2 merged commit 3359556 into files-community:main Sep 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Pull requests that are approved and ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ejecting ISOs does not unmount the virtual drive.
4 participants