From: Jason Dillaman <dillaman@redhat.com>
Date: Tue, 25 Oct 2016 13:43:06 +0000 (-0400)
Subject: librbd: discard after write can result in assertion failure
X-Git-Tag: v10.2.4~15^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=refs%2Fpull%2F11644%2Fhead;p=ceph.git

librbd: discard after write can result in assertion failure

With journaling enabled, the proper lock is not held when handling
a discard after write to overlapping extents. This issue is only present
on the jewel branch due to design changes on the master branch.

Fixes: http://tracker.ceph.com/issues/17695
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
---

diff --git a/src/librbd/AioImageRequest.cc b/src/librbd/AioImageRequest.cc
index 8c95390159f1..d2aee487aeaa 100644
--- a/src/librbd/AioImageRequest.cc
+++ b/src/librbd/AioImageRequest.cc
@@ -48,6 +48,7 @@ struct C_DiscardJournalCommit : public Context {
     ldout(cct, 20) << this << " C_DiscardJournalCommit: "
                    << "journal committed: discarding from cache" << dendl;
 
+    RWLock::RLocker owner_locker(image_ctx.owner_lock);
     Mutex::Locker cache_locker(image_ctx.cache_lock);
     image_ctx.object_cacher->discard_set(image_ctx.object_set, object_extents);
     aio_comp->complete_request(r);
diff --git a/src/test/librbd/test_librbd.cc b/src/test/librbd/test_librbd.cc
index 5101a7e4b2aa..ff91f25564d8 100644
--- a/src/test/librbd/test_librbd.cc
+++ b/src/test/librbd/test_librbd.cc
@@ -4357,3 +4357,45 @@ TEST_F(TestLibRBD, FlushCacheWithCopyupOnExternalSnapshot) {
   ASSERT_EQ(0, read_comp->wait_for_complete());
   read_comp->release();
 }
+
+TEST_F(TestLibRBD, DiscardAfterWrite)
+{
+  librados::IoCtx ioctx;
+  ASSERT_EQ(0, _rados.ioctx_create(m_pool_name.c_str(), ioctx));
+
+  librbd::RBD rbd;
+  std::string name = get_temp_image_name();
+  uint64_t size = 1 << 20;
+  int order = 18;
+  ASSERT_EQ(0, create_image_pp(rbd, ioctx, name.c_str(), size, &order));
+
+  librbd::Image image;
+  ASSERT_EQ(0, rbd.open(ioctx, image, name.c_str(), NULL));
+
+  // enable writeback cache
+  ASSERT_EQ(0, image.flush());
+
+  bufferlist bl;
+  bl.append(std::string(256, '1'));
+
+  librbd::RBD::AioCompletion *write_comp =
+    new librbd::RBD::AioCompletion(NULL, NULL);
+  ASSERT_EQ(0, image.aio_write(0, bl.length(), bl, write_comp));
+  ASSERT_EQ(0, write_comp->wait_for_complete());
+  write_comp->release();
+
+  librbd::RBD::AioCompletion *discard_comp =
+    new librbd::RBD::AioCompletion(NULL, NULL);
+  ASSERT_EQ(0, image.aio_discard(0, 256, discard_comp));
+  ASSERT_EQ(0, discard_comp->wait_for_complete());
+  discard_comp->release();
+
+  librbd::RBD::AioCompletion *read_comp =
+    new librbd::RBD::AioCompletion(NULL, NULL);
+  bufferlist read_bl;
+  image.aio_read(0, bl.length(), read_bl, read_comp);
+  ASSERT_EQ(0, read_comp->wait_for_complete());
+  ASSERT_EQ(bl.length(), read_comp->get_return_value());
+  ASSERT_TRUE(read_bl.is_zero());
+  read_comp->release();
+}