From: Joao Eduardo Luis <joao@suse.de>
Date: Thu, 7 Jan 2016 19:20:47 +0000 (+0000)
Subject: mon: OSDMonitor: do not assume a session exists in send_incremental()
X-Git-Tag: v0.94.6~46^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=refs%2Fpull%2F7150%2Fhead;p=ceph.git

mon: OSDMonitor: do not assume a session exists in send_incremental()

We may not have an open session for a given osd. If we blindly assume we
do, we may end up trying to send incrementals we do not have to the osd.

And then we will crash.

This fixes a regression introduced by

  171fee1b82d2675e364da7f96dfb9dd286d9b6e6

which is meant as a backport of

  de43a02e06650a552f048dc8acd17f255126fed9

but so happens to intruduce a line that wasn't on the original patch. We
imagine it was meant to make the 's->osd_epoch' assignment work without
checking the session, as per the original patch, but the backporter must
have forgotten to also backport the assertion on the not-null session.
The unfortunate introduction of the check for a not-null session
triggered this regression.

The regression itself is due to enforcing that a session exists for the
osd we are sending the incrementals to. However, if we come via the
OSDMonitor::process_failures() path, that may very well not be the case,
as we are handling potentially-old MOSDFailure messages that may no
longer have an associated session. By enforcing the not-null session, we
don't check whether we have the requested versions (i.e., if
our_earliest_version <= requested_version), and thus we end up on the
path that assumes that we DO HAVE all the necessary versions -- when we
may not, thus finally asserting because we are reading blank
incremental versions.

Fixes: #14236

Signed-off-by: Joao Eduardo Luis <joao@suse.de>
---

diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc
index 30f20017cb7..968efc43759 100644
--- a/src/mon/OSDMonitor.cc
+++ b/src/mon/OSDMonitor.cc
@@ -2148,7 +2148,7 @@ void OSDMonitor::send_incremental(PaxosServiceMessage *req, epoch_t first)
       return;
   }
 
-  if (s && first < get_first_committed()) {
+  if (first < get_first_committed()) {
     first = get_first_committed();
     bufferlist bl;
     int err = get_version_full(first, bl);
@@ -2164,7 +2164,9 @@ void OSDMonitor::send_incremental(PaxosServiceMessage *req, epoch_t first)
     m->maps[first] = bl;
     mon->send_reply(req, m);
 
-    s->osd_epoch = osdmap.get_epoch();
+    if (s) {
+      s->osd_epoch = osdmap.get_epoch();
+    }
     return;
   }