librbd: cleanup synchronous open/close memory management

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

diff --git a/src/librbd/ImageState.cc b/src/librbd/ImageState.cc
index dfac0c5b95bb7cb079bddcacd0a1b6e6a7c4e7c8..71a3fdec3da22d58c7a66382cd2df90623a89f51 100644 (file)
--- a/src/librbd/ImageState.cc
+++ b/src/librbd/ImageState.cc
@@ -248,7 +248,12 @@ template <typename I>
 int ImageState<I>::open(bool skip_open_parent) {
   C_SaferCond ctx;
   open(skip_open_parent, &ctx);
-  return ctx.wait();
+
+  int r = ctx.wait();
+  if (r < 0) {
+    delete m_image_ctx;
+  }
+  return r;
 }
 
 template <typename I>
@@ -556,7 +561,7 @@ void ImageState<I>::complete_action_unlock(State next_state, int r) {
     ctx->complete(r);
   }
 
-  if (next_state != STATE_CLOSED) {
+  if (next_state != STATE_UNINITIALIZED && next_state != STATE_CLOSED) {
     m_lock.Lock();
     if (!is_transition_state() && !m_actions_contexts.empty()) {
       execute_next_action_unlock();

diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index 082784a140992654f454baef4a0a20494d8b7347..6f32a9f99967a6478005648484f6085d816a2df7 100644 (file)
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -674,9 +674,9 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
        if (r < 0) {
          lderr(cct) << "error opening image: "
                     << cpp_strerror(r) << dendl;
-          delete imctx;
          return r;
        }
+
        librbd::NoOpProgressContext prog_ctx;
        r = imctx->operations->flatten(prog_ctx);
        if (r < 0) {
@@ -700,7 +700,12 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
            return r;
          }
        }
-       imctx->state->close();
+
+       r = imctx->state->close();
+        if (r < 0) {
+          lderr(cct) << "failed to close image: " << cpp_strerror(r) << dendl;
+          return r;
+        }
       }
       pctx.update_progress(++i, size);
       assert(i <= size);
@@ -1045,7 +1050,6 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
     if (r < 0) {
       lderr(cct) << "error opening parent image: "
                 << cpp_strerror(r) << dendl;
-      delete p_imctx;
       return r;
     }
 
@@ -1186,7 +1190,6 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
     r = c_imctx->state->open(false);
     if (r < 0) {
       lderr(cct) << "Error opening new image: " << cpp_strerror(r) << dendl;
-      delete c_imctx;
       goto err_remove;
     }
 
@@ -1289,7 +1292,6 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
     if (r < 0) {
       lderr(ictx->cct) << "error opening source image: " << cpp_strerror(r)
                       << dendl;
-      delete ictx;
       return r;
     }
     BOOST_SCOPE_EXIT((ictx)) {
@@ -1804,7 +1806,6 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
                                          dest_md_ctx, false);
     r = dest->state->open(false);
     if (r < 0) {
-      delete dest;
       lderr(cct) << "failed to read newly created header" << dendl;
       return r;
     }
@@ -2847,7 +2848,6 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
           if (r < 0) {
             lderr(cct) << "error opening image "<< img_pair.first << ": "
                        << cpp_strerror(r) << dendl;
-            delete img_ctx;
             return r;
           }
 
@@ -2895,22 +2895,19 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
           if (r < 0) {
             lderr(cct) << "error opening image id "<< img_id << ": "
                        << cpp_strerror(r) << dendl;
-            delete img_ctx;
             return r;
           }
 
           r = mirror_image_disable(img_ctx, false);
+          int close_r = img_ctx->state->close();
           if (r < 0) {
             lderr(cct) << "error disabling mirroring for image id " << img_id
                        << cpp_strerror(r) << dendl;
             return r;
-          }
-
-          r = img_ctx->state->close();
-          if (r < 0) {
+          } else if (close_r < 0) {
             lderr(cct) << "failed to close image id " << img_id << ": "
-                       << cpp_strerror(r) << dendl;
-            return r;
+                       << cpp_strerror(close_r) << dendl;
+            return close_r;
           }
         }
       }

