From ea32e82e6e4a52ef4c6728d36dfc747b053c5edb Mon Sep 17 00:00:00 2001
From: Sage Weil <sage@redhat.com>
Date: Sun, 17 Sep 2017 18:32:45 -0500
Subject: [PATCH] osd/PG: simplify start_recovery_ops return value meaning

Instead of complex check in caller, return simple bool indicating whether
find_unfound() should be called with a rctx.

There is one remaining condition that can probably be simplified:

  if (!recovering.empty() ||
      work_in_progress || recovery_ops_active > 0 || deferred_backfill)
    return !work_in_progress && have_unfound();

but we will leave it for now.

Signed-off-by: Sage Weil <sage@redhat.com>
---
 src/osd/OSD.cc          |  4 ++--
 src/osd/PrimaryLogPG.cc | 10 +++++-----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index 922570babb9187..40ae3a8875a673 100644
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -8837,11 +8837,11 @@ void OSD::do_recovery(
     dout(20) << "  active was " << service.recovery_oids[pg->pg_id] << dendl;
 #endif
 
-    bool wip = pg->start_recovery_ops(reserved_pushes, handle, &started);
+    bool do_unfound = pg->start_recovery_ops(reserved_pushes, handle, &started);
     dout(10) << "do_recovery started " << started << "/" << reserved_pushes 
 	     << " on " << *pg << dendl;
 
-    if (!wip && pg->have_unfound()) {
+    if (do_unfound) {
       PG::RecoveryCtx rctx = create_context();
       rctx.handle = &handle;
       pg->find_unfound(queued, &rctx);
diff --git a/src/osd/PrimaryLogPG.cc b/src/osd/PrimaryLogPG.cc
index c5e403d013246e..43a6703ea9824f 100644
--- a/src/osd/PrimaryLogPG.cc
+++ b/src/osd/PrimaryLogPG.cc
@@ -11079,7 +11079,7 @@ bool PrimaryLogPG::start_recovery_ops(
     /* TODO: I think this case is broken and will make do_recovery()
      * unhappy since we're returning false */
     dout(10) << "recovery raced and were queued twice, ignoring!" << dendl;
-    return false;
+    return have_unfound();
   }
 
   const auto &missing = pg_log.get_missing();
@@ -11144,7 +11144,7 @@ bool PrimaryLogPG::start_recovery_ops(
 
   if (!recovering.empty() ||
       work_in_progress || recovery_ops_active > 0 || deferred_backfill)
-    return work_in_progress;
+    return !work_in_progress && have_unfound();
 
   assert(recovering.empty());
   assert(recovery_ops_active == 0);
@@ -11158,14 +11158,14 @@ bool PrimaryLogPG::start_recovery_ops(
   int unfound = get_num_unfound();
   if (unfound) {
     dout(10) << " still have " << unfound << " unfound" << dendl;
-    return work_in_progress;
+    return true;
   }
 
   if (missing.num_missing() > 0) {
     // this shouldn't happen!
     osd->clog->error() << info.pgid << " Unexpected Error: recovery ending with "
 		       << missing.num_missing() << ": " << missing.get_items();
-    return work_in_progress;
+    return false;
   }
 
   if (needs_recovery()) {
@@ -11173,7 +11173,7 @@ bool PrimaryLogPG::start_recovery_ops(
     // We already checked num_missing() so we must have missing replicas
     osd->clog->error() << info.pgid 
                        << " Unexpected Error: recovery ending with missing replicas";
-    return work_in_progress;
+    return false;
   }
 
   if (state_test(PG_STATE_RECOVERING)) {