librbd: improve copy_with_progress

Signed-off-by: Colin McCabe <colin.mccabe@dreamhost.com>

diff --git a/src/include/rbd/librbd.h b/src/include/rbd/librbd.h
index a143edab50e512568c78c567d744eb68340ccb16..8389f98972a75ae73f83e00c6900a20563d304a1 100644 (file)
--- a/src/include/rbd/librbd.h
+++ b/src/include/rbd/librbd.h
@@ -37,7 +37,7 @@ extern "C" {
 typedef void *rbd_snap_t;
 typedef void *rbd_image_t;
 
-typedef void (*librbd_copy_progress_fn_t)(uint64_t offset, uint64_t src_size,
+typedef int (*librbd_copy_progress_fn_t)(uint64_t offset, uint64_t src_size,
                                         void *data);
 
 typedef struct {

diff --git a/src/include/rbd/librbd.hpp b/src/include/rbd/librbd.hpp
index a48676e65095f9b614028a3a24d42270e6eb491f..956f072576e281b8eb9a7489d633bde450ae3bac 100644 (file)
--- a/src/include/rbd/librbd.hpp
+++ b/src/include/rbd/librbd.hpp
@@ -32,8 +32,6 @@ namespace librbd {
   typedef void *image_ctx_t;
   typedef void *completion_t;
   typedef void (*callback_t)(completion_t cb, void *arg);
-  typedef void (*copy_progress_fn_t)(uint64_t offset, uint64_t src_size,
-                                    void *data);
 
   typedef struct {
     uint64_t id;
@@ -43,6 +41,13 @@ namespace librbd {
 
   typedef rbd_image_info_t image_info_t;
 
+class ProgressContext
+{
+public:
+  virtual ~ProgressContext();
+  virtual int update_progress(uint64_t offset, uint64_t src_size) = 0;
+};
+
 class RBD
 {
 public:
@@ -82,7 +87,7 @@ public:
   int stat(image_info_t &info, size_t infosize);
   int copy(IoCtx& dest_io_ctx, const char *destname);
   int copy_with_progress(IoCtx& dest_io_ctx, const char *destname,
-           copy_progress_fn_t fn, void *data);
+                        ProgressContext &prog_ctx);
 
   /* snapshots */
   int snap_list(std::vector<snap_info_t>& snaps);

diff --git a/src/librbd.cc b/src/librbd.cc
index 289b10b7b9283e7a100d96243a61d80fb9f349cd..3846cb7aeae302a967da5a24c27fe772e533d9bf 100644 (file)
--- a/src/librbd.cc
+++ b/src/librbd.cc
@@ -1013,10 +1013,13 @@ int snap_rollback(ImageCtx *ictx, const char *snap_name)
 }
 
 struct CopyProgressCtx {
+  CopyProgressCtx(ProgressContext &p)
+       : prog_ctx(p)
+  {
+  }
   ImageCtx *destictx;
   uint64_t src_size;
-  copy_progress_fn_t fn;
-  void *data;
+  ProgressContext &prog_ctx;
 };
 
 int do_copy_extent(uint64_t offset, size_t len, const char *buf, void *data)
@@ -1028,16 +1031,46 @@ int do_copy_extent(uint64_t offset, size_t len, const char *buf, void *data)
       return ret;
     }
   }
-  if (cp->fn)
-    cp->fn(offset, cp->src_size, cp->data);
-  return 0;
+  return cp->prog_ctx.update_progress(offset, cp->src_size);
+}
+
+ProgressContext::~ProgressContext()
+{
 }
 
+class NoOpProgressContext : public librbd::ProgressContext
+{
+public:
+  NoOpProgressContext()
+  {
+  }
+  int update_progress(uint64_t offset, uint64_t src_size)
+  {
+    return 0;
+  }
+};
+
+class CProgressContext : public librbd::ProgressContext
+{
+public:
+  CProgressContext(librbd_copy_progress_fn_t fn, void *data)
+    : m_fn(fn), m_data(data)
+  {
+  }
+  int update_progress(uint64_t offset, uint64_t src_size)
+  {
+    return m_fn(offset, src_size, m_data);
+  }
+private:
+  librbd_copy_progress_fn_t m_fn;
+  void *m_data;
+};
+
 int copy(ImageCtx& ictx, IoCtx& dest_md_ctx, const char *destname,
-        copy_progress_fn_t fn, void *data)
+        ProgressContext &prog_ctx)
 {
   CephContext *cct = dest_md_ctx.cct();
-  CopyProgressCtx cp;
+  CopyProgressCtx cp(prog_ctx);
 
   uint64_t src_size = ictx.get_image_size();
 
@@ -1050,8 +1083,6 @@ int copy(ImageCtx& ictx, IoCtx& dest_md_ctx, const char *destname,
 
   cp.destictx = new librbd::ImageCtx(destname, dest_md_ctx);
   cp.src_size = src_size;
-  cp.fn = fn;
-  cp.data = data;
   r = open_image(dest_md_ctx, cp.destictx, destname, NULL);
   if (r < 0) {
     lderr(cct) << "failed to read newly created header" << dendl;
@@ -1059,8 +1090,9 @@ int copy(ImageCtx& ictx, IoCtx& dest_md_ctx, const char *destname,
   }
 
   r = read_iterate(&ictx, 0, src_size, do_copy_extent, &cp);
-  if ((r >= 0) && (cp.fn)) {
-    cp.fn(cp.src_size, cp.src_size, cp.data);
+
+  if (r >= 0) {
+    prog_ctx.update_progress(cp.src_size, cp.src_size);
   }
   close_image(cp.destictx);
   return r;
@@ -1577,15 +1609,16 @@ int Image::stat(image_info_t& info, size_t infosize)
 int Image::copy(IoCtx& dest_io_ctx, const char *destname)
 {
   ImageCtx *ictx = (ImageCtx *)ctx;
-  int r = librbd::copy(*ictx, dest_io_ctx, destname, NULL, NULL);
+  librbd::NoOpProgressContext prog_ctx;
+  int r = librbd::copy(*ictx, dest_io_ctx, destname, prog_ctx);
   return r;
 }
 
 int Image::copy_with_progress(IoCtx& dest_io_ctx, const char *destname,
-                             copy_progress_fn_t fn, void *data)
+                             librbd::ProgressContext &pctx)
 {
   ImageCtx *ictx = (ImageCtx *)ctx;
-  int r = librbd::copy(*ictx, dest_io_ctx, destname, fn, data);
+  int r = librbd::copy(*ictx, dest_io_ctx, destname, pctx);
   return r;
 }
 
@@ -1723,7 +1756,8 @@ extern "C" int rbd_copy(rbd_image_t image, rados_ioctx_t dest_p, const char *des
   librbd::ImageCtx *ictx = (librbd::ImageCtx *)image;
   librados::IoCtx dest_io_ctx;
   librados::IoCtx::from_rados_ioctx_t(dest_p, dest_io_ctx);
-  return librbd::copy(*ictx, dest_io_ctx, destname, NULL, NULL);
+  librbd::NoOpProgressContext prog_ctx;
+  return librbd::copy(*ictx, dest_io_ctx, destname, prog_ctx);
 }
 
 extern "C" int rbd_copy_with_progress(rbd_image_t image, rados_ioctx_t dest_p,
@@ -1732,7 +1766,9 @@ extern "C" int rbd_copy_with_progress(rbd_image_t image, rados_ioctx_t dest_p,
   librbd::ImageCtx *ictx = (librbd::ImageCtx *)image;
   librados::IoCtx dest_io_ctx;
   librados::IoCtx::from_rados_ioctx_t(dest_p, dest_io_ctx);
-  return librbd::copy(*ictx, dest_io_ctx, destname, fn, data);
+  librbd::CProgressContext prog_ctx(fn, data);
+  int ret = librbd::copy(*ictx, dest_io_ctx, destname, prog_ctx);
+  return ret;
 }
 
 extern "C" int rbd_rename(rados_ioctx_t src_p, const char *srcname, const char *destname)

diff --git a/src/testlibrbd.c b/src/testlibrbd.c
index eddffd8946be7f443c0f32707b37b12caa8de4cb..5624ebb5d9f4d9bd72484b7095c8fc7e6135f3da 100644 (file)
--- a/src/testlibrbd.c
+++ b/src/testlibrbd.c
@@ -26,6 +26,7 @@
 #include <unistd.h>
 
 #define TEST_IMAGE "testimg"
+#define TEST_IMAGE2 "testimg2"
 #define TEST_POOL "librbdtest"
 #define TEST_SNAP "testsnap"
 #define TEST_IO_SIZE 512
@@ -325,6 +326,35 @@ void test_io_to_snapshot(rados_ioctx_t io_ctx, rbd_image_t image, size_t isize)
   test_ls_snaps(image, 0);
 }
 
+void test_rbd_copy(rados_ioctx_t io_ctx, rbd_image_t image)
+{
+  int ret;
+  ret = rbd_copy(image, io_ctx, TEST_IMAGE2);
+  if (ret < 0) {
+    fprintf(stderr, "rbd_copy returned %d!\n", ret);
+    abort();
+  }
+}
+
+static int print_progress_percent(uint64_t offset, uint64_t src_size,
+                                    void *data)
+{
+  float percent = ((float)offset * 100) / src_size;
+  printf("%3.2f%% done\n", percent);
+  return 0; 
+}
+
+void test_rbd_copy_with_progress(rados_ioctx_t io_ctx, rbd_image_t image)
+{
+  int ret;
+  ret = rbd_copy_with_progress(image, io_ctx, TEST_IMAGE2,
+                                print_progress_percent, NULL);
+  if (ret < 0) {
+    fprintf(stderr, "rbd_copy_with_progress returned %d!\n", ret);
+    abort();
+  }
+}
+
 int main(int argc, const char **argv) 
 {
   rados_t cluster;
@@ -380,6 +410,10 @@ int main(int argc, const char **argv)
   test_delete(io_ctx, TEST_IMAGE "1");
   test_ls(io_ctx, 0);
 
+  test_rbd_copy(io_ctx, image);
+  test_delete(io_ctx, TEST_IMAGE2);
+  test_rbd_copy_with_progress(io_ctx, image);
+
   rados_ioctx_destroy(io_ctx);
   rados_shutdown(cluster);
 

diff --git a/src/testlibrbdpp.cc b/src/testlibrbdpp.cc
index d4dbfe151e4ffb1d7e2ac0f60a8767624a1f8d86..319ff52b93e562ac4866cbde55d70c1ef3ac6f9b 100644 (file)
--- a/src/testlibrbdpp.cc
+++ b/src/testlibrbdpp.cc
@@ -248,18 +248,23 @@ void test_rbd_copy(librados::IoCtx& io_ctx, librbd::Image& image)
   }
 }
 
-static void print_progress_percent(uint64_t offset, uint64_t src_size,
-                                    void *data)
+class PrintProgress : public librbd::ProgressContext
 {
-  float percent = ((float)offset * 100) / src_size;
-  printf("%3.2f%% done\n", percent);
-}
+public:
+  int update_progress(uint64_t offset, uint64_t src_size)
+  {
+    float percent = ((float)offset * 100) / src_size;
+    printf("%3.2f%% done\n", percent);
+    return 0;
+  }
+};
+
 
 void test_rbd_copy_with_progress(librados::IoCtx& io_ctx, librbd::Image& image)
 {
   int ret;
-  ret = image.copy_with_progress(io_ctx, TEST_IMAGE2,
-                                print_progress_percent, NULL);
+  PrintProgress prog_ctx;
+  ret = image.copy_with_progress(io_ctx, TEST_IMAGE2, prog_ctx);
   if (ret < 0) {
     fprintf(stderr, "image.copy returned %d!\n", ret);
     abort();