librbd: closing images now returns a result code

Added new librbd::Image::close method to allow checking the close result
when using the C++ librbd library.  rbd_close is no longer hard-coded to
return 0.

Fixes: #12069
Signed-off-by: Jason Dillaman <dillaman@redhat.com>

diff --git a/src/include/rbd/librbd.hpp b/src/include/rbd/librbd.hpp
index cf8b2e7b4e72a2cf9d3ca84e41c7d7ee78c1f494..e5879164f28c1eee45e702a0a82818f90f625837 100644 (file)
--- a/src/include/rbd/librbd.hpp
+++ b/src/include/rbd/librbd.hpp
@@ -108,6 +108,8 @@ public:
   Image();
   ~Image();
 
+  int close();
+
   int resize(uint64_t size);
   int resize_with_progress(uint64_t size, ProgressContext& pctx);
   int stat(image_info_t &info, size_t infosize);

diff --git a/src/librbd/ImageCtx.cc b/src/librbd/ImageCtx.cc
index 7f56c1ec95c5ff7ebac1cf6f783a7249ae492d7c..eefd0df34c1337991dac91da4da6907c80f6ec9c 100644 (file)
--- a/src/librbd/ImageCtx.cc
+++ b/src/librbd/ImageCtx.cc
@@ -671,12 +671,13 @@ public:
     return r;
   }
 
-  void ImageCtx::shutdown_cache() {
+  int ImageCtx::shutdown_cache() {
     flush_async_operations();
 
     RWLock::RLocker owner_locker(owner_lock);
-    invalidate_cache(true);
+    int r = invalidate_cache(true);
     object_cacher->stop();
+    return r;
   }
 
   int ImageCtx::invalidate_cache(bool purge_on_error) {

diff --git a/src/librbd/ImageCtx.h b/src/librbd/ImageCtx.h
index e0b5cb477ab7dd6b930f512115c78d162925303c..6a3b24e9328226e3db772997dbd58c8e4d75b3a6 100644 (file)
--- a/src/librbd/ImageCtx.h
+++ b/src/librbd/ImageCtx.h
@@ -219,7 +219,7 @@ namespace librbd {
     void user_flushed();
     void flush_cache_aio(Context *onfinish);
     int flush_cache();
-    void shutdown_cache();
+    int shutdown_cache();
     int invalidate_cache(bool purge_on_error=false);
     void invalidate_cache(Context *on_finish);
     void invalidate_cache_completion(int r, Context *on_finish);

diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index 9316e473a25c244cbc67e81833c8aa079b7e92e3..46917c8c350adb4a2abddd270d362ee26b03f44b 100644 (file)
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -1360,7 +1360,7 @@ reprotect_and_return_err:
     int order;
     uint64_t size;
     uint64_t p_features;
-    int remove_r;
+    int partial_r;
     librbd::NoOpProgressContext no_op;
     ImageCtx *c_imctx = NULL;
     map<string, bufferlist> pairs;
@@ -1458,24 +1458,27 @@ reprotect_and_return_err:
     }
 
     ldout(cct, 2) << "done." << dendl;
-    close_image(c_imctx);
-    close_image(p_imctx);
-    return 0;
+    r = close_image(c_imctx);
+    partial_r = close_image(p_imctx);
+    if (r == 0 && partial_r < 0) {
+      r = partial_r;
+    }
+    return r;
 
   err_remove_child:
-    remove_r = cls_client::remove_child(&c_ioctx, RBD_CHILDREN, pspec,
-                                       c_imctx->id);
-    if (remove_r < 0) {
+    partial_r = cls_client::remove_child(&c_ioctx, RBD_CHILDREN, pspec,
+                                         c_imctx->id);
+    if (partial_r < 0) {
      lderr(cct) << "Error removing failed clone from list of children: "
-                << cpp_strerror(remove_r) << dendl;
+                << cpp_strerror(partial_r) << dendl;
     }
   err_close_child:
     close_image(c_imctx);
   err_remove:
-    remove_r = remove(c_ioctx, c_name, no_op);
-    if (remove_r < 0) {
+    partial_r = remove(c_ioctx, c_name, no_op);
+    if (partial_r < 0) {
       lderr(cct) << "Error removing failed clone: "
-                << cpp_strerror(remove_r) << dendl;
+                << cpp_strerror(partial_r) << dendl;
     }
   err_close_parent:
     close_image(p_imctx);
@@ -2600,7 +2603,10 @@ reprotect_and_return_err:
     }
 
     r = copy(src, dest, prog_ctx);
-    close_image(dest);
+    int close_r = close_image(dest);
+    if (r == 0 && close_r < 0) {
+      r = close_r;
+    }
     return r;
   }
 
@@ -2814,7 +2820,7 @@ reprotect_and_return_err:
     return r;
   }
 
-  void close_image(ImageCtx *ictx)
+  int close_image(ImageCtx *ictx)
   {
     ldout(ictx->cct, 20) << "close_image " << ictx << dendl;
 
@@ -2831,10 +2837,15 @@ reprotect_and_return_err:
     ictx->flush_async_operations();
     ictx->readahead.wait_for_pending();
 
+    int r;
     if (ictx->object_cacher) {
-      ictx->shutdown_cache(); // implicitly flushes
+      r = ictx->shutdown_cache(); // implicitly flushes
     } else {
-      flush(ictx);
+      r = flush(ictx);
+    }
+    if (r < 0) {
+      lderr(ictx->cct) << "error flushing IO: " << cpp_strerror(r)
+                       << dendl;
     }
 
     ictx->op_work_queue->drain();
@@ -2845,7 +2856,10 @@ reprotect_and_return_err:
     }
 
     if (ictx->parent) {
-      close_image(ictx->parent);
+      int close_r = close_image(ictx->parent);
+      if (r == 0 && close_r < 0) {
+        r = close_r;
+      }
       ictx->parent = NULL;
     }
 
@@ -2853,10 +2867,13 @@ reprotect_and_return_err:
       {
        RWLock::WLocker l(ictx->owner_lock);
        if (ictx->image_watcher->is_lock_owner()) {
-         int r = ictx->image_watcher->unlock();
-         if (r < 0) {
-           lderr(ictx->cct) << "error unlocking image: " << cpp_strerror(r)
-                            << dendl;
+         int unlock_r = ictx->image_watcher->unlock();
+         if (unlock_r < 0) {
+           lderr(ictx->cct) << "error unlocking image: "
+                             << cpp_strerror(unlock_r) << dendl;
+            if (r == 0) {
+              r = unlock_r;
+            }
          }
        }
       }
@@ -2864,6 +2881,7 @@ reprotect_and_return_err:
     }
 
     delete ictx;
+    return r;
   }
 
   // 'flatten' child image by copying all parent's blocks

