osd: constify OpRequest::get_req(); fix a few cases of operator<< vs mutated message races

src/common/RefCountedObj.cc

@@ -14,10 +14,10 @@
 
 #include "common/RefCountedObj.h"
 
-void intrusive_ptr_add_ref(RefCountedObject *p) {
+void intrusive_ptr_add_ref(const RefCountedObject *p) {
   p->get();
 }
-void intrusive_ptr_release(RefCountedObject *p) {
+void intrusive_ptr_release(const RefCountedObject *p) {
   p->put();
 }
 

src/common/RefCountedObj.h

@@ -23,14 +23,22 @@
 
 struct RefCountedObject {
 private:
-  atomic_t nref;
+  mutable atomic_t nref;
   CephContext *cct;
 public:
   RefCountedObject(CephContext *c = NULL, int n=1) : nref(n), cct(c) {}
   virtual ~RefCountedObject() {
     assert(nref.read() == 0);
   }
 
+  const RefCountedObject *get() const {
+    int v = nref.inc();
+    if (cct)
+      lsubdout(cct, refs, 1) << "RefCountedObject::get " << this << " "
+			     << (v - 1) << " -> " << v
+			     << dendl;
+    return this;
+  }
   RefCountedObject *get() {
     int v = nref.inc();
     if (cct)
@@ -39,7 +47,7 @@ struct RefCountedObject {
 			     << dendl;
     return this;
   }
-  void put() {
+  void put() const {
     CephContext *local_cct = cct;
     int v = nref.dec();
     if (v == 0) {
@@ -151,7 +159,7 @@ struct RefCountedWaitObject {
   }
 };
 
-void intrusive_ptr_add_ref(RefCountedObject *p);
-void intrusive_ptr_release(RefCountedObject *p);
+void intrusive_ptr_add_ref(const RefCountedObject *p);
+void intrusive_ptr_release(const RefCountedObject *p);
 
 #endif

src/messages/MOSDFailure.h

@@ -59,7 +59,7 @@ class MOSDFailure : public PaxosServiceMessage {
   bool is_immediate() const { 
     return flags & FLAG_IMMEDIATE; 
   }
-  epoch_t get_epoch() { return epoch; }
+  epoch_t get_epoch() const { return epoch; }
 
   void decode_payload() {
     bufferlist::iterator p = payload.begin();

src/messages/MOSDMarkMeDown.h

@@ -40,8 +40,8 @@ class MOSDMarkMeDown : public PaxosServiceMessage {
   ~MOSDMarkMeDown() {}
 
 public: 
-  entity_inst_t get_target() { return target_osd; }
-  epoch_t get_epoch() { return epoch; }
+  entity_inst_t get_target() const { return target_osd; }
+  epoch_t get_epoch() const { return epoch; }
 
   void decode_payload() {
     bufferlist::iterator p = payload.begin();

src/messages/MOSDOp.h

@@ -130,14 +130,14 @@ class MOSDOp : public MOSDFastDispatchOp {
     else
       return object_locator_t(hobj);
   }
-  object_t& get_oid() {
+  const object_t& get_oid() const {
     assert(!final_decode_needed);
     return hobj.oid;
   }
   const hobject_t &get_hobj() const {
     return hobj;
   }
-  snapid_t get_snapid() {
+  snapid_t get_snapid() const {
     assert(!final_decode_needed);
     return hobj.snap;
   }
@@ -231,7 +231,7 @@ class MOSDOp : public MOSDFastDispatchOp {
     add_simple_op(CEPH_OSD_OP_STAT, 0, 0);
   }
 
-  bool has_flag(__u32 flag) { return flags & flag; };
+  bool has_flag(__u32 flag) const { return flags & flag; };
   bool wants_ack() const { return flags & CEPH_OSD_FLAG_ACK; }
   bool wants_ondisk() const { return flags & CEPH_OSD_FLAG_ONDISK; }
   bool wants_onnvram() const { return flags & CEPH_OSD_FLAG_ONNVRAM; }

src/messages/MOSDOpReply.h

@@ -128,7 +128,8 @@ class MOSDOpReply : public Message {
     : Message(CEPH_MSG_OSD_OPREPLY, HEAD_VERSION, COMPAT_VERSION) {
     do_redirect = false;
   }
-  MOSDOpReply(MOSDOp *req, int r, epoch_t e, int acktype, bool ignore_out_data)
+  MOSDOpReply(const MOSDOp *req, int r, epoch_t e, int acktype,
+	      bool ignore_out_data)
     : Message(CEPH_MSG_OSD_OPREPLY, HEAD_VERSION, COMPAT_VERSION),
       oid(req->hobj.oid), pgid(req->pgid.pgid), ops(req->ops) {
 

src/messages/MOSDPGInfo.h

@@ -28,7 +28,7 @@ class MOSDPGInfo : public Message {
 public:
   vector<pair<pg_notify_t,pg_interval_map_t> > pg_list;
 
-  epoch_t get_epoch() { return epoch; }
+  epoch_t get_epoch() const { return epoch; }
 
   MOSDPGInfo()
     : Message(MSG_OSD_PG_INFO, HEAD_VERSION, COMPAT_VERSION) {

src/messages/MOSDPGLog.h

@@ -38,9 +38,9 @@ class MOSDPGLog : public Message {
   pg_missing_t missing;
   pg_interval_map_t past_intervals;
 
-  epoch_t get_epoch() { return epoch; }
-  spg_t get_pgid() { return spg_t(info.pgid.pgid, to); }
-  epoch_t get_query_epoch() { return query_epoch; }
+  epoch_t get_epoch() const { return epoch; }
+  spg_t get_pgid() const { return spg_t(info.pgid.pgid, to); }
+  epoch_t get_query_epoch() const { return query_epoch; }
 
   MOSDPGLog() : Message(MSG_OSD_PG_LOG, HEAD_VERSION, COMPAT_VERSION) { 
     set_priority(CEPH_MSG_PRIO_HIGH); 
@@ -67,6 +67,8 @@ class MOSDPGLog : public Message {
 public:
   const char *get_type_name() const { return "PGlog"; }
   void print(ostream& out) const {
+    // NOTE: log is not const, but operator<< doesn't touch fields
+    // swapped out by OSD code.
     out << "pg_log(" << info.pgid << " epoch " << epoch
 	<< " log " << log
 	<< " query_epoch " << query_epoch << ")";

src/messages/MOSDPGNotify.h

@@ -36,12 +36,14 @@ class MOSDPGNotify : public Message {
   vector<pair<pg_notify_t,pg_interval_map_t> > pg_list;   // pgid -> version
 
  public:
-  version_t get_epoch() { return epoch; }
-  vector<pair<pg_notify_t,pg_interval_map_t> >& get_pg_list() { return pg_list; }
+  version_t get_epoch() const { return epoch; }
+  const vector<pair<pg_notify_t,pg_interval_map_t> >& get_pg_list() const {
+    return pg_list;
+  }
 
   MOSDPGNotify()
     : Message(MSG_OSD_PG_NOTIFY, HEAD_VERSION, COMPAT_VERSION) { 
-    set_priority(CEPH_MSG_PRIO_HIGH); 
+    set_priority(CEPH_MSG_PRIO_HIGH);
   }
   MOSDPGNotify(epoch_t e, vector<pair<pg_notify_t,pg_interval_map_t> >& l)
     : Message(MSG_OSD_PG_NOTIFY, HEAD_VERSION, COMPAT_VERSION),

src/messages/MOSDPGPull.h

@@ -21,12 +21,12 @@ class MOSDPGPull : public MOSDFastDispatchOp {
   static const int HEAD_VERSION = 2;
   static const int COMPAT_VERSION = 1;
 
+  vector<PullOp> pulls;
 
 public:
   pg_shard_t from;
   spg_t pgid;
   epoch_t map_epoch;
-  vector<PullOp> pulls;
   uint64_t cost;
 
   epoch_t get_map_epoch() const override {
@@ -36,6 +36,13 @@ class MOSDPGPull : public MOSDFastDispatchOp {
     return pgid;
   }
 
+  void take_pulls(vector<PullOp> *outpulls) {
+    outpulls->swap(pulls);
+  }
+  void set_pulls(vector<PullOp> *inpulls) {
+    inpulls->swap(pulls);
+  }
+
   MOSDPGPull()
     : MOSDFastDispatchOp(MSG_OSD_PG_PULL, HEAD_VERSION, COMPAT_VERSION),
       cost(0)
@@ -82,9 +89,9 @@ class MOSDPGPull : public MOSDFastDispatchOp {
 
   void print(ostream& out) const {
     out << "MOSDPGPull(" << pgid
-	<< " " << map_epoch
-	<< " " << pulls;
-    out << ")";
+	<< " e" << map_epoch
+	<< " cost " << cost
+	<< ")";
   }
 };
 

src/messages/MOSDPGQuery.h

@@ -29,7 +29,7 @@ class MOSDPGQuery : public Message {
   version_t       epoch;
 
  public:
-  version_t get_epoch() { return epoch; }
+  version_t get_epoch() const { return epoch; }
   map<spg_t, pg_query_t>  pg_list;
 
   MOSDPGQuery() : Message(MSG_OSD_PG_QUERY,

src/messages/MOSDPGRemove.h

@@ -30,7 +30,7 @@ class MOSDPGRemove : public Message {
  public:
   vector<spg_t> pg_list;
 
-  epoch_t get_epoch() { return epoch; }
+  epoch_t get_epoch() const { return epoch; }
 
   MOSDPGRemove() :
     Message(MSG_OSD_PG_REMOVE, HEAD_VERSION, COMPAT_VERSION) {}

src/messages/MOSDPGTrim.h

@@ -27,7 +27,7 @@ class MOSDPGTrim : public Message {
   spg_t pgid;
   eversion_t trim_to;
 
-  epoch_t get_epoch() { return epoch; }
+  epoch_t get_epoch() const { return epoch; }
 
   MOSDPGTrim() : Message(MSG_OSD_PG_TRIM, HEAD_VERSION, COMPAT_VERSION) {}
   MOSDPGTrim(version_t mv, spg_t p, eversion_t tt) :

src/messages/MOSDRepOpReply.h

@@ -95,11 +95,11 @@ class MOSDRepOpReply : public MOSDFastDispatchOp {
   int get_result() { return result; }
 
   void set_last_complete_ondisk(eversion_t v) { last_complete_ondisk = v; }
-  eversion_t get_last_complete_ondisk() { return last_complete_ondisk; }
+  eversion_t get_last_complete_ondisk() const { return last_complete_ondisk; }
 
 public:
   MOSDRepOpReply(
-    MOSDRepOp *req, pg_shard_t from, int result_, epoch_t e, int at) :
+    const MOSDRepOp *req, pg_shard_t from, int result_, epoch_t e, int at) :
     MOSDFastDispatchOp(MSG_OSD_REPOPREPLY, HEAD_VERSION, COMPAT_VERSION),
     map_epoch(e),
     reqid(req->reqid),

src/messages/MOSDSubOpReply.h

@@ -116,8 +116,8 @@ class MOSDSubOpReply : public MOSDFastDispatchOp {
 
   epoch_t get_map_epoch() { return map_epoch; }
 
-  spg_t get_pg() { return pgid; }
-  hobject_t get_poid() { return poid; }
+  spg_t get_pg() const { return pgid; }
+  const hobject_t& get_poid() const { return poid; }
 
   int get_ack_type() { return ack_type; }
   bool is_ondisk() { return ack_type & CEPH_OSD_FLAG_ONDISK; }
@@ -136,7 +136,7 @@ class MOSDSubOpReply : public MOSDFastDispatchOp {
 
 public:
   MOSDSubOpReply(
-    MOSDSubOp *req, pg_shard_t from, int result_, epoch_t e, int at)
+    const MOSDSubOp *req, pg_shard_t from, int result_, epoch_t e, int at)
     : MOSDFastDispatchOp(MSG_OSD_SUBOPREPLY, HEAD_VERSION, COMPAT_VERSION),
       map_epoch(e),
       reqid(req->reqid),

src/msg/Message.h

@@ -376,6 +376,7 @@ class Message : public RefCountedObject {
       byte_throttler->take(data.length());
   }
 
+  const bufferlist& get_data() const { return data; }
   bufferlist& get_data() { return data; }
   void claim_data(bufferlist& bl,
 		  unsigned int flags = buffer::list::CLAIM_DEFAULT) {

src/os/ObjectStore.h

@@ -1114,7 +1114,7 @@ class ObjectStore {
       data.ops++;
     }
     /// Set multiple xattrs of an object
-    void setattrs(const coll_t& cid, const ghobject_t& oid, map<string,bufferptr>& attrset) {
+    void setattrs(const coll_t& cid, const ghobject_t& oid, const map<string,bufferptr>& attrset) {
       Op* _op = _get_next_op();
       _op->op = OP_SETATTRS;
       _op->cid = _get_coll_id(cid);
@@ -1123,7 +1123,7 @@ class ObjectStore {
       data.ops++;
     }
     /// Set multiple xattrs of an object
-    void setattrs(const coll_t& cid, const ghobject_t& oid, map<string,bufferlist>& attrset) {
+    void setattrs(const coll_t& cid, const ghobject_t& oid, const map<string,bufferlist>& attrset) {
       Op* _op = _get_next_op();
       _op->op = OP_SETATTRS;
       _op->cid = _get_coll_id(cid);