From 4cb83c14dbe09d4b371f7b728d9b5c0549e59f1a Mon Sep 17 00:00:00 2001
From: Jason Dillaman <dillaman@redhat.com>
Date: Tue, 25 Oct 2016 09:43:06 -0400
Subject: [PATCH] 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>
---
 src/librbd/AioImageRequest.cc  |  1 +
 src/test/librbd/test_librbd.cc | 42 ++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/src/librbd/AioImageRequest.cc b/src/librbd/AioImageRequest.cc
index 8c95390159f1c..d2aee487aeaa9 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 5101a7e4b2aa6..ff91f25564d83 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();
+}
-- 
2.39.5