Merge pull request #5280 from ceph/wip-12384-hammer

librbd: add valgrind memory checks for unit tests

Reviewed-by: Loic Dachary <ldachary@redhat.com>

src/include/krbd.h

@@ -13,14 +13,15 @@
 #ifndef CEPH_KRBD_H
 #define CEPH_KRBD_H
 
+#include "rados/librados.h"
+
 #ifdef __cplusplus
 extern "C" {
 #endif
 
 struct krbd_ctx;
-struct CephContext;
 
-int krbd_create_from_context(struct CephContext *cct, struct krbd_ctx **pctx);
+int krbd_create_from_context(rados_config_t cct, struct krbd_ctx **pctx);
 void krbd_destroy(struct krbd_ctx *ctx);
 
 int krbd_map(struct krbd_ctx *ctx, const char *pool, const char *image,
@@ -38,7 +39,7 @@ namespace ceph {
   class Formatter;
 }
 
-int krbd_showmapped(struct krbd_ctx *ctx, Formatter *f);
+int krbd_showmapped(struct krbd_ctx *ctx, ceph::Formatter *f);
 
 #endif /* __cplusplus */
 

src/krbd.cc

@@ -34,6 +34,7 @@
 #include "common/TextTable.h"
 #include "include/assert.h"
 #include "include/stringify.h"
+#include "include/krbd.h"
 #include "mon/MonMap.h"
 
 #include <blkid/blkid.h>
@@ -582,12 +583,12 @@ int dump_images(struct krbd_ctx *ctx, Formatter *f)
   return r;
 }
 
-extern "C" int krbd_create_from_context(struct CephContext *cct,
+extern "C" int krbd_create_from_context(rados_config_t cct,
                                         struct krbd_ctx **pctx)
 {
   struct krbd_ctx *ctx = new struct krbd_ctx();
 
-  ctx->cct = cct;
+  ctx->cct = reinterpret_cast<CephContext *>(cct);
   ctx->udev = udev_new();
   if (!ctx->udev) {
     delete ctx;

src/librbd/TaskFinisher.h

@@ -35,6 +35,7 @@ class TaskFinisher {
       delete m_safe_timer;
     }
 
+    m_finisher->wait_for_empty();
     m_finisher->stop();
     delete m_finisher;
   }

src/osdc/ObjectCacher.cc

@@ -1144,6 +1144,7 @@ int ObjectCacher::_readx(OSDRead *rd, ObjectSet *oset, Context *onfinish,
            ++bh_it) {
 	uint64_t rx_bytes = static_cast<uint64_t>(
 	  stat_rx + bh_it->second->length());
+        bytes_not_in_cache += bh_it->second->length();
 	if (!waitfor_read.empty() || rx_bytes > max_size) {
 	  // cache is full with concurrent reads -- wait for rx's to complete
 	  // to constrain memory growth (especially during copy-ups)
@@ -1165,7 +1166,6 @@ int ObjectCacher::_readx(OSDRead *rd, ObjectSet *oset, Context *onfinish,
 	    bh_it->second->waitfor_read[bh_it->first].push_back( new C_RetryRead(this, rd, oset, onfinish) );
 	  }
 	}
-        bytes_not_in_cache += bh_it->second->length();
 	success = false;
       }
 

src/pybind/rbd.py

@@ -466,8 +466,12 @@ def parent_info(self):
             pool = create_string_buffer(size)
             name = create_string_buffer(size)
             snapname = create_string_buffer(size)
-            ret = self.librbd.rbd_get_parent_info(self.image, pool, len(pool),
-                name, len(name), snapname, len(snapname))
+            ret = self.librbd.rbd_get_parent_info(self.image, byref(pool),
+                                                  c_size_t(size),
+                                                  byref(name),
+                                                  c_size_t(size),
+                                                  byref(snapname),
+                                                  c_size_t(size))
             if ret == -errno.ERANGE:
                 size *= 2
 

src/test/Makefile-client.am

@@ -346,12 +346,11 @@ ceph_test_librbd_api_LDADD += $(LIBRBD_TP)
 endif
 
 if LINUX
-# Force use of C++ linker with dummy.cc - LIBKRBD is a C++ library
-ceph_test_librbd_fsx_SOURCES = test/librbd/fsx.c common/dummy.cc
+ceph_test_librbd_fsx_SOURCES = test/librbd/fsx.cc
 ceph_test_librbd_fsx_LDADD = \
 	$(LIBKRBD) $(LIBRBD) $(LIBRADOS) \
 	$(CRYPTO_LIBS) $(PTHREAD_LIBS) -luuid
-ceph_test_librbd_fsx_CFLAGS = ${AM_CFLAGS}
+ceph_test_librbd_fsx_CXXFLAGS = $(UNITTEST_CXXFLAGS)
 bin_DEBUGPROGRAMS += ceph_test_librbd_fsx
 endif
 endif # WITH_RBD

src/test/librados_test_stub/LibradosTestStub.cc

@@ -15,36 +15,47 @@
 #include "test/librados_test_stub/TestMemRadosClient.h"
 #include "objclass/objclass.h"
 #include <boost/bind.hpp>
+#include <boost/shared_ptr.hpp>
 #include <deque>
 #include <list>
 #include <vector>
 #include "include/assert.h"
 
 #define dout_subsys ceph_subsys_rados
 
+namespace {
+
+static void DeallocateRadosClient(librados::TestRadosClient* client)
+{
+  client->put();
+}
+
+} // anonymous namespace
+
+
 static librados::TestClassHandler *get_class_handler() {
-  static librados::TestClassHandler *s_class_handler = NULL;
-  if (s_class_handler == NULL) {
-    s_class_handler = new librados::TestClassHandler();
+  static boost::shared_ptr<librados::TestClassHandler> s_class_handler;
+  if (!s_class_handler) {
+    s_class_handler.reset(new librados::TestClassHandler());
     s_class_handler->open_all_classes();
   }
-  return s_class_handler;
+  return s_class_handler.get();
 }
 
 static librados::TestRadosClient *get_rados_client() {
   // TODO: use factory to allow tests to swap out impl
-  static librados::TestRadosClient *s_rados_client = NULL;
-  if (s_rados_client == NULL) {
+  static boost::shared_ptr<librados::TestRadosClient> s_rados_client;
+  if (!s_rados_client) {
     CephInitParameters iparams(CEPH_ENTITY_TYPE_CLIENT);
     CephContext *cct = common_preinit(iparams, CODE_ENVIRONMENT_LIBRARY, 0);
     cct->_conf->parse_env();
     cct->_conf->apply_changes(NULL);
-    g_ceph_context = cct;
-    s_rados_client = new librados::TestMemRadosClient(cct);
+    s_rados_client.reset(new librados::TestMemRadosClient(cct),
+                         &DeallocateRadosClient);
     cct->put();
   }
   s_rados_client->get();
-  return s_rados_client;
+  return s_rados_client.get();
 }
 
 static void do_out_buffer(bufferlist& outbl, char **outbuf, size_t *outbuflen) {

src/test/librados_test_stub/TestWatchNotify.cc

@@ -12,12 +12,14 @@ namespace librados {
 TestWatchNotify::TestWatchNotify(CephContext *cct)
   : m_cct(cct), m_finisher(new Finisher(cct)), m_handle(), m_notify_id(),
     m_file_watcher_lock("librados::TestWatchNotify::m_file_watcher_lock") {
+  m_cct->get();
   m_finisher->start();
 }
 
 TestWatchNotify::~TestWatchNotify() {
   m_finisher->stop();
   delete m_finisher;
+  m_cct->put();
 }
 
 TestWatchNotify::NotifyHandle::NotifyHandle()

src/test/librbd/fsx.c → src/test/librbd/fsx.cc

@@ -42,6 +42,7 @@
 #include "include/krbd.h"
 #include "include/rados/librados.h"
 #include "include/rbd/librbd.h"
+#include "common/ceph_crypto.h"
 
 #define NUMPRINTCOLUMNS 32	/* # columns of data to print on each line */
 
@@ -196,7 +197,7 @@ warn(const char * fmt, ...)  {
 #define BUF_SIZE 1024
 
 void
-prt(char *fmt, ...)
+prt(const char *fmt, ...)
 {
 	va_list args;
 	char buffer[BUF_SIZE];
@@ -210,13 +211,13 @@ prt(char *fmt, ...)
 }
 
 void
-prterr(char *prefix)
+prterr(const char *prefix)
 {
 	prt("%s%s%s\n", prefix, prefix ? ": " : "", strerror(errno));
 }
 
 void
-prterrcode(char *prefix, int code)
+prterrcode(const char *prefix, int code)
 {
 	prt("%s%s%s\n", prefix, prefix ? ": " : "", strerror(-code));
 }
@@ -264,8 +265,8 @@ struct rbd_ctx {
 struct rbd_operations {
 	int (*open)(const char *name, struct rbd_ctx *ctx);
 	int (*close)(struct rbd_ctx *ctx);
-	ssize_t (*read)(struct rbd_ctx *ctx, uint64_t off, size_t len, void *buf);
-	ssize_t (*write)(struct rbd_ctx *ctx, uint64_t off, size_t len, void *buf);
+	ssize_t (*read)(struct rbd_ctx *ctx, uint64_t off, size_t len, char *buf);
+	ssize_t (*write)(struct rbd_ctx *ctx, uint64_t off, size_t len, const char *buf);
 	int (*flush)(struct rbd_ctx *ctx);
 	int (*discard)(struct rbd_ctx *ctx, uint64_t off, uint64_t len);
 	int (*get_size)(struct rbd_ctx *ctx, uint64_t *size);
@@ -362,7 +363,7 @@ librbd_verify_object_map(struct rbd_ctx *ctx)
 }
 
 ssize_t
-librbd_read(struct rbd_ctx *ctx, uint64_t off, size_t len, void *buf)
+librbd_read(struct rbd_ctx *ctx, uint64_t off, size_t len, char *buf)
 {
 	ssize_t n;
 
@@ -374,7 +375,7 @@ librbd_read(struct rbd_ctx *ctx, uint64_t off, size_t len, void *buf)
 }
 
 ssize_t
-librbd_write(struct rbd_ctx *ctx, uint64_t off, size_t len, void *buf)
+librbd_write(struct rbd_ctx *ctx, uint64_t off, size_t len, const char *buf)
 {
 	ssize_t n;
 	int ret;
@@ -525,16 +526,16 @@ librbd_flatten(struct rbd_ctx *ctx)
 }
 
 const struct rbd_operations librbd_operations = {
-	.open	= librbd_open,
-	.close	= librbd_close,
-	.read	= librbd_read,
-	.write	= librbd_write,
-	.flush	= librbd_flush,
-	.discard = librbd_discard,
-	.get_size = librbd_get_size,
-	.resize	= librbd_resize,
-	.clone	= librbd_clone,
-	.flatten = librbd_flatten,
+	librbd_open,
+	librbd_close,
+	librbd_read,
+	librbd_write,
+	librbd_flush,
+	librbd_discard,
+	librbd_get_size,
+	librbd_resize,
+	librbd_clone,
+	librbd_flatten
 };
 
 int
@@ -595,7 +596,7 @@ krbd_close(struct rbd_ctx *ctx)
 }
 
 ssize_t
-krbd_read(struct rbd_ctx *ctx, uint64_t off, size_t len, void *buf)
+krbd_read(struct rbd_ctx *ctx, uint64_t off, size_t len, char *buf)
 {
 	ssize_t n;
 
@@ -610,7 +611,7 @@ krbd_read(struct rbd_ctx *ctx, uint64_t off, size_t len, void *buf)
 }
 
 ssize_t
-krbd_write(struct rbd_ctx *ctx, uint64_t off, size_t len, void *buf)
+krbd_write(struct rbd_ctx *ctx, uint64_t off, size_t len, const char *buf)
 {
 	ssize_t n;
 
@@ -738,16 +739,16 @@ krbd_flatten(struct rbd_ctx *ctx)
 }
 
 const struct rbd_operations krbd_operations = {
-	.open	= krbd_open,
-	.close	= krbd_close,
-	.read	= krbd_read,
-	.write	= krbd_write,
-	.flush	= krbd_flush,
-	.discard = krbd_discard,
-	.get_size = krbd_get_size,
-	.resize	= krbd_resize,
-	.clone	= krbd_clone,
-	.flatten = krbd_flatten,
+	krbd_open,
+	krbd_close,
+	krbd_read,
+	krbd_write,
+	krbd_flush,
+	krbd_discard,
+	krbd_get_size,
+	krbd_resize,
+	krbd_clone,
+	krbd_flatten,
 };
 
 struct rbd_ctx ctx = RBD_CTX_INIT;
@@ -793,7 +794,7 @@ logdump(void)
 {
 	int	i, count, down;
 	struct log_entry	*lp;
-	char *falloc_type[3] = {"PAST_EOF", "EXTENDING", "INTERIOR"};
+	const char *falloc_type[3] = {"PAST_EOF", "EXTENDING", "INTERIOR"};
 
 	prt("LOG DUMP (%d total operations):\n", logcount);
 	if (logcount < LOGSIZE) {
@@ -1744,8 +1745,7 @@ test(void)
 
 
 void
-cleanup(sig)
-	int	sig;
+cleanup(int sig)
 {
 	if (sig)
 		prt("signal %d\n", sig);
@@ -2312,6 +2312,7 @@ main(int argc, char **argv)
 	krbd_destroy(krbd);
 	rados_shutdown(cluster);
 
+        ceph::crypto::shutdown();
 	free(original_buf);
 	free(good_buf);
 	free(temp_buf);

src/test/librbd/test_internal.cc

@@ -257,6 +257,9 @@ TEST_F(TestInternal, AioWriteRequestsLock) {
   ASSERT_EQ(0, librbd::is_exclusive_lock_owner(ictx, &is_owner));
   ASSERT_FALSE(is_owner);
   ASSERT_FALSE(c->is_complete());
+
+  unlock_image();
+  ASSERT_EQ(0, c->wait_for_complete());
   c->put();
 }
 
@@ -277,6 +280,9 @@ TEST_F(TestInternal, AioDiscardRequestsLock) {
   ASSERT_EQ(0, librbd::is_exclusive_lock_owner(ictx, &is_owner));
   ASSERT_FALSE(is_owner);
   ASSERT_FALSE(c->is_complete());
+
+  unlock_image();
+  ASSERT_EQ(0, c->wait_for_complete());
   c->put();
 }