From f6034544c05307bdea277401863ce95bc1e80ef6 Mon Sep 17 00:00:00 2001 From: Alessio Balsini Date: Wed, 5 Feb 2020 18:15:31 +0000 Subject: [PATCH 1/2] fiemap: fix fiemap size and log message when last extent Fix the total fiemap struct plus extents size allocation. Fix also the logging in IsLastExtent, that was printing error messages every time. Bug: none Test: fiemap_*_test Change-Id: I85c6af63ba3a75b993a5e8ad7d7170dc7af59582 Signed-off-by: Alessio Balsini --- fs_mgr/libfiemap/fiemap_writer.cpp | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/fs_mgr/libfiemap/fiemap_writer.cpp b/fs_mgr/libfiemap/fiemap_writer.cpp index b911234c9..4dd4bcc14 100644 --- a/fs_mgr/libfiemap/fiemap_writer.cpp +++ b/fs_mgr/libfiemap/fiemap_writer.cpp @@ -526,11 +526,7 @@ static bool IsValidExtent(const fiemap_extent* extent, std::string_view file_pat } static bool IsLastExtent(const fiemap_extent* extent) { - if (!(extent->fe_flags & FIEMAP_EXTENT_LAST)) { - LOG(ERROR) << "Extents are being received out-of-order"; - return false; - } - return true; + return !!(extent->fe_flags & FIEMAP_EXTENT_LAST); } static bool FiemapToExtents(struct fiemap* fiemap, std::vector* extents, @@ -552,7 +548,10 @@ static bool FiemapToExtents(struct fiemap* fiemap, std::vectorfm_extents[i]; // Make sure extents are returned in order - if (next != last_extent && IsLastExtent(next)) return false; + if (next != last_extent && IsLastExtent(next)) { + LOG(ERROR) << "Extents are being received out-of-order"; + return false; + } // Check if extent's flags are valid if (!IsValidExtent(next, file_path)) return false; @@ -592,8 +591,7 @@ static bool ReadFiemap(int file_fd, const std::string& file_path, return false; } - uint64_t fiemap_size = - sizeof(struct fiemap_extent) + num_extents * sizeof(struct fiemap_extent); + uint64_t fiemap_size = sizeof(struct fiemap) + num_extents * sizeof(struct fiemap_extent); auto buffer = std::unique_ptr(calloc(1, fiemap_size), free); if (buffer == nullptr) { LOG(ERROR) << "Failed to allocate memory for fiemap"; From c352179e10bb608c93165e0721cf959402803ac4 Mon Sep 17 00:00:00 2001 From: Alessio Balsini Date: Tue, 4 Feb 2020 20:46:16 +0000 Subject: [PATCH 2/2] fiemap: add image_test and writer_test tests to presubmit Add fiemap_image_test and fiemap_writer_test to TEST_MAPPING to be run by TH as presubmits. Because of a bug under investigation, the actual fiemap_image_test_presubmit is used instead of the original, that has one test case disabled. Bug: none Test: TH, fiemap_image_test, fiemap_writer_test Change-Id: I63b5e69b5c245a18eceb1e5896df7bd0577f289b Signed-off-by: Alessio Balsini --- fs_mgr/TEST_MAPPING | 6 ++++++ fs_mgr/libfiemap/Android.bp | 30 ++++++++++++++++++++++++++++++ fs_mgr/libfiemap/image_test.cpp | 3 +++ 3 files changed, 39 insertions(+) diff --git a/fs_mgr/TEST_MAPPING b/fs_mgr/TEST_MAPPING index de38ff6a2..60e32261d 100644 --- a/fs_mgr/TEST_MAPPING +++ b/fs_mgr/TEST_MAPPING @@ -5,6 +5,12 @@ }, { "name": "liblp_test" + }, + { + "name": "fiemap_image_test_presubmit" + }, + { + "name": "fiemap_writer_test" } ] } diff --git a/fs_mgr/libfiemap/Android.bp b/fs_mgr/libfiemap/Android.bp index 1bf457ff3..2fd463c3f 100644 --- a/fs_mgr/libfiemap/Android.bp +++ b/fs_mgr/libfiemap/Android.bp @@ -104,4 +104,34 @@ cc_test { srcs: [ "image_test.cpp", ], + test_suites: ["device-tests"], + auto_gen_config: true, + require_root: true, +} + +/* BUG(148874852) temporary test */ +cc_test { + name: "fiemap_image_test_presubmit", + cppflags: [ + "-DSKIP_TEST_IN_PRESUBMIT", + ], + static_libs: [ + "libdm", + "libext4_utils", + "libfs_mgr", + "liblp", + ], + shared_libs: [ + "libbase", + "libcrypto", + "libcrypto_utils", + "libcutils", + "liblog", + ], + srcs: [ + "image_test.cpp", + ], + test_suites: ["device-tests"], + auto_gen_config: true, + require_root: true, } diff --git a/fs_mgr/libfiemap/image_test.cpp b/fs_mgr/libfiemap/image_test.cpp index 80c340fdc..5388b4478 100644 --- a/fs_mgr/libfiemap/image_test.cpp +++ b/fs_mgr/libfiemap/image_test.cpp @@ -212,6 +212,9 @@ TEST_F(ImageTest, DirectMount) { } TEST_F(ImageTest, IndirectMount) { +#ifdef SKIP_TEST_IN_PRESUBMIT + GTEST_SKIP() << "WIP failure b/148874852"; +#endif // Create a simple wrapper around the base device that we'll mount from // instead. This will simulate the code paths for dm-crypt/default-key/bow // and force us to use device-mapper rather than loop devices.