crostini: Block suspend while unmounting sshfs
This change ensures that the attempt to unmount crostini's sshfs
completes before the device is allowed to suspend. Previously, the
device may suspend while the unmount attempt is ongoing.
BUG=968060
TEST=Manually test that sshfs is unmounted and device is allowed to \
suspend.
(cherry picked from commit dfdadaaf4053dc6bd7d53f16e2dff522f1e884d0)
Change-Id: Ifd480c2bf6ad90cb1962786e34b2cc359f1e10f0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1719777
Commit-Queue: Abhishek Bhardwaj <abhishekbh@chromium.org>
Auto-Submit: Abhishek Bhardwaj <abhishekbh@chromium.org>
Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#681456}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1725310
Cr-Commit-Position: refs/branch-heads/3865@{#31}
Cr-Branched-From: 0cdcc6158160790658d1f033d3db873603250124-refs/heads/master@{#681094}
diff --git a/chrome/browser/chromeos/crostini/crostini_manager.cc b/chrome/browser/chromeos/crostini/crostini_manager.cc
index 1bef613..09621ea6 100644
--- a/chrome/browser/chromeos/crostini/crostini_manager.cc
+++ b/chrome/browser/chromeos/crostini/crostini_manager.cc
@@ -2555,11 +2555,13 @@
void CrostiniManager::SuspendImminent(
power_manager::SuspendImminent::Reason reason) {
- // https://crbug.com/968060. Unmount sshfs before suspend.
+ // Block suspend and try to unmount sshfs (https://crbug.com/968060).
+ auto token = base::UnguessableToken::Create();
+ chromeos::PowerManagerClient::Get()->BlockSuspend(token, "CrostiniManager");
file_manager::VolumeManager::Get(profile_)->RemoveSshfsCrostiniVolume(
- file_manager::util::GetCrostiniMountDirectory(profile_));
- SetContainerSshfsMounted(kCrostiniDefaultVmName,
- kCrostiniDefaultContainerName, false);
+ file_manager::util::GetCrostiniMountDirectory(profile_),
+ base::BindOnce(&CrostiniManager::OnRemoveSshfsCrostiniVolume,
+ weak_ptr_factory_.GetWeakPtr(), token));
}
void CrostiniManager::SuspendDone(const base::TimeDelta& sleep_duration) {
@@ -2571,4 +2573,16 @@
}
}
+void CrostiniManager::OnRemoveSshfsCrostiniVolume(
+ base::UnguessableToken power_manager_suspend_token,
+ bool result) {
+ if (result) {
+ SetContainerSshfsMounted(kCrostiniDefaultVmName,
+ kCrostiniDefaultContainerName, false);
+ }
+ // Need to let the device suspend after cleaning up.
+ chromeos::PowerManagerClient::Get()->UnblockSuspend(
+ power_manager_suspend_token);
+}
+
} // namespace crostini
diff --git a/chrome/browser/chromeos/crostini/crostini_manager.h b/chrome/browser/chromeos/crostini/crostini_manager.h
index 7649e5d..4a031a9 100644
--- a/chrome/browser/chromeos/crostini/crostini_manager.h
+++ b/chrome/browser/chromeos/crostini/crostini_manager.h
@@ -15,6 +15,7 @@
#include "base/memory/weak_ptr.h"
#include "base/observer_list.h"
#include "base/optional.h"
+#include "base/unguessable_token.h"
#include "chrome/browser/chromeos/crostini/crostini_simple_types.h"
#include "chrome/browser/chromeos/crostini/crostini_util.h"
#include "chrome/browser/component_updater/cros_component_installer_chromeos.h"
@@ -452,6 +453,13 @@
void SuspendImminent(power_manager::SuspendImminent::Reason reason) override;
void SuspendDone(const base::TimeDelta& sleep_duration) override;
+ // Callback for |RemoveSshfsCrostiniVolume| called from |SuspendImminent| when
+ // the device is allowed to suspend. Removes metadata associated with the
+ // crostini sshfs mount and unblocks a pending suspend.
+ void OnRemoveSshfsCrostiniVolume(
+ base::UnguessableToken power_manager_suspend_token,
+ bool result);
+
void RemoveCrostini(std::string vm_name, RemoveCrostiniCallback callback);
void UpdateVmState(std::string vm_name, VmState vm_state);
diff --git a/chrome/browser/chromeos/extensions/file_manager/private_api_mount.cc b/chrome/browser/chromeos/extensions/file_manager/private_api_mount.cc
index c9be2cc..0e3d694 100644
--- a/chrome/browser/chromeos/extensions/file_manager/private_api_mount.cc
+++ b/chrome/browser/chromeos/extensions/file_manager/private_api_mount.cc
@@ -254,7 +254,7 @@
}
case file_manager::VOLUME_TYPE_CROSTINI:
file_manager::VolumeManager::Get(chrome_details.GetProfile())
- ->RemoveSshfsCrostiniVolume(volume->mount_path());
+ ->RemoveSshfsCrostiniVolume(volume->mount_path(), base::DoNothing());
break;
default:
// Requested unmounting a device which is not unmountable.
diff --git a/chrome/browser/chromeos/file_manager/volume_manager.cc b/chrome/browser/chromeos/file_manager/volume_manager.cc
index 0da12c4d..72dd587 100644
--- a/chrome/browser/chromeos/file_manager/volume_manager.cc
+++ b/chrome/browser/chromeos/file_manager/volume_manager.cc
@@ -662,18 +662,22 @@
crostini::kCrostiniDefaultVmName,
crostini::kCrostiniDefaultContainerName,
base::BindOnce(&VolumeManager::RemoveSshfsCrostiniVolume,
- weak_ptr_factory_.GetWeakPtr(), sshfs_mount_path));
+ weak_ptr_factory_.GetWeakPtr(), sshfs_mount_path,
+ base::BindOnce([](bool result) {
+ if (!result)
+ LOG(ERROR) << "Failed to remove sshfs mount";
+ })));
}
void VolumeManager::RemoveSshfsCrostiniVolume(
- const base::FilePath& sshfs_mount_path) {
+ const base::FilePath& sshfs_mount_path,
+ RemoveSshfsCrostiniVolumeCallback callback) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
- // Call DiskMountManager first since DoUnmountEvent deletes Volume.
disk_mount_manager_->UnmountPath(
sshfs_mount_path.value(), chromeos::UNMOUNT_OPTIONS_NONE,
- chromeos::disks::DiskMountManager::UnmountPathCallback());
- DoUnmountEvent(chromeos::MOUNT_ERROR_NONE,
- *Volume::CreateForSshfsCrostini(sshfs_mount_path));
+ base::BindOnce(&VolumeManager::OnSshfsCrostiniUnmountCallback,
+ base::Unretained(this), sshfs_mount_path,
+ std::move(callback)));
}
bool VolumeManager::RegisterAndroidFilesDirectoryForTesting(
@@ -1411,4 +1415,24 @@
return drive_integration_service_->GetMountPointPath();
}
+void VolumeManager::OnSshfsCrostiniUnmountCallback(
+ const base::FilePath& sshfs_mount_path,
+ RemoveSshfsCrostiniVolumeCallback callback,
+ chromeos::MountError error_code) {
+ if ((error_code == chromeos::MOUNT_ERROR_NONE) ||
+ (error_code == chromeos::MOUNT_ERROR_PATH_NOT_MOUNTED)) {
+ // Remove metadata associated with the mount. It will be a no-op if it
+ // wasn't mounted or unmounted out of band.
+ DoUnmountEvent(chromeos::MOUNT_ERROR_NONE,
+ *Volume::CreateForSshfsCrostini(sshfs_mount_path));
+ if (callback)
+ std::move(callback).Run(true);
+ return;
+ }
+
+ LOG(ERROR) << "Unmounting " << sshfs_mount_path.value() << " failed";
+ if (callback)
+ std::move(callback).Run(false);
+}
+
} // namespace file_manager
diff --git a/chrome/browser/chromeos/file_manager/volume_manager.h b/chrome/browser/chromeos/file_manager/volume_manager.h
index 6ce0775..d775f2ec 100644
--- a/chrome/browser/chromeos/file_manager/volume_manager.h
+++ b/chrome/browser/chromeos/file_manager/volume_manager.h
@@ -280,6 +280,9 @@
const std::string&,
device::mojom::MtpManager::GetStorageInfoCallback)>;
+ // Callback for |RemoveSshfsCrostiniVolume|.
+ using RemoveSshfsCrostiniVolumeCallback = base::OnceCallback<void(bool)>;
+
VolumeManager(
Profile* profile,
drive::DriveIntegrationService* drive_integration_service,
@@ -316,8 +319,11 @@
// Add sshfs crostini volume mounted at specified path.
void AddSshfsCrostiniVolume(const base::FilePath& sshfs_mount_path);
- // Removes specified sshfs crostini mount.
- void RemoveSshfsCrostiniVolume(const base::FilePath& sshfs_mount_path);
+ // Removes specified sshfs crostini mount. Runs |callback| with true if the
+ // mount was removed successfully or wasn't mounted to begin with. Runs
+ // |callback| with false in all other cases.
+ void RemoveSshfsCrostiniVolume(const base::FilePath& sshfs_mount_path,
+ RemoveSshfsCrostiniVolumeCallback callback);
// Removes Downloads volume used for testing.
void RemoveDownloadsDirectoryForTesting();
@@ -442,6 +448,11 @@
// Returns the path of the mount point for drive.
base::FilePath GetDriveMountPointPath() const;
+ void OnSshfsCrostiniUnmountCallback(
+ const base::FilePath& sshfs_mount_path,
+ RemoveSshfsCrostiniVolumeCallback callback,
+ chromeos::MountError error_code);
+
Profile* profile_;
drive::DriveIntegrationService* drive_integration_service_; // Not owned.
chromeos::disks::DiskMountManager* disk_mount_manager_; // Not owned.
