0

cros-disks: Plumb through mount options.

Add parameter to allow setting custom mount options for Mount() D-Bus
call (e.g. for network or FUSE mounts).

BUG=chromium:832507

Change-Id: Ie645bdddd3331fa14185a7691f548b9edb36562f
Reviewed-on: https://chromium-review.googlesource.com/1053029
Commit-Queue: Sergei Datsenko <dats@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557764}
This commit is contained in:
Sergei Datsenko
2018-05-11 01:52:56 +00:00
committed by Commit Bot
parent 0ea4e59bf6
commit d1924818aa
18 changed files with 72 additions and 35 deletions

@ -193,7 +193,7 @@ void FileManagerPrivateAddMountFunction::RunAfterMarkCacheFileAsMounted(
disk_mount_manager->MountPath(
file_path.AsUTF8Unsafe(),
base::FilePath(display_name.Extension()).AsUTF8Unsafe(),
display_name.AsUTF8Unsafe(), chromeos::MOUNT_TYPE_ARCHIVE,
display_name.AsUTF8Unsafe(), {}, chromeos::MOUNT_TYPE_ARCHIVE,
chromeos::MOUNT_ACCESS_MODE_READ_WRITE);
}

@ -10,17 +10,21 @@ FakeDiskMountManager::MountRequest::MountRequest(
const std::string& source_path,
const std::string& source_format,
const std::string& mount_label,
const std::vector<std::string>& mount_options,
chromeos::MountType type,
chromeos::MountAccessMode access_mode)
: source_path(source_path),
source_format(source_format),
mount_label(mount_label),
mount_options(mount_options),
type(type),
access_mode(access_mode) {}
FakeDiskMountManager::MountRequest::MountRequest(const MountRequest& other) =
default;
FakeDiskMountManager::MountRequest::~MountRequest() = default;
FakeDiskMountManager::UnmountRequest::UnmountRequest(
const std::string& mount_path,
chromeos::UnmountOptions options)
@ -69,13 +73,15 @@ void FakeDiskMountManager::EnsureMountInfoRefreshed(
callback.Run(true);
}
void FakeDiskMountManager::MountPath(const std::string& source_path,
const std::string& source_format,
const std::string& mount_label,
chromeos::MountType type,
chromeos::MountAccessMode access_mode) {
mount_requests_.emplace_back(source_path, source_format, mount_label, type,
access_mode);
void FakeDiskMountManager::MountPath(
const std::string& source_path,
const std::string& source_format,
const std::string& mount_label,
const std::vector<std::string>& mount_options,
chromeos::MountType type,
chromeos::MountAccessMode access_mode) {
mount_requests_.emplace_back(source_path, source_format, mount_label,
mount_options, type, access_mode);
const MountPointInfo mount_point(
source_path,

@ -24,13 +24,16 @@ class FakeDiskMountManager : public chromeos::disks::DiskMountManager {
MountRequest(const std::string& source_path,
const std::string& source_format,
const std::string& mount_label,
const std::vector<std::string>& mount_options,
chromeos::MountType type,
chromeos::MountAccessMode access_mode);
MountRequest(const MountRequest& other);
~MountRequest();
std::string source_path;
std::string source_format;
std::string mount_label;
std::vector<std::string> mount_options;
chromeos::MountType type;
chromeos::MountAccessMode access_mode;
};
@ -44,7 +47,7 @@ class FakeDiskMountManager : public chromeos::disks::DiskMountManager {
};
struct RemountAllRequest {
RemountAllRequest(chromeos::MountAccessMode access_mode);
explicit RemountAllRequest(chromeos::MountAccessMode access_mode);
chromeos::MountAccessMode access_mode;
};
@ -79,6 +82,7 @@ class FakeDiskMountManager : public chromeos::disks::DiskMountManager {
void MountPath(const std::string& source_path,
const std::string& source_format,
const std::string& mount_label,
const std::vector<std::string>& mount_options,
chromeos::MountType type,
chromeos::MountAccessMode access_mode) override;
// In order to simulate asynchronous invocation of callbacks after unmount

@ -560,7 +560,8 @@ void VolumeManager::OnAutoMountableDiskEvent(
// format if the second argument is empty. The third argument (mount
// label) is not used in a disk mount operation.
disk_mount_manager_->MountPath(disk.device_path(), std::string(),
mount_label, chromeos::MOUNT_TYPE_DEVICE,
mount_label, {},
chromeos::MOUNT_TYPE_DEVICE,
GetExternalStorageAccessMode(profile_));
mounting = true;
}
@ -677,7 +678,7 @@ void VolumeManager::OnFormatEvent(
// empty. The third argument (mount label) is not used in a disk mount
// operation.
disk_mount_manager_->MountPath(device_path, std::string(),
std::string(),
std::string(), {},
chromeos::MOUNT_TYPE_DEVICE,
GetExternalStorageAccessMode(profile_));
}
@ -723,7 +724,7 @@ void VolumeManager::OnRenameEvent(
// disk when it was first time mounted (to preserve mount point regardless
// of the volume name).
disk_mount_manager_->MountPath(device_path, std::string(), mount_label,
chromeos::MOUNT_TYPE_DEVICE,
{}, chromeos::MOUNT_TYPE_DEVICE,
GetExternalStorageAccessMode(profile_));
bool successfully_renamed = error_code == chromeos::RENAME_ERROR_NONE;

@ -594,7 +594,8 @@ TEST_F(VolumeManagerTest, OnMountEvent_Remounting) {
chromeos::DEVICE_TYPE_UNKNOWN, 0, false, false, false, false, false,
false, "", ""));
disk_mount_manager_->AddDiskForTest(std::move(disk));
disk_mount_manager_->MountPath("device1", "", "", chromeos::MOUNT_TYPE_DEVICE,
disk_mount_manager_->MountPath("device1", "", "", {},
chromeos::MOUNT_TYPE_DEVICE,
chromeos::MOUNT_ACCESS_MODE_READ_WRITE);
const chromeos::disks::DiskMountManager::MountPointInfo kMountPoint(
@ -745,9 +746,11 @@ TEST_F(VolumeManagerTest, OnFormatEvent_CompletedFailed) {
TEST_F(VolumeManagerTest, OnExternalStorageDisabledChanged) {
// Here create two mount points.
disk_mount_manager_->MountPath("mount1", "", "", chromeos::MOUNT_TYPE_DEVICE,
disk_mount_manager_->MountPath("mount1", "", "", {},
chromeos::MOUNT_TYPE_DEVICE,
chromeos::MOUNT_ACCESS_MODE_READ_WRITE);
disk_mount_manager_->MountPath("mount2", "", "", chromeos::MOUNT_TYPE_DEVICE,
disk_mount_manager_->MountPath("mount2", "", "", {},
chromeos::MOUNT_TYPE_DEVICE,
chromeos::MOUNT_ACCESS_MODE_READ_ONLY);
// Initially, there are two mount points.

@ -369,7 +369,7 @@ class KioskFakeDiskMountManager : public file_manager::FakeDiskMountManager {
void MountUsbStick() {
DCHECK(!usb_mount_path_.empty());
MountPath(usb_mount_path_, "", "", chromeos::MOUNT_TYPE_DEVICE,
MountPath(usb_mount_path_, "", "", {}, chromeos::MOUNT_TYPE_DEVICE,
chromeos::MOUNT_ACCESS_MODE_READ_ONLY);
}

@ -88,7 +88,7 @@ class DriveFsHost::MountState : public mojom::DriveFsDelegate,
// TODO(sammc): Switch the mount type once a more appropriate mount type
// exists.
chromeos::disks::DiskMountManager::GetInstance()->MountPath(
source_path_, "", "", chromeos::MOUNT_TYPE_ARCHIVE,
source_path_, "", "", {}, chromeos::MOUNT_TYPE_ARCHIVE,
chromeos::MOUNT_ACCESS_MODE_READ_WRITE);
auto bootstrap =

@ -252,7 +252,7 @@ class DriveFsHostTest : public ::testing::Test,
MountPath(testing::AllOf(
testing::StartsWith("drivefs://"),
testing::EndsWith("@/path/to/profile/GCache/v2/g-ID")),
"", "", _, chromeos::MOUNT_ACCESS_MODE_READ_WRITE))
"", "", _, _, chromeos::MOUNT_ACCESS_MODE_READ_WRITE))
.WillOnce(testing::SaveArg<0>(&source));
mojom::DriveFsBootstrapPtrInfo bootstrap;
@ -445,7 +445,7 @@ TEST_F(DriveFsHostTest, MountWhileAlreadyMounted) {
}
TEST_F(DriveFsHostTest, UnsupportedAccountTypes) {
EXPECT_CALL(*disk_manager_, MountPath(_, _, _, _, _)).Times(0);
EXPECT_CALL(*disk_manager_, MountPath(_, _, _, _, _, _)).Times(0);
const AccountId unsupported_accounts[] = {
AccountId::FromGaiaId("ID"),
AccountId::FromUserEmail("test2@example.com"),

@ -112,6 +112,7 @@ class CrosDisksClientImpl : public CrosDisksClient {
void Mount(const std::string& source_path,
const std::string& source_format,
const std::string& mount_label,
const std::vector<std::string>& mount_options,
MountAccessMode access_mode,
RemountOption remount,
VoidDBusMethodCallback callback) override {
@ -120,9 +121,9 @@ class CrosDisksClientImpl : public CrosDisksClient {
dbus::MessageWriter writer(&method_call);
writer.AppendString(source_path);
writer.AppendString(source_format);
std::vector<std::string> mount_options =
ComposeMountOptions(mount_label, access_mode, remount);
writer.AppendArrayOfStrings(mount_options);
std::vector<std::string> options =
ComposeMountOptions(mount_options, mount_label, access_mode, remount);
writer.AppendArrayOfStrings(options);
proxy_->CallMethod(
&method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
base::BindOnce(&CrosDisksClientImpl::OnVoidMethod,
@ -674,12 +675,13 @@ base::FilePath CrosDisksClient::GetRemovableDiskMountPoint() {
// static
std::vector<std::string> CrosDisksClient::ComposeMountOptions(
const std::vector<std::string>& options,
const std::string& mount_label,
MountAccessMode access_mode,
RemountOption remount) {
std::vector<std::string> mount_options(
kDefaultMountOptions,
kDefaultMountOptions + arraysize(kDefaultMountOptions));
std::vector<std::string> mount_options = options;
mount_options.insert(mount_options.end(), kDefaultMountOptions,
kDefaultMountOptions + base::size(kDefaultMountOptions));
switch (access_mode) {
case MOUNT_ACCESS_MODE_READ_ONLY:
mount_options.push_back(kReadOnlyOption);
@ -697,6 +699,7 @@ std::vector<std::string> CrosDisksClient::ComposeMountOptions(
base::StringPrintf("%s=%s", kMountLabelOption, mount_label.c_str());
mount_options.push_back(mount_label_option);
}
return mount_options;
}

@ -322,6 +322,7 @@ class CHROMEOS_EXPORT CrosDisksClient : public DBusClient {
virtual void Mount(const std::string& source_path,
const std::string& source_format,
const std::string& mount_label,
const std::vector<std::string>& mount_options,
MountAccessMode access_mode,
RemountOption remount,
VoidDBusMethodCallback callback) = 0;
@ -379,6 +380,7 @@ class CHROMEOS_EXPORT CrosDisksClient : public DBusClient {
// Composes a list of mount options.
static std::vector<std::string> ComposeMountOptions(
const std::vector<std::string>& options,
const std::string& mount_label,
MountAccessMode access_mode,
RemountOption remount);

@ -163,7 +163,7 @@ TEST(CrosDisksClientTest, ComposeMountOptions) {
std::string kExpectedMountLabelOption =
std::string("mountlabel=") + kMountLabel;
std::vector<std::string> rw_mount_options =
CrosDisksClient::ComposeMountOptions(kMountLabel,
CrosDisksClient::ComposeMountOptions({}, kMountLabel,
MOUNT_ACCESS_MODE_READ_WRITE,
REMOUNT_OPTION_MOUNT_NEW_DEVICE);
ASSERT_EQ(5U, rw_mount_options.size());
@ -174,7 +174,7 @@ TEST(CrosDisksClientTest, ComposeMountOptions) {
EXPECT_EQ(kExpectedMountLabelOption, rw_mount_options[4]);
std::vector<std::string> ro_mount_options =
CrosDisksClient::ComposeMountOptions(kMountLabel,
CrosDisksClient::ComposeMountOptions({}, kMountLabel,
MOUNT_ACCESS_MODE_READ_ONLY,
REMOUNT_OPTION_MOUNT_NEW_DEVICE);
ASSERT_EQ(5U, ro_mount_options.size());
@ -186,7 +186,7 @@ TEST(CrosDisksClientTest, ComposeMountOptions) {
std::vector<std::string> remount_mount_options =
CrosDisksClient::ComposeMountOptions(
kMountLabel, MOUNT_ACCESS_MODE_READ_WRITE,
{}, kMountLabel, MOUNT_ACCESS_MODE_READ_WRITE,
REMOUNT_OPTION_REMOUNT_EXISTING_DEVICE);
ASSERT_EQ(6U, remount_mount_options.size());
EXPECT_EQ("nodev", remount_mount_options[0]);
@ -195,6 +195,19 @@ TEST(CrosDisksClientTest, ComposeMountOptions) {
EXPECT_EQ("rw", remount_mount_options[3]);
EXPECT_EQ("remount", remount_mount_options[4]);
EXPECT_EQ(kExpectedMountLabelOption, remount_mount_options[5]);
std::vector<std::string> custom_mount_options =
CrosDisksClient::ComposeMountOptions({"foo", "bar=baz"}, kMountLabel,
MOUNT_ACCESS_MODE_READ_WRITE,
REMOUNT_OPTION_MOUNT_NEW_DEVICE);
ASSERT_EQ(7U, custom_mount_options.size());
EXPECT_EQ("foo", custom_mount_options[0]);
EXPECT_EQ("bar=baz", custom_mount_options[1]);
EXPECT_EQ("nodev", custom_mount_options[2]);
EXPECT_EQ("noexec", custom_mount_options[3]);
EXPECT_EQ("nosuid", custom_mount_options[4]);
EXPECT_EQ("rw", custom_mount_options[5]);
EXPECT_EQ(kExpectedMountLabelOption, custom_mount_options[6]);
}
} // namespace chromeos

@ -69,6 +69,7 @@ void FakeCrosDisksClient::RemoveObserver(Observer* observer) {
void FakeCrosDisksClient::Mount(const std::string& source_path,
const std::string& source_format,
const std::string& mount_label,
const std::vector<std::string>& mount_options,
MountAccessMode access_mode,
RemountOption remount,
VoidDBusMethodCallback callback) {

@ -35,6 +35,7 @@ class CHROMEOS_EXPORT FakeCrosDisksClient : public CrosDisksClient {
void Mount(const std::string& source_path,
const std::string& source_format,
const std::string& mount_label,
const std::vector<std::string>& mount_options,
MountAccessMode access_mode,
RemountOption remount,
VoidDBusMethodCallback callback) override;

@ -67,6 +67,7 @@ class DiskMountManagerImpl : public DiskMountManager,
void MountPath(const std::string& source_path,
const std::string& source_format,
const std::string& mount_label,
const std::vector<std::string>& mount_options,
MountType type,
MountAccessMode access_mode) override {
// Hidden and non-existent devices should not be mounted.
@ -79,7 +80,7 @@ class DiskMountManagerImpl : public DiskMountManager,
}
}
cros_disks_client_->Mount(
source_path, source_format, mount_label, access_mode,
source_path, source_format, mount_label, mount_options, access_mode,
REMOUNT_OPTION_MOUNT_NEW_DEVICE,
base::BindOnce(&DiskMountManagerImpl::OnMount,
weak_ptr_factory_.GetWeakPtr(), source_path, type));
@ -356,7 +357,7 @@ class DiskMountManagerImpl : public DiskMountManager,
access_modes_[source_path] = access_mode;
cros_disks_client_->Mount(
mount_point->second.source_path, std::string(), std::string(),
mount_point->second.source_path, std::string(), std::string(), {},
access_mode, REMOUNT_OPTION_REMOUNT_EXISTING_DEVICE,
base::BindOnce(&DiskMountManagerImpl::OnMount,
weak_ptr_factory_.GetWeakPtr(), source_path,

@ -311,6 +311,7 @@ class CHROMEOS_EXPORT DiskMountManager {
virtual void MountPath(const std::string& source_path,
const std::string& source_format,
const std::string& mount_label,
const std::vector<std::string>& mount_options,
MountType type,
MountAccessMode access_mode) = 0;

@ -953,10 +953,10 @@ TEST_F(DiskMountManagerTest, MountPath_RecordAccessMode) {
const std::string kMountPath1 = "/media/foo";
const std::string kMountPath2 = "/media/bar";
manager->MountPath(kSourcePath1, kSourceFormat, std::string(),
manager->MountPath(kSourcePath1, kSourceFormat, std::string(), {},
chromeos::MOUNT_TYPE_DEVICE,
chromeos::MOUNT_ACCESS_MODE_READ_WRITE);
manager->MountPath(kSourcePath2, kSourceFormat, std::string(),
manager->MountPath(kSourcePath2, kSourceFormat, std::string(), {},
chromeos::MOUNT_TYPE_DEVICE,
chromeos::MOUNT_ACCESS_MODE_READ_ONLY);
// Simulate cros_disks reporting mount completed.
@ -999,7 +999,7 @@ TEST_F(DiskMountManagerTest, MountPath_ReadOnlyDevice) {
// Attempt to mount a read-only device in read-write mode.
manager->MountPath(kReadOnlyDeviceSourcePath, kSourceFormat, std::string(),
chromeos::MOUNT_TYPE_DEVICE,
{}, chromeos::MOUNT_TYPE_DEVICE,
chromeos::MOUNT_ACCESS_MODE_READ_WRITE);
// Simulate cros_disks reporting mount completed.
fake_cros_disks_client_->NotifyMountCompleted(

@ -156,7 +156,7 @@ void MockDiskMountManager::SetupDefaultReplies() {
EXPECT_CALL(*this, FindDiskBySourcePath(_))
.Times(AnyNumber());
EXPECT_CALL(*this, EnsureMountInfoRefreshed(_, _)).Times(AnyNumber());
EXPECT_CALL(*this, MountPath(_, _, _, _, _)).Times(AnyNumber());
EXPECT_CALL(*this, MountPath(_, _, _, _, _, _)).Times(AnyNumber());
EXPECT_CALL(*this, UnmountPath(_, _, _))
.Times(AnyNumber());
EXPECT_CALL(*this, RemountAllRemovableDrives(_)).Times(AnyNumber());

@ -37,10 +37,11 @@ class MockDiskMountManager : public DiskMountManager {
const DiskMountManager::MountPointMap&(void));
MOCK_METHOD2(EnsureMountInfoRefreshed,
void(const EnsureMountInfoRefreshedCallback&, bool));
MOCK_METHOD5(MountPath,
MOCK_METHOD6(MountPath,
void(const std::string&,
const std::string&,
const std::string&,
const std::vector<std::string>&,
MountType,
MountAccessMode));
MOCK_METHOD3(UnmountPath, void(const std::string&,