From: Ilya Dryomov <idryomov@gmail.com>
Date: Tue, 30 Aug 2022 09:45:44 +0000 (+0200)
Subject: rbd-mirror: skip setting error code on snapshot replayer shutdown
X-Git-Tag: v18.0.0~139^2
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=ced071f0de57af8cddffebca24baeb27f2a211d8;p=ceph-ci.git

rbd-mirror: skip setting error code on snapshot replayer shutdown

This is regarding failures in unregister_remote_update_watcher() and
unregister_local_update_watcher().  handle_replay_complete() can't be
called in these cases anymore as it would blindly attempt to unregister
watchers from scratch again.  Dropping handle_replay_complete() calls
there means that these failures would only be logged and would not be
surfaced by snapshot replayer.  But the only caller ignores them
anyway:

  void ImageReplayer<I>::shut_down(int r) {
    ...
    // close the replayer
    if (m_replayer != nullptr) {
      ctx = new LambdaContext([this, ctx](int r) {
        m_replayer->destroy();
        m_replayer = nullptr;
        ctx->complete(0);             <------
      });
      ctx = new LambdaContext([this, ctx](int r) {
        m_replayer->shut_down(ctx);
      });
    }

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
---

diff --git a/src/test/rbd_mirror/image_replayer/snapshot/test_mock_Replayer.cc b/src/test/rbd_mirror/image_replayer/snapshot/test_mock_Replayer.cc
index e1ee953e7f6..26b4d32cc4f 100644
--- a/src/test/rbd_mirror/image_replayer/snapshot/test_mock_Replayer.cc
+++ b/src/test/rbd_mirror/image_replayer/snapshot/test_mock_Replayer.cc
@@ -1908,7 +1908,7 @@ TEST_F(TestMockImageReplayerSnapshotReplayer, UnregisterRemoteUpdateWatcherError
 
   C_SaferCond shutdown_ctx;
   mock_replayer.shut_down(&shutdown_ctx);
-  ASSERT_EQ(-EINVAL, shutdown_ctx.wait());
+  ASSERT_EQ(0, shutdown_ctx.wait());
 }
 
 TEST_F(TestMockImageReplayerSnapshotReplayer, UnregisterLocalUpdateWatcherError) {
@@ -1950,7 +1950,7 @@ TEST_F(TestMockImageReplayerSnapshotReplayer, UnregisterLocalUpdateWatcherError)
 
   C_SaferCond shutdown_ctx;
   mock_replayer.shut_down(&shutdown_ctx);
-  ASSERT_EQ(-EINVAL, shutdown_ctx.wait());
+  ASSERT_EQ(0, shutdown_ctx.wait());
 }
 
 TEST_F(TestMockImageReplayerSnapshotReplayer, LoadImageMetaError) {
diff --git a/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.cc b/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.cc
index 55c63d5f982..6e180f0cd25 100644
--- a/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.cc
+++ b/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.cc
@@ -1402,8 +1402,6 @@ void Replayer<I>::handle_unregister_remote_update_watcher(int r) {
   if (r < 0) {
     derr << "failed to unregister remote update watcher: " << cpp_strerror(r)
          << dendl;
-    handle_replay_complete(
-      r, "failed to unregister remote image update watcher");
   }
 
   unregister_local_update_watcher();
@@ -1427,8 +1425,6 @@ void Replayer<I>::handle_unregister_local_update_watcher(int r) {
   if (r < 0) {
     derr << "failed to unregister local update watcher: " << cpp_strerror(r)
          << dendl;
-    handle_replay_complete(
-      r, "failed to unregister local image update watcher");
   }
 
   delete m_update_watch_ctx;