diff --git a/src/librbd/librbd.cc b/src/librbd/librbd.cc
index 51b93f14821ddb05256cca83d65762f8d3f074e0..762696b9da07f9edff116cf9ad6f2af6ec237044 100644 (file)
--- a/src/librbd/librbd.cc
+++ b/src/librbd/librbd.cc
@@ -81,16 +81,13 @@ struct C_OpenComplete : public Context {
   void **ictxp;
   bool reopen;
   C_OpenComplete(librbd::ImageCtx *ictx, librbd::io::AioCompletion* comp,
-                void **ictxp, bool reopen = false)
-    : ictx(ictx), comp(comp), ictxp(ictxp), reopen(reopen) {
+                void **ictxp)
+    : ictx(ictx), comp(comp), ictxp(ictxp) {
     comp->init_time(ictx, librbd::io::AIO_TYPE_OPEN);
     comp->get();
   }
   void finish(int r) override {
     ldout(ictx->cct, 20) << "C_OpenComplete::finish: r=" << r << dendl;
-    if (reopen) {
-      delete reinterpret_cast<librbd::ImageCtx*>(*ictxp);
-    }
     if (r < 0) {
       *ictxp = nullptr;
       comp->fail(r);
@@ -115,7 +112,10 @@ struct C_OpenAfterCloseComplete : public Context {
   void finish(int r) override {
     ldout(ictx->cct, 20) << "C_OpenAfterCloseComplete::finish: r=" << r
                         << dendl;
-    ictx->state->open(false, new C_OpenComplete(ictx, comp, ictxp, true));
+    delete reinterpret_cast<librbd::ImageCtx*>(*ictxp);
+    *ictxp = nullptr;
+
+    ictx->state->open(false, new C_OpenComplete(ictx, comp, ictxp));
   }
 };
 
@@ -240,7 +240,6 @@ namespace librbd {
 
     int r = ictx->state->open(false);
     if (r < 0) {
-      delete ictx;
       tracepoint(librbd, open_image_exit, r);
       return r;
     }
@@ -282,7 +281,6 @@ namespace librbd {
 
     int r = ictx->state->open(false);
     if (r < 0) {
-      delete ictx;
       tracepoint(librbd, open_image_exit, r);
       return r;
     }
@@ -2116,9 +2114,7 @@ extern "C" int rbd_open(rados_ioctx_t p, const char *name, rbd_image_t *image,
   tracepoint(librbd, open_image_enter, ictx, ictx->name.c_str(), ictx->id.c_str(), ictx->snap_name.c_str(), ictx->read_only);
 
   int r = ictx->state->open(false);
-  if (r < 0) {
-    delete ictx;
-  } else {
+  if (r >= 0) {
     *image = (rbd_image_t)ictx;
   }
   tracepoint(librbd, open_image_exit, r);
@@ -2152,9 +2148,7 @@ extern "C" int rbd_open_read_only(rados_ioctx_t p, const char *name,
   tracepoint(librbd, open_image_enter, ictx, ictx->name.c_str(), ictx->id.c_str(), ictx->snap_name.c_str(), ictx->read_only);
 
   int r = ictx->state->open(false);
-  if (r < 0) {
-    delete ictx;
-  } else {
+  if (r >= 0) {
     *image = (rbd_image_t)ictx;
   }
   tracepoint(librbd, open_image_exit, r);
@@ -3550,7 +3544,6 @@ extern "C" int rbd_image_get_group(rados_ioctx_t image_p,
   librbd::ImageCtx *ictx = new librbd::ImageCtx(image_name, "", "", io_ctx, false);
   int r = ictx->state->open(false);
   if (r < 0) {
-    delete ictx;
     tracepoint(librbd, open_image_exit, r);
     return r;
   }

diff --git a/src/test/rbd_mirror/test_ImageDeleter.cc b/src/test/rbd_mirror/test_ImageDeleter.cc
index 9893832a4be3148e21e671e26cb1505be2fdf972..c61aaaaf70e70e303756bd9f5e62161ade2a2888 100644 (file)
--- a/src/test/rbd_mirror/test_ImageDeleter.cc
+++ b/src/test/rbd_mirror/test_ImageDeleter.cc
@@ -197,7 +197,6 @@ public:
     ImageCtx *ictx = new ImageCtx("", m_local_image_id, "", m_local_io_ctx,
                                   false);
     EXPECT_EQ(-ENOENT, ictx->state->open(false));
-    delete ictx;
 
     cls::rbd::MirrorImage mirror_image;
     EXPECT_EQ(-ENOENT, cls_client::mirror_image_get(&m_local_io_ctx,

diff --git a/src/tools/rbd_mirror/ImageDeleter.cc b/src/tools/rbd_mirror/ImageDeleter.cc
index 4f8cbc50736eb1f8236f26e243ecf5154bd6f117..fd420872a62b3366c5bf1e235290bbb088cf084e 100644 (file)
--- a/src/tools/rbd_mirror/ImageDeleter.cc
+++ b/src/tools/rbd_mirror/ImageDeleter.cc
@@ -339,7 +339,6 @@ bool ImageDeleter::process_image_delete() {
       derr << "error opening image id " << m_active_delete->local_image_id
            << ": " << cpp_strerror(r) << dendl;
       enqueue_failed_delete(r);
-      delete imgctx;
       return true;
     }