libsnapshot: Fix checks for compression to work with new snapuserd.
Most checks for compression are used as a gate to decide whether dm-user
is enabled. However, dm-user can now be enabled without compression, so
these checks have to be audited and adjusted.
Once the "old" compression path is removed, these checks will become
simpler.
Bug: 208944665
Test: vts_libsnapshot_test -force_config vab
Change-Id: I2afd644464935c965d1b84205ef54ca605d32d78
diff --git a/fs_mgr/libsnapshot/partition_cow_creator.cpp b/fs_mgr/libsnapshot/partition_cow_creator.cpp
index 5569da0..5fcbdfe 100644
--- a/fs_mgr/libsnapshot/partition_cow_creator.cpp
+++ b/fs_mgr/libsnapshot/partition_cow_creator.cpp
@@ -143,7 +143,7 @@
}
std::optional<uint64_t> PartitionCowCreator::GetCowSize() {
- if (compression_enabled) {
+ if (compression_enabled || userspace_snapshots_enabled) {
if (update == nullptr || !update->has_estimate_cow_size()) {
LOG(ERROR) << "Update manifest does not include a COW size";
return std::nullopt;
diff --git a/fs_mgr/libsnapshot/partition_cow_creator.h b/fs_mgr/libsnapshot/partition_cow_creator.h
index 34b39ca..1f34177 100644
--- a/fs_mgr/libsnapshot/partition_cow_creator.h
+++ b/fs_mgr/libsnapshot/partition_cow_creator.h
@@ -58,6 +58,7 @@
std::vector<ChromeOSExtent> extra_extents = {};
// True if compression is enabled.
bool compression_enabled = false;
+ bool userspace_snapshots_enabled = false;
std::string compression_algorithm;
struct Return {
diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp
index e6e17bd..dfa467e 100644
--- a/fs_mgr/libsnapshot/snapshot.cpp
+++ b/fs_mgr/libsnapshot/snapshot.cpp
@@ -1329,7 +1329,7 @@
const SnapshotStatus& status) {
CHECK(lock);
- if (!status.compression_enabled()) {
+ if (!status.compression_enabled() && !UpdateUsesUserSnapshots(lock)) {
// Do not try to verify old-style COWs yet.
return MergeFailureCode::Ok;
}
@@ -2345,13 +2345,15 @@
remaining_time = GetRemainingTime(params.timeout_ms, begin);
if (remaining_time.count() < 0) return false;
- if (context == SnapshotContext::Update && live_snapshot_status->compression_enabled()) {
- // Stop here, we can't run dm-user yet, the COW isn't built.
- created_devices.Release();
- return true;
+ if (context == SnapshotContext::Update) {
+ if (UpdateUsesUserSnapshots(lock) || live_snapshot_status->compression_enabled()) {
+ // Stop here, we can't run dm-user yet, the COW isn't built.
+ created_devices.Release();
+ return true;
+ }
}
- if (live_snapshot_status->compression_enabled()) {
+ if (UpdateUsesUserSnapshots(lock) || live_snapshot_status->compression_enabled()) {
// Get the source device (eg the view of the partition from before it was resized).
std::string source_device_path;
if (live_snapshot_status->old_partition_size() > 0) {
@@ -3132,6 +3134,61 @@
.compression_algorithm = compression_algorithm,
};
+ if (!device()->IsTestDevice()) {
+ cow_creator.userspace_snapshots_enabled = IsUserspaceSnapshotsEnabled();
+ if (cow_creator.userspace_snapshots_enabled) {
+ LOG(INFO) << "User-space snapshots enabled";
+ } else {
+ LOG(INFO) << "User-space snapshots disabled";
+ }
+
+ // Terminate stale daemon if any
+ std::unique_ptr<SnapuserdClient> snapuserd_client =
+ SnapuserdClient::Connect(kSnapuserdSocket, 10s);
+ if (snapuserd_client) {
+ snapuserd_client->DetachSnapuserd();
+ snapuserd_client->CloseConnection();
+ snapuserd_client = nullptr;
+ }
+
+ // Clear the cached client if any
+ if (snapuserd_client_) {
+ snapuserd_client_->CloseConnection();
+ snapuserd_client_ = nullptr;
+ }
+ } else {
+ cow_creator.userspace_snapshots_enabled = !IsDmSnapshotTestingEnabled();
+ if (cow_creator.userspace_snapshots_enabled) {
+ LOG(INFO) << "User-space snapshots disabled for testing";
+ } else {
+ LOG(INFO) << "User-space snapshots enabled for testing";
+ }
+ }
+
+ is_snapshot_userspace_ = cow_creator.userspace_snapshots_enabled;
+
+ // If compression is enabled, we need to retain a copy of the old metadata
+ // so we can access original blocks in case they are moved around. We do
+ // not want to rely on the old super metadata slot because we don't
+ // guarantee its validity after the slot switch is successful.
+ //
+ // Note that we do this for userspace merges even if compression is
+ // disabled, since the code path expects it even if the source device will
+ // be unused.
+ if (cow_creator.compression_enabled || cow_creator.userspace_snapshots_enabled) {
+ auto metadata = current_metadata->Export();
+ if (!metadata) {
+ LOG(ERROR) << "Could not export current metadata";
+ return Return::Error();
+ }
+
+ auto path = GetOldPartitionMetadataPath();
+ if (!android::fs_mgr::WriteToImageFile(path, *metadata.get())) {
+ LOG(ERROR) << "Cannot write old metadata to " << path;
+ return Return::Error();
+ }
+ }
+
auto ret = CreateUpdateSnapshotsInternal(lock.get(), manifest, &cow_creator, &created_devices,
&all_snapshot_status);
if (!ret.is_ok()) return ret;
@@ -3153,64 +3210,11 @@
return Return::Error();
}
- // If compression is enabled, we need to retain a copy of the old metadata
- // so we can access original blocks in case they are moved around. We do
- // not want to rely on the old super metadata slot because we don't
- // guarantee its validity after the slot switch is successful.
- if (cow_creator.compression_enabled) {
- auto metadata = current_metadata->Export();
- if (!metadata) {
- LOG(ERROR) << "Could not export current metadata";
- return Return::Error();
- }
-
- auto path = GetOldPartitionMetadataPath();
- if (!android::fs_mgr::WriteToImageFile(path, *metadata.get())) {
- LOG(ERROR) << "Cannot write old metadata to " << path;
- return Return::Error();
- }
- }
-
SnapshotUpdateStatus status = ReadSnapshotUpdateStatus(lock.get());
status.set_state(update_state);
status.set_compression_enabled(cow_creator.compression_enabled);
- if (cow_creator.compression_enabled) {
- if (!device()->IsTestDevice()) {
- // Userspace snapshots is enabled only if compression is enabled
- status.set_userspace_snapshots(IsUserspaceSnapshotsEnabled());
- if (IsUserspaceSnapshotsEnabled()) {
- is_snapshot_userspace_ = true;
- LOG(INFO) << "User-space snapshots enabled";
- } else {
- is_snapshot_userspace_ = false;
- LOG(INFO) << "User-space snapshots disabled";
- }
+ status.set_userspace_snapshots(cow_creator.userspace_snapshots_enabled);
- // Terminate stale daemon if any
- std::unique_ptr<SnapuserdClient> snapuserd_client =
- SnapuserdClient::Connect(kSnapuserdSocket, 10s);
- if (snapuserd_client) {
- snapuserd_client->DetachSnapuserd();
- snapuserd_client->CloseConnection();
- snapuserd_client = nullptr;
- }
-
- // Clear the cached client if any
- if (snapuserd_client_) {
- snapuserd_client_->CloseConnection();
- snapuserd_client_ = nullptr;
- }
- } else {
- status.set_userspace_snapshots(!IsDmSnapshotTestingEnabled());
- if (IsDmSnapshotTestingEnabled()) {
- is_snapshot_userspace_ = false;
- LOG(INFO) << "User-space snapshots disabled for testing";
- } else {
- is_snapshot_userspace_ = true;
- LOG(INFO) << "User-space snapshots enabled for testing";
- }
- }
- }
if (!WriteSnapshotUpdateStatus(lock.get(), status)) {
LOG(ERROR) << "Unable to write new update state";
return Return::Error();
@@ -3374,6 +3378,8 @@
const std::map<std::string, SnapshotStatus>& all_snapshot_status) {
CHECK(lock);
+ bool userspace_merges = UpdateUsesUserSnapshots(lock);
+
CreateLogicalPartitionParams cow_params{
.block_device = LP_METADATA_DEFAULT_PARTITION_NAME,
.metadata = exported_target_metadata,
@@ -3403,7 +3409,7 @@
return Return::Error();
}
- if (it->second.compression_enabled()) {
+ if (userspace_merges || it->second.compression_enabled()) {
unique_fd fd(open(cow_path.c_str(), O_RDWR | O_CLOEXEC));
if (fd < 0) {
PLOG(ERROR) << "open " << cow_path << " failed for snapshot "
@@ -3449,8 +3455,8 @@
if (!ReadSnapshotStatus(lock.get(), params.GetPartitionName(), &status)) {
return false;
}
- if (status.compression_enabled()) {
- LOG(ERROR) << "Cannot use MapUpdateSnapshot with compressed snapshots";
+ if (status.compression_enabled() || UpdateUsesUserSnapshots(lock.get())) {
+ LOG(ERROR) << "Cannot use MapUpdateSnapshot with user snapshots";
return false;
}
@@ -3507,7 +3513,7 @@
return nullptr;
}
- if (status.compression_enabled()) {
+ if (status.compression_enabled() || UpdateUsesUserSnapshots(lock.get())) {
return OpenCompressedSnapshotWriter(lock.get(), source_device, params.GetPartitionName(),
status, paths);
}
@@ -3647,6 +3653,7 @@
<< (access(GetForwardMergeIndicatorPath().c_str(), F_OK) == 0 ? "exists" : strerror(errno))
<< std::endl;
ss << "Source build fingerprint: " << update_status.source_build_fingerprint() << std::endl;
+ ss << "Using userspace snapshots: " << UpdateUsesUserSnapshots(file.get()) << std::endl;
bool ok = true;
std::vector<std::string> snapshots;
@@ -3847,6 +3854,10 @@
bool SnapshotManager::EnsureNoOverflowSnapshot(LockedFile* lock) {
CHECK(lock);
+ if (UpdateUsesUserSnapshots(lock)) {
+ return true;
+ }
+
std::vector<std::string> snapshots;
if (!ListSnapshots(lock, &snapshots)) {
LOG(ERROR) << "Could not list snapshots.";
@@ -3859,6 +3870,7 @@
return false;
}
if (status.compression_enabled()) {
+ // Compressed snapshots are never written through dm-snapshot.
continue;
}
@@ -4022,7 +4034,10 @@
if (!lock) return false;
auto status = ReadSnapshotUpdateStatus(lock.get());
- return status.state() != UpdateState::None && status.compression_enabled();
+ if (status.state() != UpdateState::None && status.compression_enabled()) {
+ return true;
+ }
+ return UpdateUsesUserSnapshots(lock.get());
}
bool SnapshotManager::DetachSnapuserdForSelinux(std::vector<std::string>* snapuserd_argv) {
@@ -4048,6 +4063,9 @@
}
MergePhase SnapshotManager::DecideMergePhase(const SnapshotStatus& status) {
+ // Note: disabling compression disables move operations, so we don't need
+ // separate phases when compression is disabled (irrespective of userspace
+ // merges).
if (status.compression_enabled() && status.device_size() < status.old_partition_size()) {
return MergePhase::FIRST_PHASE;
}
diff --git a/fs_mgr/libsnapshot/snapshot_test.cpp b/fs_mgr/libsnapshot/snapshot_test.cpp
index 14f2d45..c70b353 100644
--- a/fs_mgr/libsnapshot/snapshot_test.cpp
+++ b/fs_mgr/libsnapshot/snapshot_test.cpp
@@ -357,7 +357,7 @@
DeltaArchiveManifest manifest;
auto dynamic_partition_metadata = manifest.mutable_dynamic_partition_metadata();
- dynamic_partition_metadata->set_vabc_enabled(IsCompressionEnabled());
+ dynamic_partition_metadata->set_vabc_enabled(ShouldUseCompression());
dynamic_partition_metadata->set_cow_version(android::snapshot::kCowVersionMajor);
auto group = dynamic_partition_metadata->add_groups();
@@ -396,7 +396,7 @@
if (!res) {
return res;
}
- } else if (!IsCompressionEnabled()) {
+ } else if (!ShouldUseUserspaceSnapshots()) {
std::string ignore;
if (!MapUpdateSnapshot("test_partition_b", &ignore)) {
return AssertionFailure() << "Failed to map test_partition_b";
@@ -1030,7 +1030,7 @@
}
AssertionResult MapOneUpdateSnapshot(const std::string& name) {
- if (ShouldUseCompression()) {
+ if (ShouldUseUserspaceSnapshots()) {
std::unique_ptr<ISnapshotWriter> writer;
return MapUpdateSnapshot(name, &writer);
} else {
@@ -1040,7 +1040,7 @@
}
AssertionResult WriteSnapshotAndHash(const std::string& name) {
- if (ShouldUseCompression()) {
+ if (ShouldUseUserspaceSnapshots()) {
std::unique_ptr<ISnapshotWriter> writer;
auto res = MapUpdateSnapshot(name, &writer);
if (!res) {
@@ -2072,7 +2072,7 @@
// Test for overflow bit after update
TEST_F(SnapshotUpdateTest, Overflow) {
- if (ShouldUseCompression()) {
+ if (ShouldUseUserspaceSnapshots()) {
GTEST_SKIP() << "No overflow bit set for userspace COWs";
}