From 9304f93bc63bc499b7e797f90d70d5b07aa3ac9c Mon Sep 17 00:00:00 2001
From: Jason Dillaman <dillaman@redhat.com>
Date: Fri, 30 Oct 2020 10:00:27 -0400
Subject: [PATCH] librbd/migration: close image if failed to open source

To remain consistent between the native which will automatically close
the ImageCtx if it fails to open the image, the raw format should also
close the ImageCtx.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
---
 src/librbd/api/Migration.cc                   | 11 +++++---
 src/librbd/migration/RawFormat.cc             | 12 ++++++--
 .../librbd/migration/test_mock_RawFormat.cc   | 28 +++++++++++++++++++
 3 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/src/librbd/api/Migration.cc b/src/librbd/api/Migration.cc
index fa0aaaa95d2..8093b78b371 100644
--- a/src/librbd/api/Migration.cc
+++ b/src/librbd/api/Migration.cc
@@ -532,18 +532,16 @@ int Migration<I>::prepare_import(
                  << dest_image_name << ", opts=" << opts << dendl;
 
   auto src_image_ctx = I::create("", "", nullptr, dest_io_ctx, true);
-  BOOST_SCOPE_EXIT_TPL(src_image_ctx) {
-    src_image_ctx->state->close();
-  } BOOST_SCOPE_EXIT_END;
+  auto asio_engine = src_image_ctx->asio_engine;
 
   migration::SourceSpecBuilder<I> source_spec_builder(src_image_ctx);
-
   json_spirit::mObject source_spec_object;
   int r = source_spec_builder.parse_source_spec(source_spec,
                                                 &source_spec_object);
   if (r < 0) {
     lderr(cct) << "failed to parse source spec: " << cpp_strerror(r)
                << dendl;
+    src_image_ctx->state->close();
     return r;
   }
 
@@ -552,6 +550,7 @@ int Migration<I>::prepare_import(
   if (r < 0) {
     lderr(cct) << "failed to build migration format handler: "
                << cpp_strerror(r) << dendl;
+    src_image_ctx->state->close();
     return r;
   }
 
@@ -564,6 +563,10 @@ int Migration<I>::prepare_import(
     return r;
   }
 
+  BOOST_SCOPE_EXIT_TPL(src_image_ctx) {
+    src_image_ctx->state->close();
+  } BOOST_SCOPE_EXIT_END;
+
   uint64_t image_format = 2;
   if (opts.get(RBD_IMAGE_OPTION_FORMAT, &image_format) != 0) {
     opts.set(RBD_IMAGE_OPTION_FORMAT, image_format);
diff --git a/src/librbd/migration/RawFormat.cc b/src/librbd/migration/RawFormat.cc
index 2136f6ccf59..582cb664fd8 100644
--- a/src/librbd/migration/RawFormat.cc
+++ b/src/librbd/migration/RawFormat.cc
@@ -5,6 +5,7 @@
 #include "common/dout.h"
 #include "common/errno.h"
 #include "librbd/ImageCtx.h"
+#include "librbd/ImageState.h"
 #include "librbd/Utils.h"
 #include "librbd/io/AioCompletion.h"
 #include "librbd/io/ReadResult.h"
@@ -92,7 +93,13 @@ struct RawFormat<I>::OpenRequest {
     auto cct = raw_format->m_image_ctx->cct;
     ldout(cct, 10) << "r=" << r << dendl;
 
-    on_finish->complete(r);
+    if (r < 0) {
+      raw_format->m_image_ctx->state->close(new LambdaContext(
+        [r, on_finish=on_finish](int _) { on_finish->complete(r); }));
+    } else {
+      on_finish->complete(0);
+    }
+
     delete this;
   }
 };
@@ -118,7 +125,8 @@ void RawFormat<I>::open(Context* on_finish) {
   if (r < 0) {
     lderr(cct) << "failed to build migration stream handler" << cpp_strerror(r)
                << dendl;
-    on_finish->complete(r);
+    m_image_ctx->state->close(
+      new LambdaContext([r, on_finish](int _) { on_finish->complete(r); }));
     return;
   }
 
diff --git a/src/test/librbd/migration/test_mock_RawFormat.cc b/src/test/librbd/migration/test_mock_RawFormat.cc
index b269ac2dc6c..aa04f88449b 100644
--- a/src/test/librbd/migration/test_mock_RawFormat.cc
+++ b/src/test/librbd/migration/test_mock_RawFormat.cc
@@ -105,6 +105,13 @@ public:
         })));
   }
 
+  void expect_close(MockTestImageCtx &mock_image_ctx, int r) {
+    EXPECT_CALL(*mock_image_ctx.state, close(_))
+      .WillOnce(Invoke([this, r](Context* ctx) {
+                  ctx->complete(r);
+                }));
+  }
+
   json_spirit::mObject json_object;
 };
 
@@ -134,6 +141,27 @@ TEST_F(TestMockMigrationRawFormat, OpenClose) {
   ASSERT_EQ(0, ctx2.wait());
 }
 
+TEST_F(TestMockMigrationRawFormat, OpenError) {
+  MockTestImageCtx mock_image_ctx(*m_image_ctx);
+
+  InSequence seq;
+  MockSourceSpecBuilder mock_source_spec_builder;
+
+  auto mock_stream_interface = new MockStreamInterface();
+  expect_build_stream(mock_source_spec_builder, mock_stream_interface, 0);
+
+  expect_stream_open(*mock_stream_interface, -ENOENT);
+
+  expect_close(mock_image_ctx, 0);
+
+  MockRawFormat mock_raw_format(&mock_image_ctx, json_object,
+                                &mock_source_spec_builder);
+
+  C_SaferCond ctx;
+  mock_raw_format.open(&ctx);
+  ASSERT_EQ(-ENOENT, ctx.wait());
+}
+
 TEST_F(TestMockMigrationRawFormat, GetSnapshots) {
   MockTestImageCtx mock_image_ctx(*m_image_ctx);
 
-- 
2.39.5