Merge ac7fc8e1debff1f57afe79f144d8d16a1a0ec470 on remote branch

Change-Id: I2cd41cefedfd492f9e20617ad9929ebf1cdde79e

25 files changed, 337 insertions, 59 deletions

diff --git a/Android.bp b/Android.bp
index dab97110..d74e78f3 100644
--- a/Android.bp
+++ b/Android.bp
@@ -348,6 +348,9 @@ cc_defaults {
         "libstatslog",
         "libutils",
     ],
+    whole_static_libs: [
+        "com.android.sysprop.apex",
+    ],
 }
 
 cc_library_static {
@@ -789,6 +792,7 @@ cc_test {
         "libcurl_http_fetcher_unittest.cc",
         "payload_consumer/bzip_extent_writer_unittest.cc",
         "payload_consumer/cached_file_descriptor_unittest.cc",
+        "payload_consumer/cow_writer_file_descriptor_unittest.cc",
         "payload_consumer/certificate_parser_android_unittest.cc",
         "payload_consumer/delta_performer_integration_test.cc",
         "payload_consumer/delta_performer_unittest.cc",
diff --git a/aosp/apex_handler_android.cc b/aosp/apex_handler_android.cc
index 38ec410b..8beef966 100644
--- a/aosp/apex_handler_android.cc
+++ b/aosp/apex_handler_android.cc
@@ -14,10 +14,13 @@
 // limitations under the License.
 //
 
+#include <memory>
 #include <utility>
 
 #include <base/files/file_util.h>
 
+#include <ApexProperties.sysprop.h>
+
 #include "update_engine/aosp/apex_handler_android.h"
 #include "update_engine/common/utils.h"
 
@@ -44,6 +47,14 @@ android::apex::CompressedApexInfoList CreateCompressedApexInfoList(
 
 }  // namespace
 
+std::unique_ptr<ApexHandlerInterface> CreateApexHandler() {
+  if (android::sysprop::ApexProperties::updatable().value_or(false)) {
+    return std::make_unique<ApexHandlerAndroid>();
+  } else {
+    return std::make_unique<FlattenedApexHandlerAndroid>();
+  }
+}
+
 android::base::Result<uint64_t> ApexHandlerAndroid::CalculateSize(
     const std::vector<ApexInfo>& apex_infos) const {
   // We might not need to decompress every APEX. Communicate with apexd to get
@@ -86,4 +97,14 @@ android::sp<android::apex::IApexService> ApexHandlerAndroid::GetApexService()
   return android::interface_cast<android::apex::IApexService>(binder);
 }
 
+android::base::Result<uint64_t> FlattenedApexHandlerAndroid::CalculateSize(
+    const std::vector<ApexInfo>& apex_infos) const {
+  return 0;
+}
+
+bool FlattenedApexHandlerAndroid::AllocateSpace(
+    const std::vector<ApexInfo>& apex_infos) const {
+  return true;
+}
+
 }  // namespace chromeos_update_engine
diff --git a/aosp/apex_handler_android.h b/aosp/apex_handler_android.h
index 00f3a80b..767f5618 100644
--- a/aosp/apex_handler_android.h
+++ b/aosp/apex_handler_android.h
@@ -17,6 +17,7 @@
 #ifndef SYSTEM_UPDATE_ENGINE_AOSP_APEX_HANDLER_ANDROID_H_
 #define SYSTEM_UPDATE_ENGINE_AOSP_APEX_HANDLER_ANDROID_H_
 
+#include <memory>
 #include <string>
 #include <vector>
 
@@ -28,6 +29,8 @@
 
 namespace chromeos_update_engine {
 
+std::unique_ptr<ApexHandlerInterface> CreateApexHandler();
+
 class ApexHandlerAndroid : virtual public ApexHandlerInterface {
  public:
   android::base::Result<uint64_t> CalculateSize(
@@ -38,6 +41,13 @@ class ApexHandlerAndroid : virtual public ApexHandlerInterface {
   android::sp<android::apex::IApexService> GetApexService() const;
 };
 
+class FlattenedApexHandlerAndroid : virtual public ApexHandlerInterface {
+ public:
+  android::base::Result<uint64_t> CalculateSize(
+      const std::vector<ApexInfo>& apex_infos) const;
+  bool AllocateSpace(const std::vector<ApexInfo>& apex_infos) const;
+};
+
 }  // namespace chromeos_update_engine
 
 #endif  // SYSTEM_UPDATE_ENGINE_AOSP_APEX_HANDLER_ANDROID_H_
diff --git a/aosp/apex_handler_android_unittest.cc b/aosp/apex_handler_android_unittest.cc
index 981ae9dd..847ccaad 100644
--- a/aosp/apex_handler_android_unittest.cc
+++ b/aosp/apex_handler_android_unittest.cc
@@ -41,7 +41,7 @@ ApexInfo CreateApexInfo(const std::string& package_name,
   return std::move(result);
 }
 
-TEST(ApexHandlerAndroidTest, CalculateSize) {
+TEST(ApexHandlerAndroidTest, CalculateSizeUpdatableApex) {
   ApexHandlerAndroid apex_handler;
   std::vector<ApexInfo> apex_infos;
   ApexInfo compressed_apex_1 = CreateApexInfo("sample1", 1, true, 1);
@@ -52,10 +52,10 @@ TEST(ApexHandlerAndroidTest, CalculateSize) {
   apex_infos.push_back(uncompressed_apex);
   auto result = apex_handler.CalculateSize(apex_infos);
   ASSERT_TRUE(result.ok());
-  EXPECT_EQ(*result, 3u);
+  ASSERT_EQ(*result, 3u);
 }
 
-TEST(ApexHandlerAndroidTest, AllocateSpace) {
+TEST(ApexHandlerAndroidTest, AllocateSpaceUpdatableApex) {
   ApexHandlerAndroid apex_handler;
   std::vector<ApexInfo> apex_infos;
   ApexInfo compressed_apex_1 = CreateApexInfo("sample1", 1, true, 1);
@@ -64,10 +64,39 @@ TEST(ApexHandlerAndroidTest, AllocateSpace) {
   apex_infos.push_back(compressed_apex_1);
   apex_infos.push_back(compressed_apex_2);
   apex_infos.push_back(uncompressed_apex);
-  EXPECT_TRUE(apex_handler.AllocateSpace(apex_infos));
+  ASSERT_TRUE(apex_handler.AllocateSpace(apex_infos));
 
   // Should be able to pass empty list
-  EXPECT_TRUE(apex_handler.AllocateSpace({}));
+  ASSERT_TRUE(apex_handler.AllocateSpace({}));
+}
+
+TEST(ApexHandlerAndroidTest, CalculateSizeFlattenedApex) {
+  FlattenedApexHandlerAndroid apex_handler;
+  std::vector<ApexInfo> apex_infos;
+  ApexInfo compressed_apex_1 = CreateApexInfo("sample1", 1, true, 1);
+  ApexInfo compressed_apex_2 = CreateApexInfo("sample2", 2, true, 2);
+  ApexInfo uncompressed_apex = CreateApexInfo("uncompressed", 1, false, 4);
+  apex_infos.push_back(compressed_apex_1);
+  apex_infos.push_back(compressed_apex_2);
+  apex_infos.push_back(uncompressed_apex);
+  auto result = apex_handler.CalculateSize(apex_infos);
+  ASSERT_TRUE(result.ok());
+  ASSERT_EQ(*result, 0u);
+}
+
+TEST(ApexHandlerAndroidTest, AllocateSpaceFlattenedApex) {
+  FlattenedApexHandlerAndroid apex_handler;
+  std::vector<ApexInfo> apex_infos;
+  ApexInfo compressed_apex_1 = CreateApexInfo("sample1", 1, true, 1);
+  ApexInfo compressed_apex_2 = CreateApexInfo("sample2", 2, true, 2);
+  ApexInfo uncompressed_apex = CreateApexInfo("uncompressed", 1, false, 4);
+  apex_infos.push_back(compressed_apex_1);
+  apex_infos.push_back(compressed_apex_2);
+  apex_infos.push_back(uncompressed_apex);
+  ASSERT_TRUE(apex_handler.AllocateSpace(apex_infos));
+
+  // Should be able to pass empty list
+  ASSERT_TRUE(apex_handler.AllocateSpace({}));
 }
 
 }  // namespace chromeos_update_engine
diff --git a/aosp/cleanup_previous_update_action.cc b/aosp/cleanup_previous_update_action.cc
index 68954f6f..dde6b89a 100644
--- a/aosp/cleanup_previous_update_action.cc
+++ b/aosp/cleanup_previous_update_action.cc
@@ -324,6 +324,7 @@ void CleanupPreviousUpdateAction::WaitForMergeOrSchedule() {
 
     case UpdateState::MergeFailed: {
       LOG(ERROR) << "Merge failed. Device may be corrupted.";
+      merge_stats_->set_merge_failure_code(snapshot_->ReadMergeFailureCode());
       processor_->ActionComplete(this, ErrorCode::kDeviceCorrupted);
       return;
     }
@@ -492,7 +493,8 @@ void CleanupPreviousUpdateAction::ReportMergeStats() {
                              report.total_cow_size_bytes(),
                              report.estimated_cow_size_bytes(),
                              report.boot_complete_time_ms(),
-                             report.boot_complete_to_merge_start_time_ms());
+                             report.boot_complete_to_merge_start_time_ms(),
+                             static_cast<int32_t>(report.merge_failure_code()));
 #endif
 }
 
diff --git a/aosp/daemon_state_android.cc b/aosp/daemon_state_android.cc
index fc89d73e..da49080c 100644
--- a/aosp/daemon_state_android.cc
+++ b/aosp/daemon_state_android.cc
@@ -65,12 +65,11 @@ bool DaemonStateAndroid::Initialize() {
   certificate_checker_->Init();
 
   // Initialize the UpdateAttempter before the UpdateManager.
-  update_attempter_.reset(
-      new UpdateAttempterAndroid(this,
-                                 prefs_.get(),
-                                 boot_control_.get(),
-                                 hardware_.get(),
-                                 std::make_unique<ApexHandlerAndroid>()));
+  update_attempter_.reset(new UpdateAttempterAndroid(this,
+                                                     prefs_.get(),
+                                                     boot_control_.get(),
+                                                     hardware_.get(),
+                                                     CreateApexHandler()));
 
   return true;
 }
diff --git a/aosp/dynamic_partition_control_android.cc b/aosp/dynamic_partition_control_android.cc
index ab349a82..538b57ce 100644
--- a/aosp/dynamic_partition_control_android.cc
+++ b/aosp/dynamic_partition_control_android.cc
@@ -1116,9 +1116,9 @@ DynamicPartitionControlAndroid::GetPartitionDevice(
   if (UpdateUsesSnapshotCompression() && slot != current_slot &&
       IsDynamicPartition(partition_name, slot)) {
     return {
-        {.mountable_device_path = base::FilePath{std::string{VABC_DEVICE_DIR}}
-                                      .Append(partition_name_suffix)
-                                      .value(),
+        {.readonly_device_path = base::FilePath{std::string{VABC_DEVICE_DIR}}
+                                     .Append(partition_name_suffix)
+                                     .value(),
          .is_dynamic = true}};
   }
 
@@ -1137,7 +1137,7 @@ DynamicPartitionControlAndroid::GetPartitionDevice(
                                       &device)) {
       case DynamicPartitionDeviceStatus::SUCCESS:
         return {{.rw_device_path = device,
-                 .mountable_device_path = device,
+                 .readonly_device_path = device,
                  .is_dynamic = true}};
 
       case DynamicPartitionDeviceStatus::TRY_STATIC:
@@ -1155,7 +1155,7 @@ DynamicPartitionControlAndroid::GetPartitionDevice(
   }
 
   return {{.rw_device_path = static_path,
-           .mountable_device_path = static_path,
+           .readonly_device_path = static_path,
            .is_dynamic = false}};
 }
 
@@ -1470,7 +1470,8 @@ bool DynamicPartitionControlAndroid::IsDynamicPartition(
 }
 
 bool DynamicPartitionControlAndroid::UpdateUsesSnapshotCompression() {
-  return snapshot_->UpdateUsesCompression();
+  return GetVirtualAbFeatureFlag().IsEnabled() &&
+         snapshot_->UpdateUsesCompression();
 }
 
 }  // namespace chromeos_update_engine
diff --git a/aosp/dynamic_partition_control_android_unittest.cc b/aosp/dynamic_partition_control_android_unittest.cc
index 0bb8df7a..6f1d4ef8 100644
--- a/aosp/dynamic_partition_control_android_unittest.cc
+++ b/aosp/dynamic_partition_control_android_unittest.cc
@@ -431,7 +431,7 @@ TEST_P(DynamicPartitionControlAndroidTestP, GetMountableDevicePath) {
   auto device_info =
       dynamicControl().GetPartitionDevice("system", target(), source(), false);
   ASSERT_TRUE(device_info.has_value());
-  ASSERT_EQ(device_info->mountable_device_path, device);
+  ASSERT_EQ(device_info->readonly_device_path, device);
 }
 
 TEST_P(DynamicPartitionControlAndroidTestP, GetMountableDevicePathVABC) {
@@ -475,7 +475,7 @@ TEST_P(DynamicPartitionControlAndroidTestP, GetMountableDevicePathVABC) {
   ASSERT_TRUE(device_info.has_value());
   base::FilePath vabc_device_dir{
       std::string{DynamicPartitionControlAndroid::VABC_DEVICE_DIR}};
-  ASSERT_EQ(device_info->mountable_device_path,
+  ASSERT_EQ(device_info->readonly_device_path,
             vabc_device_dir.Append(T("system")).value());
 }
 
diff --git a/common/dynamic_partition_control_interface.h b/common/dynamic_partition_control_interface.h
index d5e1d8d2..a5be6e1a 100644
--- a/common/dynamic_partition_control_interface.h
+++ b/common/dynamic_partition_control_interface.h
@@ -39,7 +39,7 @@ namespace chromeos_update_engine {
 
 struct PartitionDevice {
   std::string rw_device_path;
-  std::string mountable_device_path;
+  std::string readonly_device_path;
   bool is_dynamic;
 };
 
diff --git a/common/fake_boot_control.h b/common/fake_boot_control.h
index fc7839df..79e21390 100644
--- a/common/fake_boot_control.h
+++ b/common/fake_boot_control.h
@@ -137,7 +137,7 @@ class FakeBootControl : public BootControlInterface {
     PartitionDevice device;
     device.is_dynamic = false;
     device.rw_device_path = device_path->second;
-    device.mountable_device_path = device.rw_device_path;
+    device.readonly_device_path = device.rw_device_path;
     return device;
   }
 
diff --git a/common/hash_calculator.cc b/common/hash_calculator.cc
index d010a532..60812d56 100644
--- a/common/hash_calculator.cc
+++ b/common/hash_calculator.cc
@@ -95,6 +95,11 @@ bool HashCalculator::RawHashOfData(const brillo::Blob& data,
   return RawHashOfBytes(data.data(), data.size(), out_hash);
 }
 
+bool HashCalculator::RawHashOfFile(const string& name, brillo::Blob* out_hash) {
+  const auto file_size = utils::FileSize(name);
+  return RawHashOfFile(name, file_size, out_hash) == file_size;
+}
+
 off_t HashCalculator::RawHashOfFile(const string& name,
                                     off_t length,
                                     brillo::Blob* out_hash) {
diff --git a/common/hash_calculator.h b/common/hash_calculator.h
index b7e4d86c..44261284 100644
--- a/common/hash_calculator.h
+++ b/common/hash_calculator.h
@@ -75,6 +75,7 @@ class HashCalculator {
   static off_t RawHashOfFile(const std::string& name,
                              off_t length,
                              brillo::Blob* out_hash);
+  static bool RawHashOfFile(const std::string& name, brillo::Blob* out_hash);
 
  private:
   // If non-empty, the final raw hash. Will only be set to non-empty when
diff --git a/common/utils.h b/common/utils.h
index 5f6e4757..59f236ef 100644
--- a/common/utils.h
+++ b/common/utils.h
@@ -399,13 +399,19 @@ class ScopedTempFile {
 
   // If |open_fd| is true, a writable file descriptor will be opened for this
   // file.
-  explicit ScopedTempFile(const std::string& pattern, bool open_fd = false) {
+  // If |truncate_size| is non-zero, truncate file to that size on creation.
+  explicit ScopedTempFile(const std::string& pattern,
+                          bool open_fd = false,
+                          size_t truncate_size = 0) {
     CHECK(utils::MakeTempFile(pattern, &path_, open_fd ? &fd_ : nullptr));
     unlinker_.reset(new ScopedPathUnlinker(path_));
     if (open_fd) {
       CHECK_GE(fd_, 0);
       fd_closer_.reset(new ScopedFdCloser(&fd_));
     }
+    if (truncate_size > 0) {
+      CHECK_EQ(0, truncate(path_.c_str(), truncate_size));
+    }
   }
   virtual ~ScopedTempFile() = default;
 
diff --git a/payload_consumer/cow_writer_file_descriptor.cc b/payload_consumer/cow_writer_file_descriptor.cc
index d8c7afb8..2de6664a 100644
--- a/payload_consumer/cow_writer_file_descriptor.cc
+++ b/payload_consumer/cow_writer_file_descriptor.cc
@@ -28,7 +28,10 @@ namespace chromeos_update_engine {
 CowWriterFileDescriptor::CowWriterFileDescriptor(
     std::unique_ptr<android::snapshot::ISnapshotWriter> cow_writer)
     : cow_writer_(std::move(cow_writer)),
-      cow_reader_(cow_writer_->OpenReader()) {}
+      cow_reader_(cow_writer_->OpenReader()) {
+  CHECK_NE(cow_writer_, nullptr);
+  CHECK_NE(cow_reader_, nullptr);
+}
 
 bool CowWriterFileDescriptor::Open(const char* path, int flags, mode_t mode) {
   LOG(ERROR) << "CowWriterFileDescriptor doesn't support Open()";
@@ -113,7 +116,17 @@ bool CowWriterFileDescriptor::Flush() {
 
 bool CowWriterFileDescriptor::Close() {
   if (cow_writer_) {
-    TEST_AND_RETURN_FALSE(cow_writer_->Finalize());
+    // b/186196758
+    // When calling
+    // InitializeAppend(kEndOfInstall), the SnapshotWriter only reads up to the
+    // given label. But OpenReader() completely disregards the resume label and
+    // reads all ops. Therefore, update_engine sees the verity data. However,
+    // when calling SnapshotWriter::Finalize(), data after resume label are
+    // discarded, therefore verity data is gone. To prevent phantom reads, don't
+    // call Finalize() unless we actually write something.
+    if (dirty_) {
+      TEST_AND_RETURN_FALSE(cow_writer_->Finalize());
+    }
     cow_writer_ = nullptr;
   }
   if (cow_reader_) {
diff --git a/payload_consumer/cow_writer_file_descriptor_unittest.cc b/payload_consumer/cow_writer_file_descriptor_unittest.cc
new file mode 100644
index 00000000..c596e3bb
--- /dev/null
+++ b/payload_consumer/cow_writer_file_descriptor_unittest.cc
@@ -0,0 +1,120 @@
+//
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+#include "update_engine/payload_consumer/cow_writer_file_descriptor.h"
+
+#include <cstring>
+#include <memory>
+#include <utility>
+#include <vector>
+
+#include <android-base/unique_fd.h>
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+#include <libsnapshot/snapshot_writer.h>
+
+#include "update_engine/common/utils.h"
+
+namespace chromeos_update_engine {
+constexpr size_t BLOCK_SIZE = 4096;
+constexpr size_t PARTITION_SIZE = BLOCK_SIZE * 10;
+
+using android::base::unique_fd;
+using android::snapshot::CompressedSnapshotWriter;
+using android::snapshot::CowOptions;
+using android::snapshot::ISnapshotWriter;
+
+class CowWriterFileDescriptorUnittest : public ::testing::Test {
+ public:
+  void SetUp() override {
+    ASSERT_EQ(ftruncate64(cow_device_file_.fd(), PARTITION_SIZE), 0)
+        << "Failed to truncate cow_device file to " << PARTITION_SIZE
+        << strerror(errno);
+    ASSERT_EQ(ftruncate64(cow_source_file_.fd(), PARTITION_SIZE), 0)
+        << "Failed to truncate cow_source file to " << PARTITION_SIZE
+        << strerror(errno);
+  }
+
+  std::unique_ptr<CompressedSnapshotWriter> GetCowWriter() {
+    const CowOptions options{.block_size = BLOCK_SIZE, .compression = "gz"};
+    auto snapshot_writer = std::make_unique<CompressedSnapshotWriter>(options);
+    int fd = open(cow_device_file_.path().c_str(), O_RDWR);
+    EXPECT_NE(fd, -1);
+    EXPECT_TRUE(snapshot_writer->SetCowDevice(unique_fd{fd}));
+    snapshot_writer->SetSourceDevice(cow_source_file_.path());
+    return snapshot_writer;
+  }
+  CowWriterFileDescriptor GetCowFd() {
+    auto cow_writer = GetCowWriter();
+    return CowWriterFileDescriptor{std::move(cow_writer)};
+  }
+
+  ScopedTempFile cow_source_file_{"cow_source.XXXXXX", true};
+  ScopedTempFile cow_device_file_{"cow_device.XXXXXX", true};
+};
+
+TEST_F(CowWriterFileDescriptorUnittest, ReadAfterWrite) {
+  std::vector<unsigned char> buffer;
+  buffer.resize(BLOCK_SIZE);
+  std::fill(buffer.begin(), buffer.end(), 234);
+
+  std::vector<unsigned char> verity_data;
+  verity_data.resize(BLOCK_SIZE);
+  std::fill(verity_data.begin(), verity_data.end(), 0xAA);
+
+  auto cow_writer = GetCowWriter();
+  cow_writer->Initialize();
+
+  // Simulate Writing InstallOp data
+  ASSERT_TRUE(cow_writer->AddRawBlocks(0, buffer.data(), buffer.size()));
+  ASSERT_TRUE(cow_writer->AddZeroBlocks(1, 2));
+  ASSERT_TRUE(cow_writer->AddCopy(3, 1));
+  // Fake label to simulate "end of install"
+  ASSERT_TRUE(cow_writer->AddLabel(23));
+  ASSERT_TRUE(
+      cow_writer->AddRawBlocks(4, verity_data.data(), verity_data.size()));
+  ASSERT_TRUE(cow_writer->Finalize());
+
+  cow_writer = GetCowWriter();
+  ASSERT_NE(nullptr, cow_writer);
+  ASSERT_TRUE(cow_writer->InitializeAppend(23));
+  auto cow_fd =
+      std::make_unique<CowWriterFileDescriptor>(std::move(cow_writer));
+
+  ASSERT_EQ((ssize_t)BLOCK_SIZE * 4, cow_fd->Seek(BLOCK_SIZE * 4, SEEK_SET));
+  std::vector<unsigned char> read_back(4096);
+  ASSERT_EQ((ssize_t)read_back.size(),
+            cow_fd->Read(read_back.data(), read_back.size()));
+  ASSERT_EQ(verity_data, read_back);
+
+  // Since we didn't write anything to this instance of cow_fd, destructor
+  // should not call Finalize(). As finalize will drop ops after resume label,
+  // causing subsequent reads to fail.
+  cow_writer = GetCowWriter();
+  ASSERT_NE(nullptr, cow_writer);
+  ASSERT_TRUE(cow_writer->InitializeAppend(23));
+  cow_fd = std::make_unique<CowWriterFileDescriptor>(std::move(cow_writer));
+
+  ASSERT_EQ((ssize_t)BLOCK_SIZE * 4, cow_fd->Seek(BLOCK_SIZE * 4, SEEK_SET));
+  ASSERT_EQ((ssize_t)read_back.size(),
+            cow_fd->Read(read_back.data(), read_back.size()));
+  ASSERT_EQ(verity_data, read_back)
+      << "Could not read verity data afeter InitializeAppend() => Read() => "
+         "InitializeAppend() sequence. If no writes happened while CowWriterFd "
+         "is open, Finalize() should not be called.";
+}
+
+}  // namespace chromeos_update_engine
diff --git a/payload_consumer/filesystem_verifier_action.cc b/payload_consumer/filesystem_verifier_action.cc
index 09dc638b..b14cbc8d 100644
--- a/payload_consumer/filesystem_verifier_action.cc
+++ b/payload_consumer/filesystem_verifier_action.cc
@@ -112,6 +112,14 @@ void FilesystemVerifierAction::Cleanup(ErrorCode code) {
   // This memory is not used anymore.
   buffer_.clear();
 
+  // If we didn't write verity, partitions were maped. Releaase resource now.
+  if (!install_plan_.write_verity &&
+      dynamic_control_->UpdateUsesSnapshotCompression()) {
+    LOG(INFO) << "Not writing verity and VABC is enabled, unmapping all "
+                 "partitions";
+    dynamic_control_->UnmapAllPartitions();
+  }
+
   if (cancelled_)
     return;
   if (code == ErrorCode::kSuccess && HasOutputPipe())
@@ -130,6 +138,21 @@ bool FilesystemVerifierAction::InitializeFdVABC() {
   const InstallPlan::Partition& partition =
       install_plan_.partitions[partition_index_];
 
+  if (!ShouldWriteVerity()) {
+    // In VABC, if we are not writing verity, just map all partitions,
+    // and read using regular fd on |postinstall_mount_device| .
+    // All read will go through snapuserd, which provides a consistent
+    // view: device will use snapuserd to read partition during boot.
+    // b/186196758
+    // Call UnmapAllPartitions() first, because if we wrote verity before, these
+    // writes won't be visible to previously opened snapuserd daemon. To ensure
+    // that we will see the most up to date data from partitions, call Unmap()
+    // then Map() to re-spin daemon.
+    dynamic_control_->UnmapAllPartitions();
+    dynamic_control_->MapAllPartitions();
+    return InitializeFd(partition.readonly_target_path);
+  }
+
   // FilesystemVerifierAction need the read_fd_.
   partition_fd_ =
       dynamic_control_->OpenCowFd(partition.name, partition.source_path, true);
diff --git a/payload_consumer/filesystem_verifier_action_unittest.cc b/payload_consumer/filesystem_verifier_action_unittest.cc
index c1006842..586662d9 100644
--- a/payload_consumer/filesystem_verifier_action_unittest.cc
+++ b/payload_consumer/filesystem_verifier_action_unittest.cc
@@ -48,8 +48,23 @@ using testing::SetArgPointee;
 namespace chromeos_update_engine {
 
 class FilesystemVerifierActionTest : public ::testing::Test {
+ public:
+  static constexpr size_t BLOCK_SIZE = 4096;
+  static constexpr size_t PARTITION_SIZE = BLOCK_SIZE * 1024;
+
  protected:
-  void SetUp() override { loop_.SetAsCurrent(); }
+  void SetUp() override {
+    brillo::Blob part_data(PARTITION_SIZE);
+    test_utils::FillWithData(&part_data);
+    ASSERT_TRUE(utils::WriteFile(
+        source_part.path().c_str(), part_data.data(), part_data.size()));
+    // FillWithData() will will with different data next call. We want
+    // source/target partitions to contain different data for testing.
+    test_utils::FillWithData(&part_data);
+    ASSERT_TRUE(utils::WriteFile(
+        target_part.path().c_str(), part_data.data(), part_data.size()));
+    loop_.SetAsCurrent();
+  }
 
   void TearDown() override {
     EXPECT_EQ(0, brillo::MessageLoopRunMaxIterations(&loop_, 1));
@@ -62,11 +77,34 @@ class FilesystemVerifierActionTest : public ::testing::Test {
   void BuildActions(const InstallPlan& install_plan,
                     DynamicPartitionControlInterface* dynamic_control);
 
+  InstallPlan::Partition* AddFakePartition(InstallPlan* install_plan,
+                                           std::string name = "fake_part") {
+    InstallPlan::Partition& part = install_plan->partitions.emplace_back();
+    part.name = name;
+    part.target_path = target_part.path();
+    part.readonly_target_path = part.target_path;
+    part.target_size = PARTITION_SIZE;
+    part.block_size = BLOCK_SIZE;
+    part.source_path = source_part.path();
+    EXPECT_TRUE(
+        HashCalculator::RawHashOfFile(source_part.path(), &part.source_hash));
+    EXPECT_TRUE(
+        HashCalculator::RawHashOfFile(target_part.path(), &part.target_hash));
+    return &part;
+  }
+
   brillo::FakeMessageLoop loop_{nullptr};
   ActionProcessor processor_;
   DynamicPartitionControlStub dynamic_control_stub_;
+  static ScopedTempFile source_part;
+  static ScopedTempFile target_part;
 };
 
+ScopedTempFile FilesystemVerifierActionTest::source_part{
+    "source_part.XXXXXX", false, PARTITION_SIZE};
+ScopedTempFile FilesystemVerifierActionTest::target_part{
+    "target_part.XXXXXX", false, PARTITION_SIZE};
+
 class FilesystemVerifierActionTestDelegate : public ActionProcessorDelegate {
  public:
   FilesystemVerifierActionTestDelegate()
@@ -406,32 +444,27 @@ TEST_F(FilesystemVerifierActionTest, RunAsRootSkipWriteVerityTest) {
 
 TEST_F(FilesystemVerifierActionTest, RunWithVABCNoVerity) {
   InstallPlan install_plan;
-  InstallPlan::Partition& part = install_plan.partitions.emplace_back();
-  part.name = "fake_part";
-  part.target_path = "/dev/fake_target_path";
-  part.target_size = 4096 * 4096;
-  part.block_size = 4096;
-  part.source_path = "/dev/fake_source_path";
-  part.fec_size = 0;
-  part.hash_tree_size = 0;
-  part.target_hash.clear();
-  part.source_hash.clear();
+  auto part_ptr = AddFakePartition(&install_plan);
+  ASSERT_NE(part_ptr, nullptr);
+  InstallPlan::Partition& part = *part_ptr;
+  part.target_path = "Shouldn't attempt to open this path";
 
   NiceMock<MockDynamicPartitionControl> dynamic_control;
-  auto fake_fd = std::make_shared<FakeFileDescriptor>();
 
   ON_CALL(dynamic_control, GetDynamicPartitionsFeatureFlag())
       .WillByDefault(Return(FeatureFlag(FeatureFlag::Value::LAUNCH)));
   ON_CALL(dynamic_control, UpdateUsesSnapshotCompression())
       .WillByDefault(Return(true));
-  ON_CALL(dynamic_control, OpenCowFd(_, _, _)).WillByDefault(Return(fake_fd));
   ON_CALL(dynamic_control, IsDynamicPartition(part.name, _))
       .WillByDefault(Return(true));
 
   EXPECT_CALL(dynamic_control, UpdateUsesSnapshotCompression())
       .Times(AtLeast(1));
+  // Since we are not writing verity, we should not attempt to OpenCowFd()
+  // reads should go through regular file descriptors on mapped partitions.
   EXPECT_CALL(dynamic_control, OpenCowFd(part.name, {part.source_path}, _))
-      .Times(1);
+      .Times(0);
+  EXPECT_CALL(dynamic_control, MapAllPartitions()).Times(AtLeast(1));
   EXPECT_CALL(dynamic_control, ListDynamicPartitionsForSlot(_, _, _))
       .WillRepeatedly(
           DoAll(SetArgPointee<2, std::vector<std::string>>({part.name}),
@@ -451,20 +484,10 @@ TEST_F(FilesystemVerifierActionTest, RunWithVABCNoVerity) {
 
   ASSERT_FALSE(processor_.IsRunning());
   ASSERT_TRUE(delegate.ran());
-  // Filesystem verifier will fail, because we set an empty hash
-  ASSERT_EQ(ErrorCode::kNewRootfsVerificationError, delegate.code());
-  const auto& read_pos = fake_fd->GetReadOps();
-  size_t expected_offset = 0;
-  for (const auto& [off, size] : read_pos) {
-    ASSERT_EQ(off, expected_offset);
-    expected_offset += size;
-  }
-  const auto actual_read_size = expected_offset;
-  ASSERT_EQ(actual_read_size, part.target_size);
+  ASSERT_EQ(ErrorCode::kSuccess, delegate.code());
 }
 
 TEST_F(FilesystemVerifierActionTest, ReadAfterWrite) {
-  constexpr auto BLOCK_SIZE = 4096;
   ScopedTempFile cow_device_file("cow_device.XXXXXX", true);
   android::snapshot::CompressedSnapshotWriter snapshot_writer{
       {.block_size = BLOCK_SIZE}};
@@ -483,6 +506,12 @@ TEST_F(FilesystemVerifierActionTest, ReadAfterWrite) {
   ASSERT_TRUE(snapshot_writer.Finalize());
   cow_reader = snapshot_writer.OpenReader();
   ASSERT_NE(cow_reader, nullptr);
+  std::vector<unsigned char> read_back;
+  read_back.resize(buffer.size());
+  cow_reader->Seek(BLOCK_SIZE, SEEK_SET);
+  const auto bytes_read = cow_reader->Read(read_back.data(), read_back.size());
+  ASSERT_EQ((size_t)(bytes_read), BLOCK_SIZE);
+  ASSERT_EQ(read_back, buffer);
 }
 
 }  // namespace chromeos_update_engine
diff --git a/payload_consumer/install_plan.cc b/payload_consumer/install_plan.cc
index 39827a4a..06b7dd8c 100644
--- a/payload_consumer/install_plan.cc
+++ b/payload_consumer/install_plan.cc
@@ -122,6 +122,7 @@ string InstallPlan::ToString() const {
                              partition.target_hash.size())},
             {"run_postinstall", utils::ToString(partition.run_postinstall)},
             {"postinstall_path", partition.postinstall_path},
+            {"readonly_target_path", partition.readonly_target_path},
             {"filesystem_type", partition.filesystem_type},
         },
         "\n  "));
@@ -165,7 +166,7 @@ bool InstallPlan::LoadPartitionsFromSlots(BootControlInterface* boot_control) {
           partition.name, target_slot, source_slot);
       TEST_AND_RETURN_FALSE(device.has_value());
       partition.target_path = device->rw_device_path;
-      partition.postinstall_mount_device = device->mountable_device_path;
+      partition.readonly_target_path = device->readonly_device_path;
     } else {
       partition.target_path.clear();
     }
diff --git a/payload_consumer/install_plan.h b/payload_consumer/install_plan.h
index 43b94fc3..7c77789b 100644
--- a/payload_consumer/install_plan.h
+++ b/payload_consumer/install_plan.h
@@ -109,7 +109,7 @@ struct InstallPlan {
     std::string target_path;
     // |mountable_target_device| is intended to be a path to block device which
     // can be used for mounting this block device's underlying filesystem.
-    std::string postinstall_mount_device;
+    std::string readonly_target_path;
     uint64_t target_size{0};
     brillo::Blob target_hash;
 
diff --git a/payload_consumer/install_plan_unittest.cc b/payload_consumer/install_plan_unittest.cc
index 2ca8d811..77794949 100644
--- a/payload_consumer/install_plan_unittest.cc
+++ b/payload_consumer/install_plan_unittest.cc
@@ -38,6 +38,7 @@ TEST(InstallPlanTest, Dump) {
           .source_path = "foo-source-path",
           .source_hash = {0xb1, 0xb2},
           .target_path = "foo-target-path",
+          .readonly_target_path = "mountable-device",
           .target_hash = {0xb3, 0xb4},
           .postinstall_path = "foo-path",
           .filesystem_type = "foo-type",
@@ -66,6 +67,7 @@ Partition: foo-partition_name
   target_hash: B3B4
   run_postinstall: false
   postinstall_path: foo-path
+  readonly_target_path: mountable-device
   filesystem_type: foo-type
 Payload: 0
   urls: (url1,url2)
diff --git a/payload_consumer/postinstall_runner_action.cc b/payload_consumer/postinstall_runner_action.cc
index 8f2d674a..051ccbf7 100644
--- a/payload_consumer/postinstall_runner_action.cc
+++ b/payload_consumer/postinstall_runner_action.cc
@@ -140,7 +140,7 @@ void PostinstallRunnerAction::PerformPartitionPostinstall() {
   const InstallPlan::Partition& partition =
       install_plan_.partitions[current_partition_];
 
-  const string mountable_device = partition.postinstall_mount_device;
+  const string mountable_device = partition.readonly_target_path;
   if (mountable_device.empty()) {
     LOG(ERROR) << "Cannot make mountable device from " << partition.target_path;
     return CompletePostinstall(ErrorCode::kPostinstallRunnerError);
@@ -383,6 +383,11 @@ void PostinstallRunnerAction::CompletePostinstall(ErrorCode error_code) {
     }
   }
 
+  auto dynamic_control = boot_control_->GetDynamicPartitionControl();
+  CHECK(dynamic_control);
+  dynamic_control->UnmapAllPartitions();
+  LOG(INFO) << "Unmapped all partitions.";
+
   ScopedActionCompleter completer(processor_, this);
   completer.set_code(error_code);
 
@@ -401,10 +406,6 @@ void PostinstallRunnerAction::CompletePostinstall(ErrorCode error_code) {
   if (HasOutputPipe()) {
     SetOutputObject(install_plan_);
   }
-  auto dynamic_control = boot_control_->GetDynamicPartitionControl();
-  CHECK(dynamic_control);
-  dynamic_control->UnmapAllPartitions();
-  LOG(INFO) << "Unmapped all partitions.";
 }
 
 void PostinstallRunnerAction::SuspendAction() {
diff --git a/payload_consumer/postinstall_runner_action_unittest.cc b/payload_consumer/postinstall_runner_action_unittest.cc
index 5ee29898..792ee282 100644
--- a/payload_consumer/postinstall_runner_action_unittest.cc
+++ b/payload_consumer/postinstall_runner_action_unittest.cc
@@ -195,7 +195,7 @@ void PostinstallRunnerActionTest::RunPostinstallAction(
   InstallPlan::Partition part;
   part.name = "part";
   part.target_path = device_path;
-  part.postinstall_mount_device = device_path;
+  part.readonly_target_path = device_path;
   part.run_postinstall = true;
   part.postinstall_path = postinstall_program;
   InstallPlan install_plan;
@@ -360,7 +360,7 @@ TEST_F(PostinstallRunnerActionTest, RunAsRootSkipOptionalPostinstallTest) {
   InstallPlan::Partition part;
   part.name = "part";
   part.target_path = "/dev/null";
-  part.postinstall_mount_device = "/dev/null";
+  part.readonly_target_path = "/dev/null";
   part.run_postinstall = true;
   part.postinstall_path = kPostinstallDefaultScript;
   part.postinstall_optional = true;
diff --git a/payload_consumer/snapshot_extent_writer.h b/payload_consumer/snapshot_extent_writer.h
index c3c64cda..c3a948ee 100644
--- a/payload_consumer/snapshot_extent_writer.h
+++ b/payload_consumer/snapshot_extent_writer.h
@@ -14,6 +14,9 @@
 // limitations under the License.
 //
 
+#ifndef UPDATE_ENGINE_SNAPSHOT_EXTENT_WRITER_H_
+#define UPDATE_ENGINE_SNAPSHOT_EXTENT_WRITER_H_
+
 #include <cstdint>
 #include <vector>
 
@@ -52,3 +55,5 @@ class SnapshotExtentWriter : public chromeos_update_engine::ExtentWriter {
 };
 
 }  // namespace chromeos_update_engine
+
+#endif
diff --git a/payload_generator/payload_generation_config.cc b/payload_generator/payload_generation_config.cc
index d45de6a6..2cd2ebcc 100644
--- a/payload_generator/payload_generation_config.cc
+++ b/payload_generator/payload_generation_config.cc
@@ -23,6 +23,7 @@
 #include <base/logging.h>
 #include <base/strings/string_number_conversions.h>
 #include <brillo/strings/string_utils.h>
+#include <libsnapshot/cow_format.h>
 
 #include "update_engine/common/utils.h"
 #include "update_engine/payload_consumer/delta_performer.h"
@@ -185,6 +186,7 @@ bool ImageConfig::LoadDynamicPartitionMetadata(
   // We use "gz" compression by default for VABC.
   if (metadata->vabc_enabled()) {
     metadata->set_vabc_compression_param("gz");
+    metadata->set_cow_version(android::snapshot::kCowVersionManifest);
   }
   dynamic_partition_metadata = std::move(metadata);
   return true;
diff --git a/update_metadata.proto b/update_metadata.proto
index bc9e34ac..93e4e2e1 100644
--- a/update_metadata.proto
+++ b/update_metadata.proto
@@ -359,6 +359,10 @@ message DynamicPartitionMetadata {
   // See system/core/fs_mgr/libsnapshot/cow_writer.cpp for available options,
   // as this parameter is ultimated forwarded to libsnapshot's CowWriter
   optional string vabc_compression_param = 4;
+
+  // COW version used by VABC. The represents the major version in the COW
+  // header
+  optional uint32 cow_version = 5;
 }
 
 // Definition has been duplicated from