diff options
Diffstat (limited to 'test')
-rw-r--r-- | test/syscalls/linux/BUILD | 2 | ||||
-rw-r--r-- | test/syscalls/linux/mknod.cc | 30 | ||||
-rw-r--r-- | test/syscalls/linux/mount.cc | 38 | ||||
-rw-r--r-- | test/syscalls/linux/proc_pid_smaps.cc | 2 | ||||
-rw-r--r-- | test/syscalls/linux/stat.cc | 34 | ||||
-rw-r--r-- | test/util/BUILD | 4 | ||||
-rw-r--r-- | test/util/posix_error.cc | 2 | ||||
-rw-r--r-- | test/util/posix_error.h | 34 | ||||
-rw-r--r-- | test/util/save_util.cc | 52 | ||||
-rw-r--r-- | test/util/save_util.h | 20 | ||||
-rw-r--r-- | test/util/save_util_linux.cc | 22 | ||||
-rw-r--r-- | test/util/save_util_other.cc | 8 |
12 files changed, 151 insertions, 97 deletions
diff --git a/test/syscalls/linux/BUILD b/test/syscalls/linux/BUILD index 572f39a5d..42fa18683 100644 --- a/test/syscalls/linux/BUILD +++ b/test/syscalls/linux/BUILD @@ -1285,6 +1285,7 @@ cc_binary( "//test/util:mount_util", "//test/util:multiprocess_util", "//test/util:posix_error", + "//test/util:save_util", "//test/util:temp_path", "//test/util:test_main", "//test/util:test_util", @@ -3441,6 +3442,7 @@ cc_binary( "@com_google_absl//absl/strings", gtest, "//test/util:posix_error", + "//test/util:save_util", "//test/util:temp_path", "//test/util:test_main", "//test/util:test_util", diff --git a/test/syscalls/linux/mknod.cc b/test/syscalls/linux/mknod.cc index b96907b30..1635c6d0c 100644 --- a/test/syscalls/linux/mknod.cc +++ b/test/syscalls/linux/mknod.cc @@ -125,6 +125,16 @@ TEST(MknodTest, Socket) { ASSERT_THAT(unlink(filename.c_str()), SyscallSucceeds()); } +PosixErrorOr<FileDescriptor> OpenRetryEINTR(std::string const& path, int flags, + mode_t mode = 0) { + while (true) { + auto maybe_fd = Open(path, flags, mode); + if (maybe_fd.ok() || maybe_fd.error().errno_value() != EINTR) { + return maybe_fd; + } + } +} + TEST(MknodTest, Fifo) { const std::string fifo = NewTempAbsPath(); ASSERT_THAT(mknod(fifo.c_str(), S_IFIFO | S_IRUSR | S_IWUSR, 0), @@ -139,14 +149,16 @@ TEST(MknodTest, Fifo) { // Read-end of the pipe. ScopedThread t([&fifo, &buf, &msg]() { - FileDescriptor fd = ASSERT_NO_ERRNO_AND_VALUE(Open(fifo.c_str(), O_RDONLY)); + FileDescriptor fd = + ASSERT_NO_ERRNO_AND_VALUE(OpenRetryEINTR(fifo.c_str(), O_RDONLY)); EXPECT_THAT(ReadFd(fd.get(), buf.data(), buf.size()), SyscallSucceedsWithValue(msg.length())); EXPECT_EQ(msg, std::string(buf.data())); }); // Write-end of the pipe. - FileDescriptor wfd = ASSERT_NO_ERRNO_AND_VALUE(Open(fifo.c_str(), O_WRONLY)); + FileDescriptor wfd = + ASSERT_NO_ERRNO_AND_VALUE(OpenRetryEINTR(fifo.c_str(), O_WRONLY)); EXPECT_THAT(WriteFd(wfd.get(), msg.c_str(), msg.length()), SyscallSucceedsWithValue(msg.length())); } @@ -164,15 +176,16 @@ TEST(MknodTest, FifoOtrunc) { std::vector<char> buf(512); // Read-end of the pipe. ScopedThread t([&fifo, &buf, &msg]() { - FileDescriptor fd = ASSERT_NO_ERRNO_AND_VALUE(Open(fifo.c_str(), O_RDONLY)); + FileDescriptor fd = + ASSERT_NO_ERRNO_AND_VALUE(OpenRetryEINTR(fifo.c_str(), O_RDONLY)); EXPECT_THAT(ReadFd(fd.get(), buf.data(), buf.size()), SyscallSucceedsWithValue(msg.length())); EXPECT_EQ(msg, std::string(buf.data())); }); // Write-end of the pipe. - FileDescriptor wfd = - ASSERT_NO_ERRNO_AND_VALUE(Open(fifo.c_str(), O_WRONLY | O_TRUNC)); + FileDescriptor wfd = ASSERT_NO_ERRNO_AND_VALUE( + OpenRetryEINTR(fifo.c_str(), O_WRONLY | O_TRUNC)); EXPECT_THAT(WriteFd(wfd.get(), msg.c_str(), msg.length()), SyscallSucceedsWithValue(msg.length())); } @@ -192,14 +205,15 @@ TEST(MknodTest, FifoTruncNoOp) { std::vector<char> buf(512); // Read-end of the pipe. ScopedThread t([&fifo, &buf, &msg]() { - FileDescriptor fd = ASSERT_NO_ERRNO_AND_VALUE(Open(fifo.c_str(), O_RDONLY)); + FileDescriptor fd = + ASSERT_NO_ERRNO_AND_VALUE(OpenRetryEINTR(fifo.c_str(), O_RDONLY)); EXPECT_THAT(ReadFd(fd.get(), buf.data(), buf.size()), SyscallSucceedsWithValue(msg.length())); EXPECT_EQ(msg, std::string(buf.data())); }); - FileDescriptor wfd = - ASSERT_NO_ERRNO_AND_VALUE(Open(fifo.c_str(), O_WRONLY | O_TRUNC)); + FileDescriptor wfd = ASSERT_NO_ERRNO_AND_VALUE( + OpenRetryEINTR(fifo.c_str(), O_WRONLY | O_TRUNC)); EXPECT_THAT(ftruncate(wfd.get(), 0), SyscallFailsWithErrno(EINVAL)); EXPECT_THAT(WriteFd(wfd.get(), msg.c_str(), msg.length()), SyscallSucceedsWithValue(msg.length())); diff --git a/test/syscalls/linux/mount.cc b/test/syscalls/linux/mount.cc index 3aab25b23..d65b7d031 100644 --- a/test/syscalls/linux/mount.cc +++ b/test/syscalls/linux/mount.cc @@ -34,6 +34,7 @@ #include "test/util/mount_util.h" #include "test/util/multiprocess_util.h" #include "test/util/posix_error.h" +#include "test/util/save_util.h" #include "test/util/temp_path.h" #include "test/util/test_util.h" #include "test/util/thread_util.h" @@ -131,7 +132,9 @@ TEST(MountTest, UmountDetach) { ASSERT_NO_ERRNO_AND_VALUE(Mount("", dir.path(), "tmpfs", 0, "mode=0700", /* umountflags= */ MNT_DETACH)); const struct stat after = ASSERT_NO_ERRNO_AND_VALUE(Stat(dir.path())); - EXPECT_NE(before.st_ino, after.st_ino); + EXPECT_FALSE(before.st_dev == after.st_dev && before.st_ino == after.st_ino) + << "mount point has device number " << before.st_dev + << " and inode number " << before.st_ino << " before and after mount"; // Create files in the new mount. constexpr char kContents[] = "no no no"; @@ -147,12 +150,14 @@ TEST(MountTest, UmountDetach) { // Unmount the tmpfs. mount.Release()(); - // Only check for inode number equality if the directory is not in overlayfs. - // If xino option is not enabled and if all overlayfs layers do not belong to - // the same filesystem then "the value of st_ino for directory objects may not - // be persistent and could change even while the overlay filesystem is - // mounted." -- Documentation/filesystems/overlayfs.txt - if (!ASSERT_NO_ERRNO_AND_VALUE(IsOverlayfs(dir.path()))) { + // Inode numbers for gofer-accessed files may change across save/restore. + // + // For overlayfs, if xino option is not enabled and if all overlayfs layers do + // not belong to the same filesystem then "the value of st_ino for directory + // objects may not be persistent and could change even while the overlay + // filesystem is mounted." -- Documentation/filesystems/overlayfs.txt + if (!IsRunningWithSaveRestore() && + !ASSERT_NO_ERRNO_AND_VALUE(IsOverlayfs(dir.path()))) { const struct stat after2 = ASSERT_NO_ERRNO_AND_VALUE(Stat(dir.path())); EXPECT_EQ(before.st_ino, after2.st_ino); } @@ -214,18 +219,23 @@ TEST(MountTest, MountTmpfs) { const struct stat s = ASSERT_NO_ERRNO_AND_VALUE(Stat(dir.path())); EXPECT_EQ(s.st_mode, S_IFDIR | 0700); - EXPECT_NE(s.st_ino, before.st_ino); + EXPECT_FALSE(before.st_dev == s.st_dev && before.st_ino == s.st_ino) + << "mount point has device number " << before.st_dev + << " and inode number " << before.st_ino << " before and after mount"; EXPECT_NO_ERRNO(Open(JoinPath(dir.path(), "foo"), O_CREAT | O_RDWR, 0777)); } // Now that dir is unmounted again, we should have the old inode back. - // Only check for inode number equality if the directory is not in overlayfs. - // If xino option is not enabled and if all overlayfs layers do not belong to - // the same filesystem then "the value of st_ino for directory objects may not - // be persistent and could change even while the overlay filesystem is - // mounted." -- Documentation/filesystems/overlayfs.txt - if (!ASSERT_NO_ERRNO_AND_VALUE(IsOverlayfs(dir.path()))) { + // + // Inode numbers for gofer-accessed files may change across save/restore. + // + // For overlayfs, if xino option is not enabled and if all overlayfs layers do + // not belong to the same filesystem then "the value of st_ino for directory + // objects may not be persistent and could change even while the overlay + // filesystem is mounted." -- Documentation/filesystems/overlayfs.txt + if (!IsRunningWithSaveRestore() && + !ASSERT_NO_ERRNO_AND_VALUE(IsOverlayfs(dir.path()))) { const struct stat after = ASSERT_NO_ERRNO_AND_VALUE(Stat(dir.path())); EXPECT_EQ(before.st_ino, after.st_ino); } diff --git a/test/syscalls/linux/proc_pid_smaps.cc b/test/syscalls/linux/proc_pid_smaps.cc index 9fb1b3a2c..738923822 100644 --- a/test/syscalls/linux/proc_pid_smaps.cc +++ b/test/syscalls/linux/proc_pid_smaps.cc @@ -191,7 +191,7 @@ PosixErrorOr<std::vector<ProcPidSmapsEntry>> ParseProcPidSmaps( // amount of whitespace). if (!entry) { std::cerr << "smaps line not considered a maps line: " - << maybe_maps_entry.error_message() << std::endl; + << maybe_maps_entry.error().message() << std::endl; return PosixError( EINVAL, absl::StrCat("smaps field line without preceding maps line: ", l)); diff --git a/test/syscalls/linux/stat.cc b/test/syscalls/linux/stat.cc index 92260b1e1..6e7142a42 100644 --- a/test/syscalls/linux/stat.cc +++ b/test/syscalls/linux/stat.cc @@ -31,6 +31,7 @@ #include "test/util/cleanup.h" #include "test/util/file_descriptor.h" #include "test/util/fs_util.h" +#include "test/util/save_util.h" #include "test/util/temp_path.h" #include "test/util/test_util.h" @@ -328,7 +329,10 @@ TEST_F(StatTest, LeadingDoubleSlash) { ASSERT_THAT(lstat(double_slash_path.c_str(), &double_slash_st), SyscallSucceeds()); EXPECT_EQ(st.st_dev, double_slash_st.st_dev); - EXPECT_EQ(st.st_ino, double_slash_st.st_ino); + // Inode numbers for gofer-accessed files may change across save/restore. + if (!IsRunningWithSaveRestore()) { + EXPECT_EQ(st.st_ino, double_slash_st.st_ino); + } } // Test that a rename doesn't change the underlying file. @@ -346,8 +350,14 @@ TEST_F(StatTest, StatDoesntChangeAfterRename) { EXPECT_EQ(st_old.st_nlink, st_new.st_nlink); EXPECT_EQ(st_old.st_dev, st_new.st_dev); + // Inode numbers for gofer-accessed files on which no reference is held may + // change across save/restore because the information that the gofer client + // uses to track file identity (9P QID path) is inconsistent between gofer + // processes, which are restarted across save/restore. + // // Overlay filesystems may synthesize directory inode numbers on the fly. - if (!ASSERT_NO_ERRNO_AND_VALUE(IsOverlayfs(GetAbsoluteTestTmpdir()))) { + if (!IsRunningWithSaveRestore() && + !ASSERT_NO_ERRNO_AND_VALUE(IsOverlayfs(GetAbsoluteTestTmpdir()))) { EXPECT_EQ(st_old.st_ino, st_new.st_ino); } EXPECT_EQ(st_old.st_mode, st_new.st_mode); @@ -541,6 +551,26 @@ TEST_F(StatTest, LstatELOOPPath) { ASSERT_THAT(lstat(path.c_str(), &s), SyscallFailsWithErrno(ELOOP)); } +TEST(SimpleStatTest, DifferentFilesHaveDifferentDeviceInodeNumberPairs) { + // TODO(gvisor.dev/issue/1624): This test case fails in VFS1 save/restore + // tests because VFS1 gofer inode number assignment restarts after + // save/restore, such that the inodes for file1 and file2 (which are + // unreferenced and therefore not retained in sentry checkpoints before the + // calls to lstat()) are assigned the same inode number. + SKIP_IF(IsRunningWithVFS1() && IsRunningWithSaveRestore()); + + TempPath file1 = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile()); + TempPath file2 = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile()); + + MaybeSave(); + struct stat st1 = ASSERT_NO_ERRNO_AND_VALUE(Lstat(file1.path())); + MaybeSave(); + struct stat st2 = ASSERT_NO_ERRNO_AND_VALUE(Lstat(file2.path())); + EXPECT_FALSE(st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino) + << "both files have device number " << st1.st_dev << " and inode number " + << st1.st_ino; +} + // Ensure that inode allocation for anonymous devices work correctly across // save/restore. In particular, inode numbers should be unique across S/R. TEST(SimpleStatTest, AnonDeviceAllocatesUniqueInodesAcrossSaveRestore) { diff --git a/test/util/BUILD b/test/util/BUILD index 26c2b6a2f..1b028a477 100644 --- a/test/util/BUILD +++ b/test/util/BUILD @@ -155,6 +155,10 @@ cc_library( ], hdrs = ["save_util.h"], defines = select_system(), + deps = [ + ":logging", + "@com_google_absl//absl/types:optional", + ], ) cc_library( diff --git a/test/util/posix_error.cc b/test/util/posix_error.cc index cebf7e0ac..deed0c05b 100644 --- a/test/util/posix_error.cc +++ b/test/util/posix_error.cc @@ -87,7 +87,7 @@ bool PosixErrorIsMatcherCommonImpl::MatchAndExplain( return false; } - if (!message_matcher_.Matches(error.error_message())) { + if (!message_matcher_.Matches(error.message())) { return false; } diff --git a/test/util/posix_error.h b/test/util/posix_error.h index ad666bce0..b634a7f78 100644 --- a/test/util/posix_error.h +++ b/test/util/posix_error.h @@ -26,11 +26,6 @@ namespace gvisor { namespace testing { -class PosixErrorIsMatcherCommonImpl; - -template <typename T> -class PosixErrorOr; - class ABSL_MUST_USE_RESULT PosixError { public: PosixError() {} @@ -49,7 +44,8 @@ class ABSL_MUST_USE_RESULT PosixError { // PosixErrorOr. const PosixError& error() const { return *this; } - std::string error_message() const { return msg_; } + int errno_value() const { return errno_; } + std::string message() const { return msg_; } // ToString produces a full string representation of this posix error // including the printable representation of the errno and the error message. @@ -61,14 +57,8 @@ class ABSL_MUST_USE_RESULT PosixError { void IgnoreError() const {} private: - int errno_value() const { return errno_; } int errno_ = 0; std::string msg_; - - friend class PosixErrorIsMatcherCommonImpl; - - template <typename T> - friend class PosixErrorOr; }; template <typename T> @@ -94,15 +84,12 @@ class ABSL_MUST_USE_RESULT PosixErrorOr { template <typename U> PosixErrorOr& operator=(PosixErrorOr<U> other); - // Return a reference to the error or NoError(). - PosixError error() const; - - // Returns this->error().error_message(); - std::string error_message() const; - // Returns true if this PosixErrorOr contains some T. bool ok() const; + // Return a copy of the contained PosixError or NoError(). + PosixError error() const; + // Returns a reference to our current value, or CHECK-fails if !this->ok(). const T& ValueOrDie() const&; T& ValueOrDie() &; @@ -115,7 +102,6 @@ class ABSL_MUST_USE_RESULT PosixErrorOr { void IgnoreError() const {} private: - int errno_value() const; absl::variant<T, PosixError> value_; friend class PosixErrorIsMatcherCommonImpl; @@ -171,16 +157,6 @@ PosixError PosixErrorOr<T>::error() const { } template <typename T> -int PosixErrorOr<T>::errno_value() const { - return error().errno_value(); -} - -template <typename T> -std::string PosixErrorOr<T>::error_message() const { - return error().error_message(); -} - -template <typename T> bool PosixErrorOr<T>::ok() const { return absl::holds_alternative<T>(value_); } diff --git a/test/util/save_util.cc b/test/util/save_util.cc index 384d626f0..59d47e06e 100644 --- a/test/util/save_util.cc +++ b/test/util/save_util.cc @@ -21,35 +21,47 @@ #include <atomic> #include <cerrno> -#define GVISOR_COOPERATIVE_SAVE_TEST "GVISOR_COOPERATIVE_SAVE_TEST" +#include "absl/types/optional.h" namespace gvisor { namespace testing { namespace { -enum class CooperativeSaveMode { - kUnknown = 0, // cooperative_save_mode is statically-initialized to 0 - kAvailable, - kNotAvailable, -}; - -std::atomic<CooperativeSaveMode> cooperative_save_mode; - -bool CooperativeSaveEnabled() { - auto mode = cooperative_save_mode.load(); - if (mode == CooperativeSaveMode::kUnknown) { - mode = (getenv(GVISOR_COOPERATIVE_SAVE_TEST) != nullptr) - ? CooperativeSaveMode::kAvailable - : CooperativeSaveMode::kNotAvailable; - cooperative_save_mode.store(mode); +std::atomic<absl::optional<bool>> cooperative_save_present; +std::atomic<absl::optional<bool>> random_save_present; + +bool CooperativeSavePresent() { + auto present = cooperative_save_present.load(); + if (!present.has_value()) { + present = getenv("GVISOR_COOPERATIVE_SAVE_TEST") != nullptr; + cooperative_save_present.store(present); + } + return present.value(); +} + +bool RandomSavePresent() { + auto present = random_save_present.load(); + if (!present.has_value()) { + present = getenv("GVISOR_RANDOM_SAVE_TEST") != nullptr; + random_save_present.store(present); } - return mode == CooperativeSaveMode::kAvailable; + return present.value(); } std::atomic<int> save_disable; } // namespace +bool IsRunningWithSaveRestore() { + return CooperativeSavePresent() || RandomSavePresent(); +} + +void MaybeSave() { + if (CooperativeSavePresent() && save_disable.load() == 0) { + internal::DoCooperativeSave(); + } +} + DisableSave::DisableSave() { save_disable++; } DisableSave::~DisableSave() { reset(); } @@ -61,11 +73,5 @@ void DisableSave::reset() { } } -namespace internal { -bool ShouldSave() { - return CooperativeSaveEnabled() && (save_disable.load() == 0); -} -} // namespace internal - } // namespace testing } // namespace gvisor diff --git a/test/util/save_util.h b/test/util/save_util.h index bddad6120..e7218ae88 100644 --- a/test/util/save_util.h +++ b/test/util/save_util.h @@ -17,9 +17,17 @@ namespace gvisor { namespace testing { -// Disable save prevents saving while the given function executes. + +// Returns true if the environment in which the calling process is executing +// allows the test to be checkpointed and restored during execution. +bool IsRunningWithSaveRestore(); + +// May perform a co-operative save cycle. // -// This lasts the duration of the object, unless reset is called. +// errno is guaranteed to be preserved. +void MaybeSave(); + +// Causes MaybeSave to become a no-op until destroyed or reset. class DisableSave { public: DisableSave(); @@ -37,13 +45,13 @@ class DisableSave { bool reset_ = false; }; -// May perform a co-operative save cycle. +namespace internal { + +// Causes a co-operative save cycle to occur. // // errno is guaranteed to be preserved. -void MaybeSave(); +void DoCooperativeSave(); -namespace internal { -bool ShouldSave(); } // namespace internal } // namespace testing diff --git a/test/util/save_util_linux.cc b/test/util/save_util_linux.cc index fbac94912..57431b3ea 100644 --- a/test/util/save_util_linux.cc +++ b/test/util/save_util_linux.cc @@ -30,19 +30,19 @@ namespace gvisor { namespace testing { - -void MaybeSave() { - if (internal::ShouldSave()) { - int orig_errno = errno; - // We use it to trigger saving the sentry state - // when this syscall is called. - // Notice: this needs to be a valid syscall - // that is not used in any of the syscall tests. - syscall(SYS_TRIGGER_SAVE, nullptr, 0); - errno = orig_errno; - } +namespace internal { + +void DoCooperativeSave() { + int orig_errno = errno; + // We use it to trigger saving the sentry state + // when this syscall is called. + // Notice: this needs to be a valid syscall + // that is not used in any of the syscall tests. + syscall(SYS_TRIGGER_SAVE, nullptr, 0); + errno = orig_errno; } +} // namespace internal } // namespace testing } // namespace gvisor diff --git a/test/util/save_util_other.cc b/test/util/save_util_other.cc index 931af2c29..7749ded76 100644 --- a/test/util/save_util_other.cc +++ b/test/util/save_util_other.cc @@ -14,13 +14,17 @@ #ifndef __linux__ +#include "test/util/logging.h" + namespace gvisor { namespace testing { +namespace internal { -void MaybeSave() { - // Saving is never available in a non-linux environment. +void DoCooperativeSave() { + TEST_CHECK_MSG(false, "DoCooperativeSave not implemented"); } +} // namespace internal } // namespace testing } // namespace gvisor |