Merge "Explicitly cast to avoid ambiguous method call."
diff --git a/common/dynamic_partition_control_interface.h b/common/dynamic_partition_control_interface.h
index 39daf75..c17bafb 100644
--- a/common/dynamic_partition_control_interface.h
+++ b/common/dynamic_partition_control_interface.h
@@ -50,8 +50,12 @@
// Return the feature flags of Virtual A/B on this device.
virtual FeatureFlag GetVirtualAbFeatureFlag() = 0;
- // Checks if the provided InstallOperation can be skipped on this device.
- virtual bool ShouldSkipOperation(const InstallOperation& operation) = 0;
+ // Checks if |operation| can be skipped on the given partition.
+ // |partition_name| should not have the slot suffix; implementation of
+ // DynamicPartitionControlInterface checks partition at the target slot
+ // previously set with PreparePartitionsForUpdate().
+ virtual bool ShouldSkipOperation(const std::string& partition_name,
+ const InstallOperation& operation) = 0;
// Do necessary cleanups before destroying the object.
virtual void Cleanup() = 0;
diff --git a/common/dynamic_partition_control_stub.cc b/common/dynamic_partition_control_stub.cc
index 06f6b3c..bc792c8 100644
--- a/common/dynamic_partition_control_stub.cc
+++ b/common/dynamic_partition_control_stub.cc
@@ -33,7 +33,7 @@
}
bool DynamicPartitionControlStub::ShouldSkipOperation(
- const InstallOperation& operation) {
+ const std::string& partition_name, const InstallOperation& operation) {
return false;
}
diff --git a/common/dynamic_partition_control_stub.h b/common/dynamic_partition_control_stub.h
index c62758b..1704f05 100644
--- a/common/dynamic_partition_control_stub.h
+++ b/common/dynamic_partition_control_stub.h
@@ -29,7 +29,8 @@
public:
FeatureFlag GetDynamicPartitionsFeatureFlag() override;
FeatureFlag GetVirtualAbFeatureFlag() override;
- bool ShouldSkipOperation(const InstallOperation& operation) override;
+ bool ShouldSkipOperation(const std::string& partition_name,
+ const InstallOperation& operation) override;
void Cleanup() override;
bool PreparePartitionsForUpdate(uint32_t source_slot,
uint32_t target_slot,
diff --git a/dynamic_partition_control_android.cc b/dynamic_partition_control_android.cc
index c24aee9..4ad02c7 100644
--- a/dynamic_partition_control_android.cc
+++ b/dynamic_partition_control_android.cc
@@ -50,6 +50,7 @@
using android::fs_mgr::Partition;
using android::fs_mgr::PartitionOpener;
using android::fs_mgr::SlotSuffixForSlotNumber;
+using android::snapshot::SourceCopyOperationIsClone;
namespace chromeos_update_engine {
@@ -105,7 +106,18 @@
}
bool DynamicPartitionControlAndroid::ShouldSkipOperation(
- const InstallOperation& operation) {
+ const std::string& partition_name, const InstallOperation& operation) {
+ switch (operation.type()) {
+ case InstallOperation::SOURCE_COPY:
+ return target_supports_snapshot_ &&
+ GetVirtualAbFeatureFlag().IsEnabled() &&
+ mapped_devices_.count(partition_name +
+ SlotSuffixForSlotNumber(target_slot_)) > 0 &&
+ SourceCopyOperationIsClone(operation);
+ break;
+ default:
+ break;
+ }
return false;
}
@@ -361,6 +373,9 @@
uint32_t target_slot,
const DeltaArchiveManifest& manifest,
bool update) {
+ source_slot_ = source_slot;
+ target_slot_ = target_slot;
+
if (fs_mgr_overlayfs_is_setup()) {
// Non DAP devices can use overlayfs as well.
LOG(WARNING)
@@ -665,4 +680,9 @@
return DynamicPartitionDeviceStatus::ERROR;
}
+void DynamicPartitionControlAndroid::set_fake_mapped_devices(
+ const std::set<std::string>& fake) {
+ mapped_devices_ = fake;
+}
+
} // namespace chromeos_update_engine
diff --git a/dynamic_partition_control_android.h b/dynamic_partition_control_android.h
index 2bfbcb1..13fbb1a 100644
--- a/dynamic_partition_control_android.h
+++ b/dynamic_partition_control_android.h
@@ -35,7 +35,8 @@
~DynamicPartitionControlAndroid();
FeatureFlag GetDynamicPartitionsFeatureFlag() override;
FeatureFlag GetVirtualAbFeatureFlag() override;
- bool ShouldSkipOperation(const InstallOperation& operation) override;
+ bool ShouldSkipOperation(const std::string& partition_name,
+ const InstallOperation& operation) override;
void Cleanup() override;
bool PreparePartitionsForUpdate(uint32_t source_slot,
@@ -122,6 +123,8 @@
// metadata) for a given slot.
virtual std::string GetSuperPartitionName(uint32_t slot);
+ virtual void set_fake_mapped_devices(const std::set<std::string>& fake);
+
private:
friend class DynamicPartitionControlAndroidTest;
@@ -182,6 +185,8 @@
// Whether the target partitions should be loaded as dynamic partitions. Set
// by PreparePartitionsForUpdate() per each update.
bool is_target_dynamic_ = false;
+ uint32_t source_slot_ = UINT32_MAX;
+ uint32_t target_slot_ = UINT32_MAX;
DISALLOW_COPY_AND_ASSIGN(DynamicPartitionControlAndroid);
};
diff --git a/dynamic_partition_control_android_unittest.cc b/dynamic_partition_control_android_unittest.cc
index 10075ed..207a97e 100644
--- a/dynamic_partition_control_android_unittest.cc
+++ b/dynamic_partition_control_android_unittest.cc
@@ -124,6 +124,10 @@
}
void SetSlots(const TestParam& slots) { slots_ = slots; }
+ void SetSnapshotEnabled(bool enabled) {
+ dynamicControl().target_supports_snapshot_ = enabled;
+ }
+
struct Listener : public ::testing::MatchResultListener {
explicit Listener(std::ostream* os) : MatchResultListener(os) {}
};
@@ -616,4 +620,89 @@
<< "Should not be able to apply to current slot.";
}
+TEST_P(DynamicPartitionControlAndroidTestP, ShouldSkipOperationTest) {
+ ASSERT_TRUE(dynamicControl().PreparePartitionsForUpdate(
+ source(), target(), PartitionSizesToManifest({{"foo", 4_MiB}}), false));
+ dynamicControl().set_fake_mapped_devices({T("foo")});
+
+ InstallOperation iop;
+ Extent *se, *de;
+
+ // Not a SOURCE_COPY operation, cannot skip.
+ iop.set_type(InstallOperation::REPLACE);
+ EXPECT_FALSE(dynamicControl().ShouldSkipOperation("foo", iop));
+
+ iop.set_type(InstallOperation::SOURCE_COPY);
+
+ // By default GetVirtualAbFeatureFlag is disabled. Cannot skip operation.
+ EXPECT_FALSE(dynamicControl().ShouldSkipOperation("foo", iop));
+
+ // Enable GetVirtualAbFeatureFlag in the mock interface.
+ ON_CALL(dynamicControl(), GetVirtualAbFeatureFlag())
+ .WillByDefault(Return(FeatureFlag(FeatureFlag::Value::LAUNCH)));
+
+ // By default target_supports_snapshot_ is set to false. Cannot skip
+ // operation.
+ EXPECT_FALSE(dynamicControl().ShouldSkipOperation("foo", iop));
+
+ SetSnapshotEnabled(true);
+
+ // Empty source and destination. Skip.
+ EXPECT_TRUE(dynamicControl().ShouldSkipOperation("foo", iop));
+
+ se = iop.add_src_extents();
+ se->set_start_block(0);
+ se->set_num_blocks(1);
+
+ // There is something in sources, but destinations are empty. Cannot skip.
+ EXPECT_FALSE(dynamicControl().ShouldSkipOperation("foo", iop));
+
+ InstallOperation iop2;
+
+ de = iop2.add_dst_extents();
+ de->set_start_block(0);
+ de->set_num_blocks(1);
+
+ // There is something in destinations, but sources are empty. Cannot skip.
+ EXPECT_FALSE(dynamicControl().ShouldSkipOperation("foo", iop2));
+
+ de = iop.add_dst_extents();
+ de->set_start_block(0);
+ de->set_num_blocks(1);
+
+ // Sources and destinations are identical. Skip.
+ EXPECT_TRUE(dynamicControl().ShouldSkipOperation("foo", iop));
+
+ se = iop.add_src_extents();
+ se->set_start_block(1);
+ se->set_num_blocks(5);
+
+ // There is something in source, but not in destination. Cannot skip.
+ EXPECT_FALSE(dynamicControl().ShouldSkipOperation("foo", iop));
+
+ de = iop.add_dst_extents();
+ de->set_start_block(1);
+ de->set_num_blocks(5);
+
+ // There is source and destination are equal. Skip.
+ EXPECT_TRUE(dynamicControl().ShouldSkipOperation("foo", iop));
+
+ de = iop.add_dst_extents();
+ de->set_start_block(6);
+ de->set_num_blocks(5);
+
+ // There is something extra in dest. Cannot skip.
+ EXPECT_FALSE(dynamicControl().ShouldSkipOperation("foo", iop));
+
+ se = iop.add_src_extents();
+ se->set_start_block(6);
+ se->set_num_blocks(5);
+
+ // Source and dest are identical again. Skip.
+ EXPECT_TRUE(dynamicControl().ShouldSkipOperation("foo", iop));
+
+ // Don't skip for static partitions.
+ EXPECT_FALSE(dynamicControl().ShouldSkipOperation("bar", iop));
+}
+
} // namespace chromeos_update_engine
diff --git a/mock_dynamic_partition_control.h b/mock_dynamic_partition_control.h
index db8e834..09b825d 100644
--- a/mock_dynamic_partition_control.h
+++ b/mock_dynamic_partition_control.h
@@ -17,6 +17,7 @@
#include <stdint.h>
#include <memory>
+#include <set>
#include <string>
#include <gmock/gmock.h>
@@ -69,6 +70,10 @@
MOCK_METHOD1(GetSuperPartitionName, std::string(uint32_t));
MOCK_METHOD0(GetVirtualAbFeatureFlag, FeatureFlag());
MOCK_METHOD0(FinishUpdate, bool());
+
+ void set_fake_mapped_devices(const std::set<std::string>& fake) override {
+ DynamicPartitionControlAndroid::set_fake_mapped_devices(fake);
+ }
};
} // namespace chromeos_update_engine
diff --git a/payload_consumer/delta_performer.cc b/payload_consumer/delta_performer.cc
index f9244a9..c49474c 100644
--- a/payload_consumer/delta_performer.cc
+++ b/payload_consumer/delta_performer.cc
@@ -1154,7 +1154,16 @@
TEST_AND_RETURN_FALSE(source_fd_ != nullptr);
+ // The device may optimize the SOURCE_COPY operation.
+ // Being this a device-specific optimization let DynamicPartitionController
+ // decide it the operation should be skipped.
+ const PartitionUpdate& partition = partitions_[current_partition_];
+ const auto& partition_control = boot_control_->GetDynamicPartitionControl();
+ bool should_skip = partition_control->ShouldSkipOperation(
+ partition.partition_name(), operation);
+
if (operation.has_src_sha256_hash()) {
+ bool read_ok;
brillo::Blob source_hash;
brillo::Blob expected_source_hash(operation.src_sha256_hash().begin(),
operation.src_sha256_hash().end());
@@ -1163,12 +1172,17 @@
// device doesn't match or there was an error reading the source partition.
// Note that this code will also fall back if writing the target partition
// fails.
- bool read_ok = fd_utils::CopyAndHashExtents(source_fd_,
- operation.src_extents(),
- target_fd_,
- operation.dst_extents(),
- block_size_,
- &source_hash);
+ if (should_skip) {
+ read_ok = fd_utils::ReadAndHashExtents(
+ source_fd_, operation.src_extents(), block_size_, &source_hash);
+ } else {
+ read_ok = fd_utils::CopyAndHashExtents(source_fd_,
+ operation.src_extents(),
+ target_fd_,
+ operation.dst_extents(),
+ block_size_,
+ &source_hash);
+ }
if (read_ok && expected_source_hash == source_hash)
return true;
@@ -1185,12 +1199,18 @@
<< base::HexEncode(expected_source_hash.data(),
expected_source_hash.size());
- TEST_AND_RETURN_FALSE(fd_utils::CopyAndHashExtents(source_ecc_fd_,
- operation.src_extents(),
- target_fd_,
- operation.dst_extents(),
- block_size_,
- &source_hash));
+ if (should_skip) {
+ TEST_AND_RETURN_FALSE(fd_utils::ReadAndHashExtents(
+ source_ecc_fd_, operation.src_extents(), block_size_, &source_hash));
+ } else {
+ TEST_AND_RETURN_FALSE(
+ fd_utils::CopyAndHashExtents(source_ecc_fd_,
+ operation.src_extents(),
+ target_fd_,
+ operation.dst_extents(),
+ block_size_,
+ &source_hash));
+ }
TEST_AND_RETURN_FALSE(
ValidateSourceHash(source_hash, operation, source_ecc_fd_, error));
// At this point reading from the the error corrected device worked, but
@@ -1202,6 +1222,10 @@
// corrected device first since we can't verify the block in the raw device
// at this point, but we fall back to the raw device since the error
// corrected device can be shorter or not available.
+
+ if (should_skip)
+ return true;
+
if (OpenCurrentECCPartition() &&
fd_utils::CopyAndHashExtents(source_ecc_fd_,
operation.src_extents(),