From: Jason Dillaman <dillaman@redhat.com>
Date: Thu, 2 Jul 2015 16:39:42 +0000 (-0400)
Subject: librbd: closing images now returns a result code
X-Git-Tag: v9.0.2~3^2~2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=7ef4af5d33c9936ae0d9227e42d892faedeb90ea;p=ceph.git

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 cf8b2e7b4e7..e5879164f28 100644
--- 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 7f56c1ec95c..eefd0df34c1 100644
--- 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 e0b5cb477ab..6a3b24e9328 100644
--- 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 9316e473a25..46917c8c350 100644
--- 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 d738bb0e915..7eaa3c5614f 100644
--- 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 7d18efb5c8c..e802e9ce9e5 100644
--- 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 c88a4f5bcdc..f9101bd92e3 100644
--- 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,