From c02509b500a64aad0673b21212ad6c695bd82b01 Mon Sep 17 00:00:00 2001 From: Yifan Hong Date: Wed, 11 Sep 2019 18:15:22 -0700 Subject: [PATCH] fs_mgr: CreateDmTable takes CreateLogicalPartitionParams It has too many arguments. Also fixed CreateLogicalPartitionParams::InitDefaults because it doesn't use the provided partition opener to read metadata (which breaks tests). Test: libsnapshot_test Test: liblp_test Change-Id: I74cf8d468274f741c6f3743438fe8855b2aef15b --- fs_mgr/fs_mgr_dm_linear.cpp | 46 +++++++++++++++++-------------- fs_mgr/include/fs_mgr_dm_linear.h | 4 +-- fs_mgr/libsnapshot/snapshot.cpp | 22 +++++---------- 3 files changed, 34 insertions(+), 38 deletions(-) diff --git a/fs_mgr/fs_mgr_dm_linear.cpp b/fs_mgr/fs_mgr_dm_linear.cpp index ea799cef1..0dcb9fe8a 100644 --- a/fs_mgr/fs_mgr_dm_linear.cpp +++ b/fs_mgr/fs_mgr_dm_linear.cpp @@ -79,22 +79,22 @@ static bool GetPhysicalPartitionDevicePath(const IPartitionOpener& opener, return true; } -bool CreateDmTable(const IPartitionOpener& opener, const LpMetadata& metadata, - const LpMetadataPartition& partition, const std::string& super_device, - DmTable* table) { +bool CreateDmTableInternal(const CreateLogicalPartitionParams& params, DmTable* table) { + const auto& super_device = params.block_device; + uint64_t sector = 0; - for (size_t i = 0; i < partition.num_extents; i++) { - const auto& extent = metadata.extents[partition.first_extent_index + i]; + for (size_t i = 0; i < params.partition->num_extents; i++) { + const auto& extent = params.metadata->extents[params.partition->first_extent_index + i]; std::unique_ptr target; switch (extent.target_type) { case LP_TARGET_TYPE_ZERO: target = std::make_unique(sector, extent.num_sectors); break; case LP_TARGET_TYPE_LINEAR: { - const auto& block_device = metadata.block_devices[extent.target_source]; + const auto& block_device = params.metadata->block_devices[extent.target_source]; std::string dev_string; - if (!GetPhysicalPartitionDevicePath(opener, metadata, block_device, super_device, - &dev_string)) { + if (!GetPhysicalPartitionDevicePath(*params.partition_opener, *params.metadata, + block_device, super_device, &dev_string)) { LOG(ERROR) << "Unable to complete device-mapper table, unknown block device"; return false; } @@ -111,12 +111,21 @@ bool CreateDmTable(const IPartitionOpener& opener, const LpMetadata& metadata, } sector += extent.num_sectors; } - if (partition.attributes & LP_PARTITION_ATTR_READONLY) { + if (params.partition->attributes & LP_PARTITION_ATTR_READONLY) { table->set_readonly(true); } + if (params.force_writable) { + table->set_readonly(false); + } return true; } +bool CreateDmTable(CreateLogicalPartitionParams params, DmTable* table) { + CreateLogicalPartitionParams::OwnedData owned_data; + if (!params.InitDefaults(&owned_data)) return false; + return CreateDmTableInternal(params, table); +} + bool CreateLogicalPartitions(const std::string& block_device) { uint32_t slot = SlotNumberForSlotSuffix(fs_mgr_get_slot_suffix()); auto metadata = ReadMetadata(block_device.c_str(), slot); @@ -160,6 +169,11 @@ bool CreateLogicalPartitionParams::InitDefaults(CreateLogicalPartitionParams::Ow return false; } + if (!partition_opener) { + owned->partition_opener = std::make_unique(); + partition_opener = owned->partition_opener.get(); + } + // Read metadata if needed. if (!metadata) { if (!metadata_slot) { @@ -167,7 +181,8 @@ bool CreateLogicalPartitionParams::InitDefaults(CreateLogicalPartitionParams::Ow return false; } auto slot = *metadata_slot; - if (owned->metadata = ReadMetadata(block_device, slot); !owned->metadata) { + if (owned->metadata = ReadMetadata(*partition_opener, block_device, slot); + !owned->metadata) { LOG(ERROR) << "Could not read partition table for: " << block_device; return false; } @@ -195,11 +210,6 @@ bool CreateLogicalPartitionParams::InitDefaults(CreateLogicalPartitionParams::Ow return false; } - if (!partition_opener) { - owned->partition_opener = std::make_unique(); - partition_opener = owned->partition_opener.get(); - } - if (device_name.empty()) { device_name = partition_name; } @@ -212,13 +222,9 @@ bool CreateLogicalPartition(CreateLogicalPartitionParams params, std::string* pa if (!params.InitDefaults(&owned_data)) return false; DmTable table; - if (!CreateDmTable(*params.partition_opener, *params.metadata, *params.partition, - params.block_device, &table)) { + if (!CreateDmTableInternal(params, &table)) { return false; } - if (params.force_writable) { - table.set_readonly(false); - } DeviceMapper& dm = DeviceMapper::Instance(); if (!dm.CreateDevice(params.device_name, table, path, params.timeout_ms)) { diff --git a/fs_mgr/include/fs_mgr_dm_linear.h b/fs_mgr/include/fs_mgr_dm_linear.h index a9122087a..f3d09fb83 100644 --- a/fs_mgr/include/fs_mgr_dm_linear.h +++ b/fs_mgr/include/fs_mgr_dm_linear.h @@ -105,9 +105,7 @@ bool CreateLogicalPartition(CreateLogicalPartitionParams params, std::string* pa bool DestroyLogicalPartition(const std::string& name); // Helper for populating a DmTable for a logical partition. -bool CreateDmTable(const IPartitionOpener& opener, const LpMetadata& metadata, - const LpMetadataPartition& partition, const std::string& super_device, - android::dm::DmTable* table); +bool CreateDmTable(CreateLogicalPartitionParams params, android::dm::DmTable* table); } // namespace fs_mgr } // namespace android diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp index 5813a2df4..7510e3af0 100644 --- a/fs_mgr/libsnapshot/snapshot.cpp +++ b/fs_mgr/libsnapshot/snapshot.cpp @@ -992,24 +992,16 @@ bool SnapshotManager::CollapseSnapshotDevice(const std::string& name, } } - // Grab the partition metadata for the snapshot. uint32_t slot = SlotNumberForSlotSuffix(device_->GetSlotSuffix()); - auto super_device = device_->GetSuperDevice(slot); - const auto& opener = device_->GetPartitionOpener(); - auto metadata = android::fs_mgr::ReadMetadata(opener, super_device, slot); - if (!metadata) { - LOG(ERROR) << "Could not read super partition metadata."; - return false; - } - auto partition = android::fs_mgr::FindPartition(*metadata.get(), name); - if (!partition) { - LOG(ERROR) << "Snapshot does not have a partition in super: " << name; - return false; - } - // Create a DmTable that is identical to the base device. + CreateLogicalPartitionParams base_device_params{ + .block_device = device_->GetSuperDevice(slot), + .metadata_slot = slot, + .partition_name = name, + .partition_opener = &device_->GetPartitionOpener(), + }; DmTable table; - if (!CreateDmTable(opener, *metadata.get(), *partition, super_device, &table)) { + if (!CreateDmTable(base_device_params, &table)) { LOG(ERROR) << "Could not create a DmTable for partition: " << name; return false; }