OSD: use safe params in map-sharing functions

We were previously using unprotected access to OSD members.

Unfortunately, this does not make them completely safe: we are looking up
maps asynchronously from when we got access to the cached map bounds, and
so the OSD could delete a map out from underneath us. Fixing that will
require some kind of map bounds lock. :/

Signed-off-by: Greg Farnum <greg@inktank.com>

diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index 301d0eb353dbdc33df82546bc160fbc32aece7ea..d86951efc4cadadc0b27f17d057cd2e4a346e8f7 100644 (file)
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -4345,7 +4345,7 @@ void OSD::do_command(Connection *con, ceph_tid_t tid, vector<string>& cmd, buffe
 
          // send them the latest diff to ensure they realize the mapping
          // has changed.
-         send_incremental_map(osdmap->get_epoch() - 1, con);
+         send_incremental_map(osdmap->get_epoch() - 1, con, osdmap);
 
          // do not reply; they will get newer maps and realize they
          // need to resend.
@@ -4667,7 +4667,7 @@ bool OSD::_share_map_incoming(
     if (sendmap) {
       dout(10) << name << " has old map " << epoch
               << " < " << osdmap->get_epoch() << dendl;
-      send_incremental_map(epoch, con);
+      send_incremental_map(epoch, con, osdmap);
       shared = true;
     }
   }
@@ -4686,7 +4686,7 @@ bool OSD::_share_map_incoming(
               << " has old map " << epoch << " < "
               << osdmap->get_epoch() << dendl;
       note_peer_epoch(name.num(), osdmap->get_epoch());
-      send_incremental_map(epoch, con);
+      send_incremental_map(epoch, con, osdmap);
       shared = true;
     }
   }
@@ -4706,7 +4706,7 @@ void OSD::_share_map_outgoing(int peer, Connection *con, OSDMapRef map)
   epoch_t pe = get_peer_epoch(peer);
   if (pe) {
     if (pe < map->get_epoch()) {
-      send_incremental_map(pe, con);
+      send_incremental_map(pe, con, map);
       note_peer_epoch(peer, map->get_epoch());
     } else
       dout(20) << "_share_map_outgoing " << con << " already has epoch " << pe << dendl;
@@ -5984,7 +5984,8 @@ void OSD::activate_map()
 }
 
 
-MOSDMap *OSD::build_incremental_map_msg(epoch_t since, epoch_t to)
+MOSDMap *OSD::build_incremental_map_msg(epoch_t since, epoch_t to,
+                                        OSDSuperblock& superblock)
 {
   MOSDMap *m = new MOSDMap(monc->get_fsid());
   m->oldest_map = superblock.oldest_map;
@@ -6015,7 +6016,8 @@ void OSD::send_map(MOSDMap *m, Connection *con)
   msgr->send_message(m, con);
 }
 
-void OSD::send_incremental_map(epoch_t since, Connection *con)
+void OSD::send_incremental_map(epoch_t since, Connection *con,
+                               OSDMapRef& osdmap)
 {
   epoch_t to = osdmap->get_epoch();
   dout(10) << "send_incremental_map " << since << " -> " << to
@@ -6040,7 +6042,7 @@ void OSD::send_incremental_map(epoch_t since, Connection *con)
 
   if (to - since > (epoch_t)cct->_conf->osd_map_message_max)
     to = since + cct->_conf->osd_map_message_max;
-  MOSDMap *m = build_incremental_map_msg(since, to);
+  MOSDMap *m = build_incremental_map_msg(since, to, superblock);
   send_map(m, con);
 }
 

diff --git a/src/osd/OSD.h b/src/osd/OSD.h
index 25a24e7fc60feffb1d751f424048fded644116bf..8d6982b2c9ec94ba92b5c907541a657b3e336818 100644 (file)
--- a/src/osd/OSD.h
+++ b/src/osd/OSD.h
@@ -1384,8 +1384,9 @@ private:
     return service.get_inc_map_bl(e, bl);
   }
 
-  MOSDMap *build_incremental_map_msg(epoch_t from, epoch_t to);
-  void send_incremental_map(epoch_t since, Connection *con);
+  MOSDMap *build_incremental_map_msg(epoch_t from, epoch_t to,
+                                     OSDSuperblock& superblock);
+  void send_incremental_map(epoch_t since, Connection *con, OSDMapRef& osdmap);
   void send_map(MOSDMap *m, Connection *con);
 
 protected: