From b4e033ed4c8b99456596ae5739525cc478a33f36 Mon Sep 17 00:00:00 2001 From: Momoko Hattori Date: Thu, 22 Feb 2024 16:07:24 +0900 Subject: [PATCH] vold: Unmount StubVolume disks before unmounting EmulatedVolumes The current shutdown / reset logic in VolumeManager unmounts EmulatedVolume first, and unmounts the other disks. In ARC (Android on ChromeOS), ChromeOS Downloads directory (exposed from ChromeOS to Android as a disk having StubVolume) is bind-mounted to /data/media/0/Download in the ARC-customized version of StubVolume::doMount() (http://shortn/_lKaAhTLhY3), and the current unmount order causes EmulatedVolume not to be cleanly unmounted. This patch hence changes the order of the unmount of volumes to first unmount StubVolume disks, then unmount the EmulatedVolumes, then unmount the non-StubVolume disks. Bug: 304369444 Test: On an Android phone, create a virtual public volume with the following commands on adb shell (taken from android.scopedstorage.cts.lib.TestUtils.createNewPublicVolume()): $ sm set-force-adoptable on $ sm set-virtual-disk true $ sm list-disks # <- This returns the virtual disk name $ sm partition public Then, run `vdc volume reset` on lynx adb shell, observe logcat from vold and check that no error is observed. Test: Run `vdc volume reset` on ARC adb shell, and confirm that: * Without this patch, the primary emulated volume fails to unmount with "Device or resource busy", followed by MyFiles volume unmount. * With this patch, MyFiles volume is unmounted before the primary emulated volume, and no error is observed. Change-Id: I54f60e3320574ccf8d3589545ff77967fff14fc7 --- VolumeManager.cpp | 53 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 40 insertions(+), 13 deletions(-) diff --git a/VolumeManager.cpp b/VolumeManager.cpp index c981f2d..a1ac20d 100644 --- a/VolumeManager.cpp +++ b/VolumeManager.cpp @@ -921,25 +921,34 @@ int VolumeManager::abortFuse() { int VolumeManager::reset() { // Tear down all existing disks/volumes and start from a blank slate so // newly connected framework hears all events. + + // Destroy StubVolume disks. This needs to be done before destroying + // EmulatedVolumes because in ARC (Android on ChromeOS), ChromeOS Downloads + // directory (which is in a StubVolume) is bind-mounted to + // /data/media/0/Download. + // We do not recreate StubVolumes here because they are managed from outside + // Android (e.g. from ChromeOS) and their disk recreation on reset events + // should be handled from outside by calling createStubVolume() again. + for (const auto& disk : mDisks) { + if (disk->isStub()) { + disk->destroy(); + } + } + // Remove StubVolume from both mDisks and mPendingDisks. + const auto isStub = [](const auto& disk) { return disk->isStub(); }; + mDisks.remove_if(isStub); + mPendingDisks.remove_if(isStub); + for (const auto& vol : mInternalEmulatedVolumes) { vol->destroy(); } mInternalEmulatedVolumes.clear(); - // Destroy and recreate all disks except that StubVolume disks are just - // destroyed and removed from both mDisks and mPendingDisks. - // StubVolumes are managed from outside Android (e.g. from Chrome OS) and - // their disk recreation on reset events should be handled from outside by - // calling createStubVolume() again. + // Destroy and recreate non-StubVolume disks. for (const auto& disk : mDisks) { disk->destroy(); - if (!disk->isStub()) { - disk->create(); - } + disk->create(); } - const auto isStub = [](const auto& disk) { return disk->isStub(); }; - mDisks.remove_if(isStub); - mPendingDisks.remove_if(isStub); updateVirtualDisk(); mAddedUsers.clear(); @@ -958,11 +967,20 @@ int VolumeManager::shutdown() { return 0; // already shutdown } android::vold::sSleepOnUnmount = false; + // Destroy StubVolume disks before destroying EmulatedVolumes (see the + // comment in VolumeManager::reset()). + for (const auto& disk : mDisks) { + if (disk->isStub()) { + disk->destroy(); + } + } for (const auto& vol : mInternalEmulatedVolumes) { vol->destroy(); } for (const auto& disk : mDisks) { - disk->destroy(); + if (!disk->isStub()) { + disk->destroy(); + } } mInternalEmulatedVolumes.clear(); @@ -978,11 +996,20 @@ int VolumeManager::unmountAll() { ATRACE_NAME("VolumeManager::unmountAll()"); // First, try gracefully unmounting all known devices + // Unmount StubVolume disks before unmounting EmulatedVolumes (see the + // comment in VolumeManager::reset()). + for (const auto& disk : mDisks) { + if (disk->isStub()) { + disk->unmountAll(); + } + } for (const auto& vol : mInternalEmulatedVolumes) { vol->unmount(); } for (const auto& disk : mDisks) { - disk->unmountAll(); + if (!disk->isStub()) { + disk->unmountAll(); + } } // Worst case we might have some stale mounts lurking around, so