From fca78765366f3ac365cfa98224aca5fb79b2a7fe Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Mon, 10 Aug 2015 09:34:42 -0400 Subject: [PATCH 1/2] common: bit_vector extent calculation incorrect for last page It's highly probable that the last page in the bit vector will not be a full page size. As a result, the computed extents will extend beyond the data portion of the bit vector, resulting in a end_of_buffer exception. Fixes: #12611 Backport: hammer Signed-off-by: Jason Dillaman (cherry picked from commit c6d98992691683524d3d96def83a90a6f5fe7f93) --- src/common/bit_vector.hpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/common/bit_vector.hpp b/src/common/bit_vector.hpp index 55403c5d35cdb..f66294b54752e 100644 --- a/src/common/bit_vector.hpp +++ b/src/common/bit_vector.hpp @@ -261,7 +261,10 @@ void BitVector<_b>::get_data_extents(uint64_t offset, uint64_t length, end_offset += (CEPH_PAGE_SIZE - (end_offset % CEPH_PAGE_SIZE)); assert(*byte_offset <= end_offset); - *byte_length = MIN(end_offset - *byte_offset, m_data.length()); + *byte_length = end_offset - *byte_offset; + if (*byte_offset + *byte_length > m_data.length()) { + *byte_length = m_data.length() - *byte_offset; + } } template From 153744d7c596705c4f92bee5e827846b46c80141 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Mon, 10 Aug 2015 09:39:50 -0400 Subject: [PATCH 2/2] tests: increase test coverage for partial encodes/decodes Multiple combinations of offsets/lengths are now tested when performing partial encodes/decodes of the bit vector. Signed-off-by: Jason Dillaman (cherry picked from commit 3e145f714ac9b2d599b45a058c6b93595e38f424) --- src/test/common/test_bit_vector.cc | 88 ++++++++++++++++++------------ 1 file changed, 53 insertions(+), 35 deletions(-) diff --git a/src/test/common/test_bit_vector.cc b/src/test/common/test_bit_vector.cc index be31d257f943e..c58583c294741 100644 --- a/src/test/common/test_bit_vector.cc +++ b/src/test/common/test_bit_vector.cc @@ -11,6 +11,7 @@ #include #include #include "common/bit_vector.hpp" +#include using namespace ceph; @@ -87,8 +88,9 @@ TYPED_TEST(BitVectorTest, get_set) { TYPED_TEST(BitVectorTest, get_buffer_extents) { typename TestFixture::bit_vector_t bit_vector; + uint64_t element_count = 2 * CEPH_PAGE_SIZE + 51; uint64_t elements_per_byte = 8 / bit_vector.BIT_COUNT; - bit_vector.resize((2 * CEPH_PAGE_SIZE + 51) * elements_per_byte); + bit_vector.resize(element_count * elements_per_byte); uint64_t offset = (CEPH_PAGE_SIZE + 11) * elements_per_byte; uint64_t length = (CEPH_PAGE_SIZE + 31) * elements_per_byte; @@ -96,7 +98,7 @@ TYPED_TEST(BitVectorTest, get_buffer_extents) { uint64_t byte_length; bit_vector.get_data_extents(offset, length, &byte_offset, &byte_length); ASSERT_EQ(CEPH_PAGE_SIZE, byte_offset); - ASSERT_EQ(2 * CEPH_PAGE_SIZE, byte_length); + ASSERT_EQ(CEPH_PAGE_SIZE + (element_count % CEPH_PAGE_SIZE), byte_length); bit_vector.get_data_extents(1, 1, &byte_offset, &byte_length); ASSERT_EQ(0U, byte_offset); @@ -128,7 +130,7 @@ TYPED_TEST(BitVectorTest, partial_decode_encode) { typename TestFixture::bit_vector_t bit_vector; uint64_t elements_per_byte = 8 / bit_vector.BIT_COUNT; - bit_vector.resize(5111 * elements_per_byte); + bit_vector.resize(9161 * elements_per_byte); for (uint64_t i = 0; i < bit_vector.size(); ++i) { bit_vector[i] = i % 4; } @@ -148,38 +150,54 @@ TYPED_TEST(BitVectorTest, partial_decode_encode) { bufferlist::iterator footer_it = footer_bl.begin(); bit_vector.decode_footer(footer_it); - uint64_t byte_offset; - uint64_t byte_length; - bit_vector.get_data_extents(0, 1, &byte_offset, &byte_length); - - bufferlist data_bl; - data_bl.substr_of(bl, bit_vector.get_header_length() + byte_offset, - byte_length); - bufferlist::iterator data_it = data_bl.begin(); - bit_vector.decode_data(data_it, byte_offset); - - bit_vector[0] = 3; - - data_bl.clear(); - bit_vector.encode_data(data_bl, byte_offset, byte_length); - - footer_bl.clear(); - bit_vector.encode_footer(footer_bl); - - bufferlist updated_bl; - updated_bl.substr_of(bl, 0, bit_vector.get_header_length() + byte_offset); - updated_bl.append(data_bl); - - uint64_t tail_data_offset = bit_vector.get_header_length() + byte_offset + - byte_length; - data_bl.substr_of(bl, tail_data_offset, - bit_vector.get_footer_offset() - tail_data_offset); - updated_bl.append(data_bl); - updated_bl.append(footer_bl); - ASSERT_EQ(bl.length(), updated_bl.length()); - - bufferlist::iterator updated_it = updated_bl.begin(); - ::decode(bit_vector, updated_it); + typedef std::pair Extent; + typedef std::list Extents; + + Extents extents = boost::assign::list_of( + std::make_pair(0, 1))( + std::make_pair((CEPH_PAGE_SIZE * elements_per_byte) - 2, 4))( + std::make_pair((CEPH_PAGE_SIZE * elements_per_byte) + 2, 2))( + std::make_pair((2 * CEPH_PAGE_SIZE * elements_per_byte) - 2, 4))( + std::make_pair((2 * CEPH_PAGE_SIZE * elements_per_byte) + 2, 2))( + std::make_pair(2, 2 * CEPH_PAGE_SIZE)); + for (Extents::iterator it = extents.begin(); it != extents.end(); ++it) { + uint64_t element_offset = it->first; + uint64_t element_length = it->second; + uint64_t byte_offset; + uint64_t byte_length; + bit_vector.get_data_extents(element_offset, element_length, &byte_offset, + &byte_length); + + bufferlist data_bl; + data_bl.substr_of(bl, bit_vector.get_header_length() + byte_offset, + byte_length); + bufferlist::iterator data_it = data_bl.begin(); + bit_vector.decode_data(data_it, byte_offset); + + data_bl.clear(); + bit_vector.encode_data(data_bl, byte_offset, byte_length); + + footer_bl.clear(); + bit_vector.encode_footer(footer_bl); + + bufferlist updated_bl; + updated_bl.substr_of(bl, 0, bit_vector.get_header_length() + byte_offset); + updated_bl.append(data_bl); + + if (byte_offset + byte_length < bit_vector.get_footer_offset()) { + uint64_t tail_data_offset = bit_vector.get_header_length() + byte_offset + + byte_length; + data_bl.substr_of(bl, tail_data_offset, + bit_vector.get_footer_offset() - tail_data_offset); + updated_bl.append(data_bl); + } + + updated_bl.append(footer_bl); + ASSERT_EQ(bl, updated_bl); + + bufferlist::iterator updated_it = updated_bl.begin(); + ::decode(bit_vector, updated_it); + } } TYPED_TEST(BitVectorTest, header_crc) {