diff --git a/src/librbd/internal.h b/src/librbd/internal.h
index d738bb0e915a89526077810f00ca70274efd64d7..7eaa3c5614fec2fc1e3d68ca0892a2b1b06dab60 100644 (file)
--- a/src/librbd/internal.h
+++ b/src/librbd/internal.h
@@ -125,7 +125,7 @@ namespace librbd {
 
   int open_parent(ImageCtx *ictx);
   int open_image(ImageCtx *ictx);
-  void close_image(ImageCtx *ictx);
+  int close_image(ImageCtx *ictx);
 
   int copyup_block(ImageCtx *ictx, uint64_t offset, size_t len,
                   const char *buf);

diff --git a/src/librbd/librbd.cc b/src/librbd/librbd.cc
index 7d18efb5c8cc35bf3766ec3546460eba2c4afbc7..e802e9ce9e573ac3737a8f76359a9897f9c8a5d9 100644 (file)
--- a/src/librbd/librbd.cc
+++ b/src/librbd/librbd.cc
@@ -388,12 +388,20 @@ namespace librbd {
 
   Image::~Image()
   {
+    close();
+  }
+
+  int Image::close()
+  {
+    int r = 0;
     if (ctx) {
       ImageCtx *ictx = (ImageCtx *)ctx;
       tracepoint(librbd, close_image_enter, ictx, ictx->name.c_str(), ictx->id.c_str());
-      close_image(ictx);
-      tracepoint(librbd, close_image_exit);
+      r = close_image(ictx);
+      ctx = NULL;
+      tracepoint(librbd, close_image_exit, r);
     }
+    return r;
   }
 
   int Image::resize(uint64_t size)
@@ -1256,9 +1264,9 @@ extern "C" int rbd_close(rbd_image_t image)
 {
   librbd::ImageCtx *ctx = (librbd::ImageCtx *)image;
   tracepoint(librbd, close_image_enter, ctx, ctx->name.c_str(), ctx->id.c_str());
-  librbd::close_image(ctx);
-  tracepoint(librbd, close_image_exit);
-  return 0;
+  int r = librbd::close_image(ctx);
+  tracepoint(librbd, close_image_exit, r);
+  return r;
 }
 
 extern "C" int rbd_resize(rbd_image_t image, uint64_t size)

diff --git a/src/tracing/librbd.tp b/src/tracing/librbd.tp
index c88a4f5bcdcadf9646c722f9732b2d931743e40b..f9101bd92e3cdfcee155cda43f33c4bd6da77514 100644 (file)
--- a/src/tracing/librbd.tp
+++ b/src/tracing/librbd.tp
@@ -185,8 +185,10 @@ TRACEPOINT_EVENT(librbd, close_image_enter,
 )
 
 TRACEPOINT_EVENT(librbd, close_image_exit,
-    TP_ARGS(),
-    TP_FIELDS()
+    TP_ARGS(
+       int, retval),
+    TP_FIELDS(
+       ctf_integer(int, retval, retval))
 )
 
 TRACEPOINT_EVENT(librbd, list_enter,