Merge pull request #9449 from ifed01/wip-bluestore-crc32

os/bluestore: Some improvement for crc32c calculations

Reviewed-by: Sage Weil <sage@redhat.com>

src/common/Checksummer.h

@@ -24,9 +24,7 @@ public:
       size_t len,
       bufferlist::const_iterator& p
       ) {
-      bufferlist t;
-      p.copy(len, t);
-      return t.crc32c(-1);
+      return p.crc32c(len, -1);
     }
   };
 

src/common/buffer.cc

@@ -1215,9 +1215,24 @@ static simple_spinlock_t buffer_debug_lock = SIMPLE_SPINLOCK_INITIALIZER;
       ++p;
       p_off = 0;
     }
+    off += l;
     return l;
   }
 
+  template<bool is_const>
+  uint32_t buffer::list::iterator_impl<is_const>::crc32c(
+    size_t length, uint32_t crc)
+  {
+    length = MIN( length, get_remaining());
+    while (length > 0) {
+      const char *p;
+      size_t l = get_ptr_and_advance(length, &p);
+      crc = ceph_crc32c(crc, (unsigned char*)p, l);
+      length -= l;
+    }
+    return crc;
+  }
+
   // explicitly instantiate only the iterator types we need, so we can hide the
   // details in this compilation unit without introducing unnecessary link time
   // dependencies.

src/include/buffer.h

@@ -329,6 +329,9 @@ namespace buffer CEPH_BUFFER_API {
       // and advance the iterator by that amount.
       size_t get_ptr_and_advance(size_t want, const char **p);
 
+      /// calculate crc from iterator position
+      uint32_t crc32c(size_t length, uint32_t crc);
+
       friend bool operator==(const iterator_impl& lhs,
 			     const iterator_impl& rhs) {
 	return &lhs.get_bl() == &rhs.get_bl() && lhs.get_off() == rhs.get_off();

src/os/bluestore/BlueStore.cc

@@ -3335,7 +3335,7 @@ int BlueStore::_verify_csum(const bluestore_blob_t* blob, uint64_t blob_xoffset,
 {
   int bad = blob->verify_csum(blob_xoffset, bl);
   if (bad >= 0) {
-    dout(20) << __func__ << " at blob offset 0x" << bad << dendl;
+    dout(20) << __func__ << " at blob offset 0x" << std::hex << bad << dendl;
     return -1;
   }
   return 0;

src/test/bufferlist.cc

@@ -1049,6 +1049,7 @@ TEST(BufferListIterator, get_ptr_and_advance)
   const char *ptr;
   bufferlist::iterator p = bl.begin();
   ASSERT_EQ(3u, p.get_ptr_and_advance(11, &ptr));
+  ASSERT_EQ(bl.length() - 3u, p.get_remaining());
   ASSERT_EQ(0, memcmp(ptr, "one", 3));
   ASSERT_EQ(2u, p.get_ptr_and_advance(2, &ptr));
   ASSERT_EQ(0, memcmp(ptr, "tw", 2));
@@ -1056,6 +1057,43 @@ TEST(BufferListIterator, get_ptr_and_advance)
   ASSERT_EQ(0, memcmp(ptr, "o", 1));
   ASSERT_EQ(5u, p.get_ptr_and_advance(5, &ptr));
   ASSERT_EQ(0, memcmp(ptr, "three", 5));
+  ASSERT_EQ(0u, p.get_remaining());
+}
+
+TEST(BufferListIterator, iterator_crc32c) {
+  bufferlist bl1;
+  bufferlist bl2;
+  bufferlist bl3;
+
+  string s1(100, 'a');
+  string s2(50, 'b');
+  string s3(7, 'c');
+  string s;
+  bl1.append(s1);
+  bl1.append(s2);
+  bl1.append(s3);
+  s = s1 + s2 + s3;
+  bl2.append(s);
+
+  bufferlist::iterator it = bl2.begin();
+  ASSERT_EQ(bl1.crc32c(0), it.crc32c(it.get_remaining(), 0));
+  ASSERT_EQ(0u, it.get_remaining());
+
+  it = bl1.begin();
+  ASSERT_EQ(bl2.crc32c(0), it.crc32c(it.get_remaining(), 0));
+
+  bl3.append(s.substr(98, 55));
+  it = bl1.begin();
+  it.advance(98);
+  ASSERT_EQ(bl3.crc32c(0), it.crc32c(55, 0));
+  ASSERT_EQ(4u, it.get_remaining());
+
+  bl3.clear();
+  bl3.append(s.substr(98 + 55));
+  it = bl1.begin();
+  it.advance(98 + 55);
+  ASSERT_EQ(bl3.crc32c(0), it.crc32c(10, 0));
+  ASSERT_EQ(0u, it.get_remaining());
 }
 
 TEST(BufferListIterator, seek) {