crimson/osd: fix stuck recovery

ceph#62080 tested version was **different**
from the one that got merged.
The untested change was changing the boolean returned from start_recovery_ops.
While the seastar::repeat loop in BackgroundRecoveryT<T>::start() was changed accordingly,
other do_recovery() return cases were not considered.

See Tested / Merged here: https://github.com/Matan-B/ceph/pull/2/files

start_recovery_ops used by do_recovery should return whether the iteration (i.e recovery) keep going.

_Note: This has caused a regression in our suite_

Signed-off-by: Matan Breizman <[email protected]>

Author: Matan-B

src/crimson/osd/osd_operations/background_recovery.cc

@@ -82,11 +82,11 @@ seastar::future<> BackgroundRecoveryT<T>::start()
        return do_recovery();
       }, [](std::exception_ptr) {
        return seastar::make_ready_future<bool>(false);
-      }, pg, epoch_started).then([](bool recovery_done) {
-       if (recovery_done) {
-         return seastar::stop_iteration::yes;
-       } else {
+      }, pg, epoch_started).then([](bool do_recovery) {
+       if (do_recovery) {
          return seastar::stop_iteration::no;
+       } else {
+         return seastar::stop_iteration::yes;
        }
       });
     });

src/crimson/osd/pg_recovery.cc

@@ -82,8 +82,8 @@ PGRecovery::start_recovery_ops(
     ceph_assert(pg->is_recovering());
     ceph_assert(!pg->is_backfilling());
 
-    bool done = !pg->get_peering_state().needs_recovery();
-    if (done) {
+    bool do_recovery = pg->get_peering_state().needs_recovery();
+    if (!do_recovery) {
       logger().debug("start_recovery_ops: AllReplicasRecovered for pg: {}",
                      pg->get_pgid());
       using LocalPeeringEvent = crimson::osd::LocalPeeringEvent;
@@ -110,7 +110,7 @@ PGRecovery::start_recovery_ops(
       }
       pg->reset_pglog_based_recovery_op();
     }
-    return seastar::make_ready_future<bool>(done);
+    return seastar::make_ready_future<bool>(do_recovery);
   });
 }