p2p: Skip some tests if xattr support is not available.
This is needed because some builders still lack xattr / fallocate
support despite being updated to Precise. Also gracefully handle lack
of fallocate(2) support since this feature is not always available
either.
BUG=chromium:298397
TEST=Remounted the filesystem holding my chroot with nouser_xattr
(e.g. 'mount -oremount,nouser_xattr /ssd') and unit tests pass with
the newly added warning.
Change-Id: I0fe53d762fad778a3d68162ddcd8bc74557fba6b
Reviewed-on: https://chromium-review.googlesource.com/170782
Reviewed-by: Alex Deymo <deymo@chromium.org>
Tested-by: David Zeuthen <zeuthen@chromium.org>
Commit-Queue: David Zeuthen <zeuthen@chromium.org>
diff --git a/download_action_unittest.cc b/download_action_unittest.cc
index 432cb16..be9d170 100644
--- a/download_action_unittest.cc
+++ b/download_action_unittest.cc
@@ -547,6 +547,12 @@
};
TEST_F(P2PDownloadActionTest, IsWrittenTo) {
+ if (!utils::IsXAttrSupported(FilePath("/tmp"))) {
+ LOG(WARNING) << "Skipping test because /tmp does not support xattr. "
+ << "Please update your system to support this feature.";
+ return;
+ }
+
SetupDownload(0); // starting_offset
StartDownload(true); // use_p2p_to_share
@@ -562,6 +568,12 @@
}
TEST_F(P2PDownloadActionTest, DeleteIfHoleExists) {
+ if (!utils::IsXAttrSupported(FilePath("/tmp"))) {
+ LOG(WARNING) << "Skipping test because /tmp does not support xattr. "
+ << "Please update your system to support this feature.";
+ return;
+ }
+
SetupDownload(1000); // starting_offset
StartDownload(true); // use_p2p_to_share
@@ -572,6 +584,12 @@
}
TEST_F(P2PDownloadActionTest, CanAppend) {
+ if (!utils::IsXAttrSupported(FilePath("/tmp"))) {
+ LOG(WARNING) << "Skipping test because /tmp does not support xattr. "
+ << "Please update your system to support this feature.";
+ return;
+ }
+
SetupDownload(1000); // starting_offset
// Prepare the file with existing data before starting to write to
@@ -601,6 +619,12 @@
}
TEST_F(P2PDownloadActionTest, DeletePartialP2PFileIfResumingWithoutP2P) {
+ if (!utils::IsXAttrSupported(FilePath("/tmp"))) {
+ LOG(WARNING) << "Skipping test because /tmp does not support xattr. "
+ << "Please update your system to support this feature.";
+ return;
+ }
+
SetupDownload(1000); // starting_offset
// Prepare the file with all existing data before starting to write
diff --git a/p2p_manager.cc b/p2p_manager.cc
index 7536fc6..596adf1 100644
--- a/p2p_manager.cc
+++ b/p2p_manager.cc
@@ -606,18 +606,21 @@
FALLOC_FL_KEEP_SIZE, // Keep file size as 0.
0,
expected_size) != 0) {
- // ENOSPC can happen (funky race though, cf. the statvfs() check
- // above), handle it gracefully, e.g. use logging level INFO.
- //
- // NOTE: we *could* handle ENOSYS gracefully (ie. we could
- // ignore it) but currently we don't because running out of
- // space later sounds absolutely horrible. Better to fail fast.
- PLOG(INFO) << "Error allocating " << expected_size
- << " bytes for file " << path.value();
- if (unlink(path.value().c_str()) != 0) {
- PLOG(ERROR) << "Error deleting file with path " << path.value();
+ if (errno == ENOSYS || errno == EOPNOTSUPP) {
+ // If the filesystem doesn't support the fallocate, keep
+ // going. This is helpful when running unit tests on build
+ // machines with ancient filesystems and/or OSes.
+ PLOG(WARNING) << "Ignoring fallocate(2) failure";
+ } else {
+ // ENOSPC can happen (funky race though, cf. the statvfs() check
+ // above), handle it gracefully, e.g. use logging level INFO.
+ PLOG(INFO) << "Error allocating " << expected_size
+ << " bytes for file " << path.value();
+ if (unlink(path.value().c_str()) != 0) {
+ PLOG(ERROR) << "Error deleting file with path " << path.value();
+ }
+ return false;
}
- return false;
}
string decimal_size = StringPrintf("%zu", expected_size);
diff --git a/p2p_manager_unittest.cc b/p2p_manager_unittest.cc
index ce8d644..bee0248 100644
--- a/p2p_manager_unittest.cc
+++ b/p2p_manager_unittest.cc
@@ -287,6 +287,12 @@
// Check that sharing a *new* file works.
TEST_F(P2PManagerTest, ShareFile) {
+ if (!utils::IsXAttrSupported(FilePath("/tmp"))) {
+ LOG(WARNING) << "Skipping test because /tmp does not support xattr. "
+ << "Please update your system to support this feature.";
+ return;
+ }
+
scoped_ptr<P2PManager> manager(P2PManager::Construct(test_conf_,
NULL, "cros_au", 3));
EXPECT_TRUE(manager->FileShare("foo", 10 * 1000 * 1000));
@@ -304,6 +310,12 @@
// Check that making a shared file visible, does what is expected.
TEST_F(P2PManagerTest, MakeFileVisible) {
+ if (!utils::IsXAttrSupported(FilePath("/tmp"))) {
+ LOG(WARNING) << "Skipping test because /tmp does not support xattr. "
+ << "Please update your system to support this feature.";
+ return;
+ }
+
scoped_ptr<P2PManager> manager(P2PManager::Construct(test_conf_,
NULL, "cros_au", 3));
// First, check that it's not visible.
@@ -325,6 +337,12 @@
// Check that we return the right values for existing files in P2P_DIR.
TEST_F(P2PManagerTest, ExistingFiles) {
+ if (!utils::IsXAttrSupported(FilePath("/tmp"))) {
+ LOG(WARNING) << "Skipping test because /tmp does not support xattr. "
+ << "Please update your system to support this feature.";
+ return;
+ }
+
scoped_ptr<P2PManager> manager(P2PManager::Construct(test_conf_,
NULL, "cros_au", 3));
bool visible;
diff --git a/utils.cc b/utils.cc
index cd7f384..c5d3e05 100644
--- a/utils.cc
+++ b/utils.cc
@@ -4,6 +4,7 @@
#include "update_engine/utils.h"
+#include <attr/xattr.h>
#include <sys/mount.h>
#include <sys/resource.h>
#include <sys/stat.h>
@@ -1067,6 +1068,36 @@
encoded_hash.c_str());
}
+bool IsXAttrSupported(const base::FilePath& dir_path) {
+ char *path = strdup(dir_path.Append("xattr_test_XXXXXX").value().c_str());
+
+ int fd = mkstemp(path);
+ if (fd == -1) {
+ PLOG(ERROR) << "Error creating temporary file in " << dir_path.value();
+ free(path);
+ return false;
+ }
+
+ if (unlink(path) != 0) {
+ PLOG(ERROR) << "Error unlinking temporary file " << path;
+ close(fd);
+ free(path);
+ return false;
+ }
+
+ int xattr_res = fsetxattr(fd, "user.xattr-test", "value", strlen("value"), 0);
+ if (xattr_res != 0) {
+ if (errno == ENOTSUP) {
+ // Leave it to call-sites to warn about non-support.
+ } else {
+ PLOG(ERROR) << "Error setting xattr on " << path;
+ }
+ }
+ close(fd);
+ free(path);
+ return xattr_res == 0;
+}
+
} // namespace utils
} // namespace chromeos_update_engine
diff --git a/utils.h b/utils.h
index 2dd8095..6f6539f 100644
--- a/utils.h
+++ b/utils.h
@@ -12,6 +12,7 @@
#include <unistd.h>
#include <vector>
+#include <base/file_path.h>
#include <base/posix/eintr_wrapper.h>
#include <base/time.h>
#include <ext2fs/ext2fs.h>
@@ -341,6 +342,11 @@
// http://www.chromium.org/chromium-os/chromiumos-design-docs/disk-format
bool GetInstallDev(const std::string& boot_dev, std::string* install_dev);
+// Checks if xattr is supported in the directory specified by
+// |dir_path| which must be writable. Returns true if the feature is
+// supported, false if not or if an error occured.
+bool IsXAttrSupported(const base::FilePath& dir_path);
+
} // namespace utils