From 5264bc677f0f612165987bf51fe7d7b4af32fa77 Mon Sep 17 00:00:00 2001 From: Joao Eduardo Luis Date: Thu, 7 Jan 2016 19:20:47 +0000 Subject: [PATCH] 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 --- src/mon/OSDMonitor.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc index 30f20017cb71..968efc437598 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; } -- 2.47.3