diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h index ce979cf20..062e00b67 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h @@ -170,9 +170,8 @@ class SnapshotManager final { bool MapSnapshot(LockedFile* lock, const std::string& name, const std::string& base_device, const std::chrono::milliseconds& timeout_ms, std::string* dev_path); - // Remove the backing copy-on-write image for the named snapshot. If the - // device is still mapped, this will attempt an Unmap, and fail if the - // unmap fails. + // Remove the backing copy-on-write image for the named snapshot. The + // caller is responsible for ensuring that the snapshot is unmapped. bool DeleteSnapshot(LockedFile* lock, const std::string& name); // Unmap a snapshot device previously mapped with MapSnapshotDevice(). diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp index c982dd10a..ef561797f 100644 --- a/fs_mgr/libsnapshot/snapshot.cpp +++ b/fs_mgr/libsnapshot/snapshot.cpp @@ -300,11 +300,6 @@ bool SnapshotManager::DeleteSnapshot(LockedFile* lock, const std::string& name) CHECK(lock); if (!EnsureImageManager()) return false; - if (!UnmapSnapshot(lock, name)) { - LOG(ERROR) << "Snapshot could not be unmapped for deletion: " << name; - return false; - } - // Take the snapshot's lock after Unmap, since it will also try to lock. auto status_file = OpenSnapshotStatusFile(name, O_RDONLY, LOCK_EX); if (!status_file) return false; diff --git a/fs_mgr/libsnapshot/snapshot_test.cpp b/fs_mgr/libsnapshot/snapshot_test.cpp index 1b3289b4d..9cc9bd7d1 100644 --- a/fs_mgr/libsnapshot/snapshot_test.cpp +++ b/fs_mgr/libsnapshot/snapshot_test.cpp @@ -104,7 +104,7 @@ TEST_F(SnapshotTest, CreateSnapshot) { ASSERT_EQ(snapshots.size(), 1); ASSERT_EQ(snapshots[0], "test-snapshot"); - // Scope so delete can re-acquire the status file lock. + // Scope so delete can re-acquire the snapshot file lock. { auto file = sm->OpenSnapshotStatusFile("test-snapshot", O_RDONLY, LOCK_SH); ASSERT_NE(file, nullptr); @@ -116,6 +116,7 @@ TEST_F(SnapshotTest, CreateSnapshot) { ASSERT_EQ(status.snapshot_size, kDeviceSize); } + ASSERT_TRUE(sm->UnmapSnapshot(lock_.get(), "test-snapshot")); ASSERT_TRUE(sm->DeleteSnapshot(lock_.get(), "test-snapshot")); }