librados_test_stub: prevent interleaving of operations

It was possible for unrelated ops to interleave within a different
transaction for a given object.

Fixes: #13313
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
Reviewed-by: Josh Durgin <jdurgin@redhat.com>

diff --git a/src/test/librados_test_stub/LibradosTestStub.cc b/src/test/librados_test_stub/LibradosTestStub.cc
index a32a304125f5bc57ee80024ff7bc237ad7b5bd82..2a0006e662d25324d76c555185bdd60c957971b6 100644 (file)
--- a/src/test/librados_test_stub/LibradosTestStub.cc
+++ b/src/test/librados_test_stub/LibradosTestStub.cc
@@ -409,7 +409,8 @@ void IoCtx::close() {
 
 int IoCtx::create(const std::string& oid, bool exclusive) {
   TestIoCtxImpl *ctx = reinterpret_cast<TestIoCtxImpl*>(io_ctx_impl);
-  return ctx->create(oid, exclusive);
+  return ctx->execute_operation(
+    oid, boost::bind(&TestIoCtxImpl::create, _1, _2, exclusive));
 }
 
 void IoCtx::dup(const IoCtx& rhs) {
@@ -421,8 +422,9 @@ void IoCtx::dup(const IoCtx& rhs) {
 int IoCtx::exec(const std::string& oid, const char *cls, const char *method,
                 bufferlist& inbl, bufferlist& outbl) {
   TestIoCtxImpl *ctx = reinterpret_cast<TestIoCtxImpl*>(io_ctx_impl);
-  return ctx->exec(oid, get_class_handler(), cls, method, inbl, &outbl,
-                   ctx->get_snap_context());
+  return ctx->execute_operation(
+    oid, boost::bind(&TestIoCtxImpl::exec, _1, _2, get_class_handler(), cls,
+                     method, inbl, &outbl, ctx->get_snap_context()));
 }
 
 void IoCtx::from_rados_ioctx_t(rados_ioctx_t p, IoCtx &io) {
@@ -455,13 +457,15 @@ std::string IoCtx::get_pool_name() {
 
 int IoCtx::list_snaps(const std::string& o, snap_set_t *out_snaps) {
   TestIoCtxImpl *ctx = reinterpret_cast<TestIoCtxImpl*>(io_ctx_impl);
-  return ctx->list_snaps(o, out_snaps);
+  return ctx->execute_operation(
+    o, boost::bind(&TestIoCtxImpl::list_snaps, _1, _2, out_snaps));
 }
 
 int IoCtx::list_watchers(const std::string& o,
                          std::list<obj_watch_t> *out_watchers) {
   TestIoCtxImpl *ctx = reinterpret_cast<TestIoCtxImpl*>(io_ctx_impl);
-  return ctx->list_watchers(o, out_watchers);
+  return ctx->execute_operation(
+    o, boost::bind(&TestIoCtxImpl::list_watchers, _1, _2, out_watchers));
 }
 
 int IoCtx::notify(const std::string& o, uint64_t ver, bufferlist& bl) {
@@ -486,7 +490,9 @@ int IoCtx::omap_get_vals(const std::string& oid,
                          uint64_t max_return,
                          std::map<std::string, bufferlist> *out_vals) {
   TestIoCtxImpl *ctx = reinterpret_cast<TestIoCtxImpl*>(io_ctx_impl);
-  return ctx->omap_get_vals(oid, start_after, "", max_return, out_vals);
+  return ctx->execute_operation(
+    oid, boost::bind(&TestIoCtxImpl::omap_get_vals, _1, _2, start_after, "",
+                     max_return, out_vals));
 }
 
 int IoCtx::operate(const std::string& oid, ObjectWriteOperation *op) {
@@ -505,11 +511,14 @@ int IoCtx::operate(const std::string& oid, ObjectReadOperation *op,
 int IoCtx::read(const std::string& oid, bufferlist& bl, size_t len,
                 uint64_t off) {
   TestIoCtxImpl *ctx = reinterpret_cast<TestIoCtxImpl*>(io_ctx_impl);
-  return ctx->read(oid, len, off, &bl);
+  return ctx->execute_operation(
+    oid, boost::bind(&TestIoCtxImpl::read, _1, _2, len, off, &bl));
 }
 
 int IoCtx::remove(const std::string& oid) {
   TestIoCtxImpl *ctx = reinterpret_cast<TestIoCtxImpl*>(io_ctx_impl);
+  return ctx->execute_operation(
+    oid, boost::bind(&TestIoCtxImpl::remove, _1, _2));
   return ctx->remove(oid);
 }
 
@@ -542,12 +551,14 @@ void IoCtx::snap_set_read(snap_t seq) {
 
 int IoCtx::stat(const std::string& oid, uint64_t *psize, time_t *pmtime) {
   TestIoCtxImpl *ctx = reinterpret_cast<TestIoCtxImpl*>(io_ctx_impl);
-  return ctx->stat(oid, psize, pmtime);;
+  return ctx->execute_operation(
+    oid, boost::bind(&TestIoCtxImpl::stat, _1, _2, psize, pmtime));
 }
 
 int IoCtx::tmap_update(const std::string& oid, bufferlist& cmdbl) {
   TestIoCtxImpl *ctx = reinterpret_cast<TestIoCtxImpl*>(io_ctx_impl);
-  return ctx->tmap_update(oid, cmdbl);
+  return ctx->execute_operation(
+    oid, boost::bind(&TestIoCtxImpl::tmap_update, _1, _2, cmdbl));
 }
 
 int IoCtx::unwatch2(uint64_t handle) {
@@ -575,12 +586,16 @@ int IoCtx::watch2(const std::string& o, uint64_t *handle,
 int IoCtx::write(const std::string& oid, bufferlist& bl, size_t len,
                  uint64_t off) {
   TestIoCtxImpl *ctx = reinterpret_cast<TestIoCtxImpl*>(io_ctx_impl);
-  return ctx->write(oid, bl, len, off, ctx->get_snap_context());
+  return ctx->execute_operation(
+    oid, boost::bind(&TestIoCtxImpl::write, _1, _2, bl, len, off,
+                     ctx->get_snap_context()));
 }
 
 int IoCtx::write_full(const std::string& oid, bufferlist& bl) {
   TestIoCtxImpl *ctx = reinterpret_cast<TestIoCtxImpl*>(io_ctx_impl);
-  return ctx->write_full(oid, bl, ctx->get_snap_context());
+  return ctx->execute_operation(
+    oid, boost::bind(&TestIoCtxImpl::write_full, _1, _2, bl,
+                     ctx->get_snap_context()));
 }
 
 static int save_operation_result(int result, int *pval) {

diff --git a/src/test/librados_test_stub/TestIoCtxImpl.cc b/src/test/librados_test_stub/TestIoCtxImpl.cc
index 2304b10f31e08e50f57eb6f3b744c496d1246724..30c1e13a3811ef79c0d873d4180e759ea585e765 100644 (file)
--- a/src/test/librados_test_stub/TestIoCtxImpl.cc
+++ b/src/test/librados_test_stub/TestIoCtxImpl.cc
@@ -251,10 +251,17 @@ int TestIoCtxImpl::watch(const std::string& o, uint64_t *handle,
                                             ctx2);
 }
 
+int TestIoCtxImpl::execute_operation(const std::string& oid,
+                                     const Operation &operation) {
+  TestRadosClient::Transaction transaction(m_client, oid);
+  return operation(this, oid);
+}
+
 int TestIoCtxImpl::execute_aio_operations(const std::string& oid,
                                           TestObjectOperationImpl *ops,
                                           bufferlist *pbl,
                                           const SnapContext &snapc) {
+  TestRadosClient::Transaction transaction(m_client, oid);
   int ret = 0;
   for (ObjectOperations::iterator it = ops->ops.begin();
        it != ops->ops.end(); ++it) {

diff --git a/src/test/librados_test_stub/TestIoCtxImpl.h b/src/test/librados_test_stub/TestIoCtxImpl.h
index 1cb6213ff9d8c7107c0461ebb5665ad6a27af1fe..450ee59d7132e038d60736c79dc75503be01ae0d 100644 (file)
--- a/src/test/librados_test_stub/TestIoCtxImpl.h
+++ b/src/test/librados_test_stub/TestIoCtxImpl.h
@@ -34,6 +34,8 @@ private:
 
 class TestIoCtxImpl {
 public:
+  typedef boost::function<int(TestIoCtxImpl *, const std::string &)> Operation;
+
 
   TestIoCtxImpl();
   explicit TestIoCtxImpl(TestRadosClient *client, int64_t m_pool_id,
@@ -136,6 +138,9 @@ public:
                         bufferlist& bl) = 0;
   virtual int zero(const std::string& oid, uint64_t off, uint64_t len) = 0;
 
+  int execute_operation(const std::string& oid,
+                        const Operation &operation);
+
 protected:
   TestIoCtxImpl(const TestIoCtxImpl& rhs);
   virtual ~TestIoCtxImpl();

diff --git a/src/test/librados_test_stub/TestRadosClient.cc b/src/test/librados_test_stub/TestRadosClient.cc
index 925dc598385b32c6920163fe84ce38a2bc960ff6..46437ac8657e329aba6dd59b48988d3a7010e758 100644 (file)
--- a/src/test/librados_test_stub/TestRadosClient.cc
+++ b/src/test/librados_test_stub/TestRadosClient.cc
@@ -84,7 +84,8 @@ private:
 
 TestRadosClient::TestRadosClient(CephContext *cct)
   : m_cct(cct->get()),
-    m_watch_notify(m_cct)
+    m_watch_notify(m_cct),
+    m_transaction_lock("TestRadosClient::m_transaction_lock")
 {
   get();
 
@@ -225,4 +226,21 @@ Finisher *TestRadosClient::get_finisher(const std::string &oid) {
   return m_finishers[h % m_finishers.size()];
 }
 
+void TestRadosClient::transaction_start(const std::string &oid) {
+  Mutex::Locker locker(m_transaction_lock);
+  while (m_transactions.count(oid)) {
+    m_transaction_cond.Wait(m_transaction_lock);
+  }
+  std::pair<std::set<std::string>::iterator, bool> result =
+    m_transactions.insert(oid);
+  assert(result.second);
+}
+
+void TestRadosClient::transaction_finish(const std::string &oid) {
+  Mutex::Locker locker(m_transaction_lock);
+  size_t count = m_transactions.erase(oid);
+  assert(count == 1);
+  m_transaction_cond.Signal();
+}
+
 } // namespace librados

diff --git a/src/test/librados_test_stub/TestRadosClient.h b/src/test/librados_test_stub/TestRadosClient.h
index e811bafaa16ad19f0e8dff6b6a1b0dcbf724cfbc..ad0cf676dd04f188049ce25f74843be122886321 100644 (file)
--- a/src/test/librados_test_stub/TestRadosClient.h
+++ b/src/test/librados_test_stub/TestRadosClient.h
@@ -6,6 +6,8 @@
 
 #include "include/rados/librados.hpp"
 #include "common/config.h"
+#include "common/Cond.h"
+#include "common/Mutex.h"
 #include "include/atomic.h"
 #include "include/buffer.h"
 #include "test/librados_test_stub/TestWatchNotify.h"
@@ -13,6 +15,7 @@
 #include <boost/functional/hash.hpp>
 #include <list>
 #include <map>
+#include <set>
 #include <string>
 #include <vector>
 
@@ -38,6 +41,20 @@ public:
     std::string nspace;
   };
 
+  class Transaction {
+  public:
+    Transaction(TestRadosClient *rados_client, const std::string &oid)
+      : rados_client(rados_client), oid(oid) {
+      rados_client->transaction_start(oid);
+    }
+    ~Transaction() {
+      rados_client->transaction_finish(oid);
+    }
+  private:
+    TestRadosClient *rados_client;
+    std::string oid;
+  };
+
   TestRadosClient(CephContext *cct);
 
   void get();
@@ -98,6 +115,13 @@ private:
 
   TestWatchNotify m_watch_notify;
 
+  Mutex m_transaction_lock;
+  Cond m_transaction_cond;
+  std::set<std::string> m_transactions;
+
+  void transaction_start(const std::string &oid);
+  void transaction_finish(const std::string &oid);
+
 };
 
 } // namespace librados