Improve the logic for mapping target dynamic partitions.
While applying a retrofit update on a dynamic-partitions-enabled build,
it should always use static target partitions. Otherwise reading them
from the updated partition metadata would end up using mismatching info.
This CL improves the logic in detecting and handling such a case.
- It identifies a retrofit or regular update based on the absence of
DynamicPartitionMetadata field.
- Upon seeing that, it skips updating partition metadata for the target
slot, and looks up target partitions as static partitions.
- Source partitions will always be loaded according to the actual state.
This CL also removes the re-mapping of the target partitions from
InitPartitionMetadata(). It only needs to unmap those partitions, since
they may become inconsistent with the updated metadata. However, it's
unnecessary to re-map them, which will be done later as part of
GetDynamicPartitionDevice().
Also updated tests to reflect this.
Bug: 120775936
Test: update_engine_unittests
Test: Apply an update with dynamic partitions; abort and resume.
Test: Apply a retrofit update; abort and resume the update.
Change-Id: Ic07bd98847e91a003101266e426c4d23666810f2
diff --git a/boot_control_android.cc b/boot_control_android.cc
index 7d02ee2..c42c8d7 100644
--- a/boot_control_android.cc
+++ b/boot_control_android.cc
@@ -122,10 +122,6 @@
const string& partition_name_suffix,
Slot slot,
string* device) const {
- if (!dynamic_control_->IsDynamicPartitionsEnabled()) {
- return DynamicPartitionDeviceStatus::TRY_STATIC;
- }
-
string super_device =
device_dir.Append(fs_mgr_get_super_partition_name(slot)).value();
@@ -203,15 +199,22 @@
}
base::FilePath device_dir(device_dir_str);
- switch (GetDynamicPartitionDevice(
- device_dir, partition_name_suffix, slot, device)) {
- case DynamicPartitionDeviceStatus::SUCCESS:
- return true;
- case DynamicPartitionDeviceStatus::TRY_STATIC:
- break;
- case DynamicPartitionDeviceStatus::ERROR: // fallthrough
- default:
- return false;
+ // When looking up target partition devices, treat them as static if the
+ // current payload doesn't encode them as dynamic partitions. This may happen
+ // when applying a retrofit update on top of a dynamic-partitions-enabled
+ // build.
+ if (dynamic_control_->IsDynamicPartitionsEnabled() &&
+ (slot == GetCurrentSlot() || is_target_dynamic_)) {
+ switch (GetDynamicPartitionDevice(
+ device_dir, partition_name_suffix, slot, device)) {
+ case DynamicPartitionDeviceStatus::SUCCESS:
+ return true;
+ case DynamicPartitionDeviceStatus::TRY_STATIC:
+ break;
+ case DynamicPartitionDeviceStatus::ERROR: // fallthrough
+ default:
+ return false;
+ }
}
base::FilePath path = device_dir.Append(partition_name_suffix);
@@ -289,14 +292,19 @@
namespace {
-bool InitPartitionMetadataInternal(
- DynamicPartitionControlInterface* dynamic_control,
- const string& source_device,
- const string& target_device,
- Slot source_slot,
- Slot target_slot,
- const string& target_suffix,
- const PartitionMetadata& partition_metadata) {
+bool UpdatePartitionMetadata(DynamicPartitionControlInterface* dynamic_control,
+ Slot source_slot,
+ Slot target_slot,
+ const string& target_suffix,
+ const PartitionMetadata& partition_metadata) {
+ string device_dir_str;
+ if (!dynamic_control->GetDeviceDir(&device_dir_str)) {
+ return false;
+ }
+ base::FilePath device_dir(device_dir_str);
+ auto source_device =
+ device_dir.Append(fs_mgr_get_super_partition_name(source_slot)).value();
+
auto builder = dynamic_control->LoadMetadataBuilder(
source_device, source_slot, target_slot);
if (builder == nullptr) {
@@ -330,7 +338,7 @@
if (total_size > allocatable_space) {
LOG(ERROR) << "The maximum size of all groups with suffix " << target_suffix
<< " (" << total_size << ") has exceeded " << expr
- << "allocatable space for dynamic partitions "
+ << " allocatable space for dynamic partitions "
<< allocatable_space << ".";
return false;
}
@@ -364,34 +372,21 @@
}
}
+ auto target_device =
+ device_dir.Append(fs_mgr_get_super_partition_name(target_slot)).value();
return dynamic_control->StoreMetadata(
target_device, builder.get(), target_slot);
}
-// Unmap all partitions, and remap partitions as writable.
-bool Remap(DynamicPartitionControlInterface* dynamic_control,
- const string& target_device,
- Slot target_slot,
- const string& target_suffix,
- const PartitionMetadata& partition_metadata) {
+bool UnmapTargetPartitions(DynamicPartitionControlInterface* dynamic_control,
+ const string& target_suffix,
+ const PartitionMetadata& partition_metadata) {
for (const auto& group : partition_metadata.groups) {
for (const auto& partition : group.partitions) {
if (!dynamic_control->UnmapPartitionOnDeviceMapper(
partition.name + target_suffix, true /* wait */)) {
return false;
}
- if (partition.size == 0) {
- continue;
- }
- string map_path;
- if (!dynamic_control->MapPartitionOnDeviceMapper(
- target_device,
- partition.name + target_suffix,
- target_slot,
- true /* force writable */,
- &map_path)) {
- return false;
- }
}
}
return true;
@@ -405,42 +400,37 @@
return true;
}
- string device_dir_str;
- if (!dynamic_control_->GetDeviceDir(&device_dir_str)) {
- return false;
- }
- base::FilePath device_dir(device_dir_str);
- string target_device =
- device_dir.Append(fs_mgr_get_super_partition_name(target_slot)).value();
-
- Slot current_slot = GetCurrentSlot();
- if (target_slot == current_slot) {
+ auto source_slot = GetCurrentSlot();
+ if (target_slot == source_slot) {
LOG(ERROR) << "Cannot call InitPartitionMetadata on current slot.";
return false;
}
- string source_device =
- device_dir.Append(fs_mgr_get_super_partition_name(current_slot)).value();
string target_suffix;
if (!GetSuffix(target_slot, &target_suffix)) {
return false;
}
- if (!InitPartitionMetadataInternal(dynamic_control_.get(),
- source_device,
- target_device,
- current_slot,
- target_slot,
- target_suffix,
- partition_metadata)) {
+ // Although the current build supports dynamic partitions, the given payload
+ // doesn't use it for target partitions. This could happen when applying a
+ // retrofit update. Skip updating the partition metadata for the target slot.
+ is_target_dynamic_ = !partition_metadata.groups.empty();
+ if (!is_target_dynamic_) {
+ return true;
+ }
+
+ // Unmap all the target dynamic partitions because they would become
+ // inconsistent with the new metadata.
+ if (!UnmapTargetPartitions(
+ dynamic_control_.get(), target_suffix, partition_metadata)) {
return false;
}
- if (!Remap(dynamic_control_.get(),
- target_device,
- target_slot,
- target_suffix,
- partition_metadata)) {
+ if (!UpdatePartitionMetadata(dynamic_control_.get(),
+ source_slot,
+ target_slot,
+ target_suffix,
+ partition_metadata)) {
return false;
}
diff --git a/boot_control_android.h b/boot_control_android.h
index d6590fb..aa62e42 100644
--- a/boot_control_android.h
+++ b/boot_control_android.h
@@ -82,6 +82,10 @@
Slot slot,
const std::string& partition_name_suffix) const;
+ // Whether the target partitions should be loaded as dynamic partitions. Set
+ // by InitPartitionMetadata() per each update.
+ bool is_target_dynamic_{false};
+
DISALLOW_COPY_AND_ASSIGN(BootControlAndroid);
};
diff --git a/boot_control_android_unittest.cc b/boot_control_android_unittest.cc
index d706810..79f8662 100644
--- a/boot_control_android_unittest.cc
+++ b/boot_control_android_unittest.cc
@@ -24,10 +24,12 @@
#include <fs_mgr.h>
#include <gmock/gmock.h>
#include <gtest/gtest.h>
+#include <libdm/dm.h>
#include "update_engine/mock_boot_control_hal.h"
#include "update_engine/mock_dynamic_partition_control.h"
+using android::dm::DmDeviceState;
using android::fs_mgr::MetadataBuilder;
using android::hardware::Void;
using testing::_;
@@ -49,7 +51,7 @@
constexpr const uint32_t kMaxNumSlots = 2;
constexpr const char* kSlotSuffixes[kMaxNumSlots] = {"_a", "_b"};
constexpr const char* kFakeDevicePath = "/fake/dev/path/";
-constexpr const char* kFakeMappedPath = "/fake/mapped/path/";
+constexpr const char* kFakeDmDevicePath = "/fake/dm/dev/path/";
constexpr const uint32_t kFakeMetadataSize = 65536;
constexpr const char* kDefaultGroup = "foo";
@@ -125,6 +127,10 @@
return kFakeDevicePath + name;
}
+inline std::string GetDmDevice(const std::string& name) {
+ return kFakeDmDevicePath + name;
+}
+
// TODO(elsk): fs_mgr_get_super_partition_name should be mocked.
inline std::string GetSuperDevice(uint32_t slot) {
return GetDevice(fs_mgr_get_super_partition_name(slot));
@@ -277,11 +283,17 @@
.WillByDefault(Return(true));
ON_CALL(dynamicControl(), IsDynamicPartitionsRetrofit())
.WillByDefault(Return(false));
+ ON_CALL(dynamicControl(), DeviceExists(_)).WillByDefault(Return(true));
ON_CALL(dynamicControl(), GetDeviceDir(_))
.WillByDefault(Invoke([](auto path) {
*path = kFakeDevicePath;
return true;
}));
+ ON_CALL(dynamicControl(), GetDmDevicePathByName(_, _))
+ .WillByDefault(Invoke([](auto partition_name_suffix, auto device) {
+ *device = GetDmDevice(partition_name_suffix);
+ return true;
+ }));
}
// Return the mocked HAL module.
@@ -310,31 +322,6 @@
}));
}
- // Expect that MapPartitionOnDeviceMapper is called on target() metadata slot
- // with each partition in |partitions|.
- void ExpectMap(const std::set<std::string>& partitions,
- bool force_writable = true) {
- // Error when MapPartitionOnDeviceMapper is called on unknown arguments.
- ON_CALL(dynamicControl(), MapPartitionOnDeviceMapper(_, _, _, _, _))
- .WillByDefault(Return(false));
-
- for (const auto& partition : partitions) {
- EXPECT_CALL(
- dynamicControl(),
- MapPartitionOnDeviceMapper(
- GetSuperDevice(target()), partition, target(), force_writable, _))
- .WillOnce(Invoke([this](auto, auto partition, auto, auto, auto path) {
- auto it = mapped_devices_.find(partition);
- if (it != mapped_devices_.end()) {
- *path = it->second;
- return true;
- }
- mapped_devices_[partition] = *path = kFakeMappedPath + partition;
- return true;
- }));
- }
- }
-
// Expect that UnmapPartitionOnDeviceMapper is called on target() metadata
// slot with each partition in |partitions|.
void ExpectUnmap(const std::set<std::string>& partitions) {
@@ -351,11 +338,6 @@
}
}
- void ExpectRemap(const std::set<std::string>& partitions) {
- ExpectUnmap(partitions);
- ExpectMap(partitions);
- }
-
void ExpectDevicesAreMapped(const std::set<std::string>& partitions) {
ASSERT_EQ(partitions.size(), mapped_devices_.size());
for (const auto& partition : partitions) {
@@ -440,11 +422,10 @@
{S("vendor"), 1_GiB},
{T("system"), 3_GiB},
{T("vendor"), 1_GiB}});
- ExpectRemap({T("system"), T("vendor")});
+ ExpectUnmap({T("system"), T("vendor")});
EXPECT_TRUE(
InitPartitionMetadata(target(), {{"system", 3_GiB}, {"vendor", 1_GiB}}));
- ExpectDevicesAreMapped({T("system"), T("vendor")});
}
// Test resize case. Shrink if target metadata contains a partition with a size
@@ -459,22 +440,20 @@
{S("vendor"), 1_GiB},
{T("system"), 2_GiB},
{T("vendor"), 150_MiB}});
- ExpectRemap({T("system"), T("vendor")});
+ ExpectUnmap({T("system"), T("vendor")});
EXPECT_TRUE(InitPartitionMetadata(target(),
{{"system", 2_GiB}, {"vendor", 150_MiB}}));
- ExpectDevicesAreMapped({T("system"), T("vendor")});
}
// Test adding partitions on the first run.
TEST_P(BootControlAndroidTestP, AddPartitionToEmptyMetadata) {
SetMetadata(source(), PartitionSuffixSizes{});
ExpectStoreMetadata({{T("system"), 2_GiB}, {T("vendor"), 1_GiB}});
- ExpectRemap({T("system"), T("vendor")});
+ ExpectUnmap({T("system"), T("vendor")});
EXPECT_TRUE(
InitPartitionMetadata(target(), {{"system", 2_GiB}, {"vendor", 1_GiB}}));
- ExpectDevicesAreMapped({T("system"), T("vendor")});
}
// Test subsequent add case.
@@ -482,11 +461,10 @@
SetMetadata(source(), {{S("system"), 2_GiB}, {T("system"), 2_GiB}});
ExpectStoreMetadata(
{{S("system"), 2_GiB}, {T("system"), 2_GiB}, {T("vendor"), 1_GiB}});
- ExpectRemap({T("system"), T("vendor")});
+ ExpectUnmap({T("system"), T("vendor")});
EXPECT_TRUE(
InitPartitionMetadata(target(), {{"system", 2_GiB}, {"vendor", 1_GiB}}));
- ExpectDevicesAreMapped({T("system"), T("vendor")});
}
// Test delete one partition.
@@ -499,10 +477,9 @@
// No T("vendor")
ExpectStoreMetadata(
{{S("system"), 2_GiB}, {S("vendor"), 1_GiB}, {T("system"), 2_GiB}});
- ExpectRemap({T("system")});
+ ExpectUnmap({T("system")});
EXPECT_TRUE(InitPartitionMetadata(target(), {{"system", 2_GiB}}));
- ExpectDevicesAreMapped({T("system")});
}
// Test delete all partitions.
@@ -515,7 +492,6 @@
ExpectStoreMetadata({{S("system"), 2_GiB}, {S("vendor"), 1_GiB}});
EXPECT_TRUE(InitPartitionMetadata(target(), {}));
- ExpectDevicesAreMapped({});
}
// Test corrupt source metadata case.
@@ -523,6 +499,8 @@
EXPECT_CALL(dynamicControl(),
LoadMetadataBuilder(GetSuperDevice(source()), source(), _))
.WillOnce(Invoke([](auto, auto, auto) { return nullptr; }));
+ ExpectUnmap({T("system")});
+
EXPECT_FALSE(InitPartitionMetadata(target(), {{"system", 1_GiB}}))
<< "Should not be able to continue with corrupt source metadata";
}
@@ -551,6 +529,40 @@
<< "Should not be able to grow over size of super / 2";
}
+// Test applying retrofit update on a build with dynamic partitions enabled.
+TEST_P(BootControlAndroidTestP,
+ ApplyRetrofitUpdateOnDynamicPartitionsEnabledBuild) {
+ SetMetadata(source(),
+ {{S("system"), 2_GiB},
+ {S("vendor"), 1_GiB},
+ {T("system"), 2_GiB},
+ {T("vendor"), 1_GiB}});
+ // Should not try to unmap any target partition.
+ EXPECT_CALL(dynamicControl(), UnmapPartitionOnDeviceMapper(_, _)).Times(0);
+ // Should not store metadata to target slot.
+ EXPECT_CALL(dynamicControl(),
+ StoreMetadata(GetSuperDevice(target()), _, target()))
+ .Times(0);
+
+ // Not calling through BootControlAndroidTest::InitPartitionMetadata(), since
+ // we don't want any default group in the PartitionMetadata.
+ EXPECT_TRUE(bootctl_.InitPartitionMetadata(target(), {}));
+
+ // Should use dynamic source partitions.
+ EXPECT_CALL(dynamicControl(), GetState(S("system")))
+ .Times(1)
+ .WillOnce(Return(DmDeviceState::ACTIVE));
+ std::string source_device;
+ EXPECT_TRUE(bootctl_.GetPartitionDevice("system", source(), &source_device));
+ EXPECT_EQ(GetDmDevice(S("system")), source_device);
+
+ // Should use static target partitions without querying dynamic control.
+ EXPECT_CALL(dynamicControl(), GetState(T("system"))).Times(0);
+ std::string target_device;
+ EXPECT_TRUE(bootctl_.GetPartitionDevice("system", target(), &target_device));
+ EXPECT_EQ(GetDevice(T("system")), target_device);
+}
+
INSTANTIATE_TEST_CASE_P(BootControlAndroidTest,
BootControlAndroidTestP,
testing::Values(TestParam{0, 1}, TestParam{1, 0}));
@@ -611,14 +623,13 @@
SetMetadata(source(), update_sizes_0());
SetMetadata(target(), update_sizes_0());
ExpectStoreMetadata(update_sizes_1());
- ExpectRemap({"grown_b", "shrunk_b", "same_b", "added_b"});
+ ExpectUnmap({"grown_b", "shrunk_b", "same_b", "added_b"});
EXPECT_TRUE(InitPartitionMetadata(target(),
{{"grown", 3_GiB},
{"shrunk", 150_MiB},
{"same", 100_MiB},
{"added", 150_MiB}}));
- ExpectDevicesAreMapped({"grown_b", "shrunk_b", "same_b", "added_b"});
}
// After first update, test for the second update. In the second update, the
@@ -630,14 +641,13 @@
SetMetadata(target(), update_sizes_0());
ExpectStoreMetadata(update_sizes_2());
- ExpectRemap({"grown_a", "shrunk_a", "same_a", "deleted_a"});
+ ExpectUnmap({"grown_a", "shrunk_a", "same_a", "deleted_a"});
EXPECT_TRUE(InitPartitionMetadata(target(),
{{"grown", 4_GiB},
{"shrunk", 100_MiB},
{"same", 100_MiB},
{"deleted", 64_MiB}}));
- ExpectDevicesAreMapped({"grown_a", "shrunk_a", "same_a", "deleted_a"});
}
TEST_F(BootControlAndroidTest, ApplyingToCurrentSlot) {
@@ -688,14 +698,13 @@
ExpectStoreMetadata(PartitionMetadata{
.groups = {SimpleGroup(T("android"), 3_GiB, T("system"), 3_GiB),
SimpleGroup(T("oem"), 2_GiB, T("vendor"), 2_GiB)}});
- ExpectRemap({T("system"), T("vendor")});
+ ExpectUnmap({T("system"), T("vendor")});
EXPECT_TRUE(bootctl_.InitPartitionMetadata(
target(),
PartitionMetadata{
.groups = {SimpleGroup("android", 3_GiB, "system", 3_GiB),
SimpleGroup("oem", 2_GiB, "vendor", 2_GiB)}}));
- ExpectDevicesAreMapped({T("system"), T("vendor")});
}
TEST_P(BootControlAndroidGroupTestP, NotEnoughSpaceForGroup) {
@@ -722,7 +731,7 @@
.size = 3_GiB,
.partitions = {{.name = T("system"), .size = 2_GiB},
{.name = T("product_services"), .size = 1_GiB}}}}});
- ExpectRemap({T("system"), T("vendor"), T("product_services")});
+ ExpectUnmap({T("system"), T("vendor"), T("product_services")});
EXPECT_TRUE(bootctl_.InitPartitionMetadata(
target(),
@@ -733,27 +742,25 @@
.partitions = {{.name = "system", .size = 2_GiB},
{.name = "product_services", .size = 1_GiB}}},
SimpleGroup("oem", 2_GiB, "vendor", 2_GiB)}}));
- ExpectDevicesAreMapped({T("system"), T("vendor"), T("product_services")});
}
TEST_P(BootControlAndroidGroupTestP, RemovePartitionFromGroup) {
ExpectStoreMetadata(PartitionMetadata{
.groups = {{.name = T("android"), .size = 3_GiB, .partitions = {}}}});
- ExpectRemap({T("vendor")});
+ ExpectUnmap({T("vendor")});
EXPECT_TRUE(bootctl_.InitPartitionMetadata(
target(),
PartitionMetadata{
.groups = {{.name = "android", .size = 3_GiB, .partitions = {}},
SimpleGroup("oem", 2_GiB, "vendor", 2_GiB)}}));
- ExpectDevicesAreMapped({T("vendor")});
}
TEST_P(BootControlAndroidGroupTestP, AddGroup) {
ExpectStoreMetadata(PartitionMetadata{
.groups = {
SimpleGroup(T("new_group"), 2_GiB, T("new_partition"), 2_GiB)}});
- ExpectRemap({T("system"), T("vendor"), T("new_partition")});
+ ExpectUnmap({T("system"), T("vendor"), T("new_partition")});
EXPECT_TRUE(bootctl_.InitPartitionMetadata(
target(),
@@ -762,31 +769,28 @@
SimpleGroup("android", 2_GiB, "system", 2_GiB),
SimpleGroup("oem", 1_GiB, "vendor", 1_GiB),
SimpleGroup("new_group", 2_GiB, "new_partition", 2_GiB)}}));
- ExpectDevicesAreMapped({T("system"), T("vendor"), T("new_partition")});
}
TEST_P(BootControlAndroidGroupTestP, RemoveGroup) {
ExpectStoreMetadataMatch(Not(HasGroup(T("oem"))));
- ExpectRemap({T("system")});
+ ExpectUnmap({T("system")});
EXPECT_TRUE(bootctl_.InitPartitionMetadata(
target(),
PartitionMetadata{
.groups = {SimpleGroup("android", 2_GiB, "system", 2_GiB)}}));
- ExpectDevicesAreMapped({T("system")});
}
TEST_P(BootControlAndroidGroupTestP, ResizeGroup) {
ExpectStoreMetadata(PartitionMetadata{
.groups = {SimpleGroup(T("android"), 2_GiB, T("system"), 2_GiB),
SimpleGroup(T("oem"), 3_GiB, T("vendor"), 3_GiB)}});
- ExpectRemap({T("system"), T("vendor")});
+ ExpectUnmap({T("system"), T("vendor")});
EXPECT_TRUE(bootctl_.InitPartitionMetadata(
target(),
PartitionMetadata{
.groups = {SimpleGroup("android", 2_GiB, "system", 2_GiB),
SimpleGroup("oem", 3_GiB, "vendor", 3_GiB)}}));
- ExpectDevicesAreMapped({T("system"), T("vendor")});
}
INSTANTIATE_TEST_CASE_P(BootControlAndroidTest,
diff --git a/payload_consumer/delta_performer.cc b/payload_consumer/delta_performer.cc
index 3cce4d2..0711ac6 100644
--- a/payload_consumer/delta_performer.cc
+++ b/payload_consumer/delta_performer.cc
@@ -946,7 +946,7 @@
BootControlInterface::PartitionMetadata partition_metadata;
if (manifest_.has_dynamic_partition_metadata()) {
std::map<string, uint64_t> partition_sizes;
- for (const InstallPlan::Partition& partition : install_plan_->partitions) {
+ for (const auto& partition : install_plan_->partitions) {
partition_sizes.emplace(partition.name, partition.target_size);
}
for (const auto& group : manifest_.dynamic_partition_metadata().groups()) {
diff --git a/payload_consumer/install_plan.cc b/payload_consumer/install_plan.cc
index 5f2697b..1fa27ab 100644
--- a/payload_consumer/install_plan.cc
+++ b/payload_consumer/install_plan.cc
@@ -100,7 +100,8 @@
for (Partition& partition : partitions) {
if (source_slot != BootControlInterface::kInvalidSlot) {
result = boot_control->GetPartitionDevice(
- partition.name, source_slot, &partition.source_path) && result;
+ partition.name, source_slot, &partition.source_path) &&
+ result;
} else {
partition.source_path.clear();
}
@@ -108,7 +109,8 @@
if (target_slot != BootControlInterface::kInvalidSlot &&
partition.target_size > 0) {
result = boot_control->GetPartitionDevice(
- partition.name, target_slot, &partition.target_path) && result;
+ partition.name, target_slot, &partition.target_path) &&
+ result;
} else {
partition.target_path.clear();
}
diff --git a/payload_consumer/install_plan.h b/payload_consumer/install_plan.h
index f56f63c..755d913 100644
--- a/payload_consumer/install_plan.h
+++ b/payload_consumer/install_plan.h
@@ -46,7 +46,7 @@
void Dump() const;
- // Load the |source_path| and |target_path| of all |partitions| based on the
+ // Loads the |source_path| and |target_path| of all |partitions| based on the
// |source_slot| and |target_slot| if available. Returns whether it succeeded
// to load all the partitions for the valid slots.
bool LoadPartitionsFromSlots(BootControlInterface* boot_control);
