From: Sage Weil <sage@redhat.com>
Date: Tue, 9 Oct 2018 22:08:46 +0000 (-0500)
Subject: mon: use MonOpRequest get_session() instead of PaxosServiceMessage's
X-Git-Tag: v14.0.1~26^2~18
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=ec282c45a71dc9db33e570d9e7da6455adb30f70;p=ceph.git

mon: use MonOpRequest get_session() instead of PaxosServiceMessage's

The PaxosServiceMessage method relies on the msg -> con -> session linkage,
and the con -> session link is not present for forwarded messages.  Also,
the message path is redundant and unnecessary.

Signed-off-by: Sage Weil <sage@redhat.com>
---

diff --git a/src/messages/PaxosServiceMessage.h b/src/messages/PaxosServiceMessage.h
index 3532e3eb4f6a..a947b548ee04 100644
--- a/src/messages/PaxosServiceMessage.h
+++ b/src/messages/PaxosServiceMessage.h
@@ -50,19 +50,6 @@ public:
     paxos_decode(p);
   }
 
-  /** 
-   * These messages are only used by the monitors and clients,
-   * and the client doesn't care, so we're creating a monitor-specific
-   * function here. Note that this function explicitly exists to bypass
-   * the normal ref-counting, so don't expect the returned pointer to be
-   * very long-lived -- it will still only last as long as the Session would
-   * normally.
-   */
-  MonSession *get_session() {
-    auto priv = get_connection()->get_priv();
-    return static_cast<MonSession*>(priv.get());
-  }
-  
   const char *get_type_name() const override { return "PaxosServiceMessage"; }
 };
 
diff --git a/src/mon/AuthMonitor.cc b/src/mon/AuthMonitor.cc
index 2c28a972b091..9ad3de87fb1a 100644
--- a/src/mon/AuthMonitor.cc
+++ b/src/mon/AuthMonitor.cc
@@ -703,7 +703,7 @@ bool AuthMonitor::preprocess_command(MonOpRequestRef op)
     return false;
   }
 
-  MonSession *session = m->get_session();
+  MonSession *session = op->get_session();
   if (!session) {
     mon->reply_command(op, -EACCES, "access denied", rdata, get_last_committed());
     return true;
@@ -1227,7 +1227,7 @@ bool AuthMonitor::prepare_command(MonOpRequestRef op)
   cmd_getval(g_ceph_context, cmdmap, "format", format, string("plain"));
   boost::scoped_ptr<Formatter> f(Formatter::create(format));
 
-  MonSession *session = m->get_session();
+  MonSession *session = op->get_session();
   if (!session) {
     mon->reply_command(op, -EACCES, "access denied", rdata, get_last_committed());
     return true;
diff --git a/src/mon/LogMonitor.cc b/src/mon/LogMonitor.cc
index 8ca3bab0fc5a..85a982ba50ea 100644
--- a/src/mon/LogMonitor.cc
+++ b/src/mon/LogMonitor.cc
@@ -304,7 +304,7 @@ bool LogMonitor::preprocess_log(MonOpRequestRef op)
   dout(10) << "preprocess_log " << *m << " from " << m->get_orig_source() << dendl;
   int num_new = 0;
 
-  MonSession *session = m->get_session();
+  MonSession *session = op->get_session();
   if (!session)
     goto done;
   if (!session->is_capable("log", MON_CAP_W)) {
@@ -402,7 +402,7 @@ bool LogMonitor::preprocess_command(MonOpRequestRef op)
     mon->reply_command(op, -EINVAL, rs, get_last_committed());
     return true;
   }
-  MonSession *session = m->get_session();
+  MonSession *session = op->get_session();
   if (!session) {
     mon->reply_command(op, -EACCES, "access denied", get_last_committed());
     return true;
@@ -532,7 +532,7 @@ bool LogMonitor::prepare_command(MonOpRequestRef op)
   string prefix;
   cmd_getval(g_ceph_context, cmdmap, "prefix", prefix);
 
-  MonSession *session = m->get_session();
+  MonSession *session = op->get_session();
   if (!session) {
     mon->reply_command(op, -EACCES, "access denied", get_last_committed());
     return true;
diff --git a/src/mon/MDSMonitor.cc b/src/mon/MDSMonitor.cc
index 1e2351f70a11..169225c9091e 100644
--- a/src/mon/MDSMonitor.cc
+++ b/src/mon/MDSMonitor.cc
@@ -327,7 +327,7 @@ bool MDSMonitor::preprocess_beacon(MonOpRequestRef op)
   const auto &fsmap = get_fsmap();
 
   // check privileges, ignore if fails
-  MonSession *session = m->get_session();
+  MonSession *session = op->get_session();
   if (!session)
     goto ignore;
   if (!session->is_capable("mds", MON_CAP_X)) {
@@ -473,7 +473,7 @@ bool MDSMonitor::preprocess_offload_targets(MonOpRequestRef op)
   const auto &fsmap = get_fsmap();
   
   // check privileges, ignore message if fails
-  MonSession *session = m->get_session();
+  MonSession *session = op->get_session();
   if (!session)
     goto ignore;
   if (!session->is_capable("mds", MON_CAP_X)) {
@@ -904,7 +904,7 @@ bool MDSMonitor::preprocess_command(MonOpRequestRef op)
   cmd_getval(g_ceph_context, cmdmap, "format", format, string("plain"));
   std::unique_ptr<Formatter> f(Formatter::create(format));
 
-  MonSession *session = m->get_session();
+  MonSession *session = op->get_session();
   if (!session) {
     mon->reply_command(op, -EACCES, "access denied", rdata, get_last_committed());
     return true;
@@ -1215,7 +1215,7 @@ bool MDSMonitor::prepare_command(MonOpRequestRef op)
   cmd_getval(g_ceph_context, cmdmap, "prefix", prefix);
 
   /* Refuse access if message not associated with a valid session */
-  MonSession *session = m->get_session();
+  MonSession *session = op->get_session();
   if (!session) {
     mon->reply_command(op, -EACCES, "access denied", rdata, get_last_committed());
     return true;
diff --git a/src/mon/MgrMonitor.cc b/src/mon/MgrMonitor.cc
index 39442c05dc92..df053ce87e46 100644
--- a/src/mon/MgrMonitor.cc
+++ b/src/mon/MgrMonitor.cc
@@ -744,7 +744,7 @@ bool MgrMonitor::preprocess_command(MonOpRequestRef op)
     return true;
   }
 
-  MonSession *session = m->get_session();
+  MonSession *session = op->get_session();
   if (!session) {
     mon->reply_command(op, -EACCES, "access denied", rdata,
 		       get_last_committed());
@@ -887,7 +887,7 @@ bool MgrMonitor::prepare_command(MonOpRequestRef op)
     return true;
   }
 
-  MonSession *session = m->get_session();
+  MonSession *session = op->get_session();
   if (!session) {
     mon->reply_command(op, -EACCES, "access denied", rdata, get_last_committed());
     return true;
diff --git a/src/mon/MgrStatMonitor.cc b/src/mon/MgrStatMonitor.cc
index ab87ab42edc7..caf1dcb04a86 100644
--- a/src/mon/MgrStatMonitor.cc
+++ b/src/mon/MgrStatMonitor.cc
@@ -212,7 +212,7 @@ bool MgrStatMonitor::preprocess_getpoolstats(MonOpRequestRef op)
 {
   op->mark_pgmon_event(__func__);
   auto m = static_cast<MGetPoolStats*>(op->get_req());
-  auto session = m->get_session();
+  auto session = op->get_session();
   if (!session)
     return true;
   if (!session->is_capable("pg", MON_CAP_R)) {
@@ -244,7 +244,7 @@ bool MgrStatMonitor::preprocess_statfs(MonOpRequestRef op)
 {
   op->mark_pgmon_event(__func__);
   auto statfs = static_cast<MStatfs*>(op->get_req());
-  auto session = statfs->get_session();
+  auto session = op->get_session();
 
   if (!session)
     return true;
diff --git a/src/mon/MonmapMonitor.cc b/src/mon/MonmapMonitor.cc
index 667fdf2d4a19..ff2c8d753952 100644
--- a/src/mon/MonmapMonitor.cc
+++ b/src/mon/MonmapMonitor.cc
@@ -247,7 +247,7 @@ bool MonmapMonitor::preprocess_command(MonOpRequestRef op)
   string prefix;
   cmd_getval(g_ceph_context, cmdmap, "prefix", prefix);
 
-  MonSession *session = m->get_session();
+  MonSession *session = op->get_session();
   if (!session) {
     mon->reply_command(op, -EACCES, "access denied", get_last_committed());
     return true;
@@ -445,7 +445,7 @@ bool MonmapMonitor::prepare_command(MonOpRequestRef op)
   string prefix;
   cmd_getval(g_ceph_context, cmdmap, "prefix", prefix);
 
-  MonSession *session = m->get_session();
+  MonSession *session = op->get_session();
   if (!session) {
     mon->reply_command(op, -EACCES, "access denied", get_last_committed());
     return true;
@@ -725,7 +725,7 @@ bool MonmapMonitor::preprocess_join(MonOpRequestRef op)
   MMonJoin *join = static_cast<MMonJoin*>(op->get_req());
   dout(10) << __func__ << " " << join->name << " at " << join->addr << dendl;
 
-  MonSession *session = join->get_session();
+  MonSession *session = op->get_session();
   if (!session ||
       !session->is_capable("mon", MON_CAP_W | MON_CAP_X)) {
     dout(10) << " insufficient caps" << dendl;
diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc
index baf1dcaf3820..471855cf68eb 100644
--- a/src/mon/OSDMonitor.cc
+++ b/src/mon/OSDMonitor.cc
@@ -2217,8 +2217,8 @@ bool OSDMonitor::preprocess_get_osdmap(MonOpRequestRef op)
   MMonGetOSDMap *m = static_cast<MMonGetOSDMap*>(op->get_req());
 
   uint64_t features = mon->get_quorum_con_features();
-  if (m->get_session() && m->get_session()->con_features)
-    features = m->get_session()->con_features;
+  if (op->get_session() && op->get_session()->con_features)
+    features = op->get_session()->con_features;
 
   dout(10) << __func__ << " " << *m << dendl;
   MOSDMap *reply = new MOSDMap(mon->monmap->fsid, features);
@@ -2249,9 +2249,9 @@ bool OSDMonitor::preprocess_get_osdmap(MonOpRequestRef op)
 
 // failure --
 
-bool OSDMonitor::check_source(PaxosServiceMessage *m, uuid_d fsid) {
+bool OSDMonitor::check_source(MonOpRequestRef op, uuid_d fsid) {
   // check permissions
-  MonSession *session = m->get_session();
+  MonSession *session = op->get_session();
   if (!session)
     return true;
   if (!session->is_capable("osd", MON_CAP_X)) {
@@ -2276,7 +2276,7 @@ bool OSDMonitor::preprocess_failure(MonOpRequestRef op)
   int badboy = m->get_target_osd();
 
   // check permissions
-  if (check_source(m, m->fsid))
+  if (check_source(op, m->fsid))
     goto didit;
 
   // first, verify the reporting host is valid
@@ -2370,7 +2370,7 @@ bool OSDMonitor::preprocess_mark_me_down(MonOpRequestRef op)
   int from = m->target_osd;
 
   // check permissions
-  if (check_source(m, m->fsid))
+  if (check_source(op, m->fsid))
     goto reply;
 
   // first, verify the reporting host is valid
@@ -2755,7 +2755,7 @@ bool OSDMonitor::preprocess_boot(MonOpRequestRef op)
   int from = m->get_orig_source_inst().name.num();
 
   // check permissions, ignore if failed (no response expected)
-  MonSession *session = m->get_session();
+  MonSession *session = op->get_session();
   if (!session)
     goto ignore;
   if (!session->is_capable("osd", MON_CAP_X)) {
@@ -3050,7 +3050,7 @@ bool OSDMonitor::preprocess_full(MonOpRequestRef op)
   unsigned mask = CEPH_OSD_NEARFULL | CEPH_OSD_BACKFILLFULL | CEPH_OSD_FULL;
 
   // check permissions, ignore if failed
-  MonSession *session = m->get_session();
+  MonSession *session = op->get_session();
   if (!session)
     goto ignore;
   if (!session->is_capable("osd", MON_CAP_X)) {
@@ -3139,7 +3139,7 @@ bool OSDMonitor::preprocess_alive(MonOpRequestRef op)
   int from = m->get_orig_source().num();
 
   // check permissions, ignore if failed
-  MonSession *session = m->get_session();
+  MonSession *session = op->get_session();
   if (!session)
     goto ignore;
   if (!session->is_capable("osd", MON_CAP_X)) {
@@ -3204,7 +3204,7 @@ bool OSDMonitor::preprocess_pg_created(MonOpRequestRef op)
   op->mark_osdmon_event(__func__);
   auto m = static_cast<MOSDPGCreated*>(op->get_req());
   dout(10) << __func__ << " " << *m << dendl;
-  auto session = m->get_session();
+  auto session = op->get_session();
   mon->no_reply(op);
   if (!session) {
     dout(10) << __func__ << ": no monitor session!" << dendl;
@@ -3242,7 +3242,7 @@ bool OSDMonitor::preprocess_pg_ready_to_merge(MonOpRequestRef op)
   auto m = static_cast<MOSDPGReadyToMerge*>(op->get_req());
   dout(10) << __func__ << " " << *m << dendl;
   const pg_pool_t *pi;
-  auto session = m->get_session();
+  auto session = op->get_session();
   if (!session) {
     dout(10) << __func__ << ": no monitor session!" << dendl;
     goto ignore;
@@ -3343,7 +3343,7 @@ bool OSDMonitor::preprocess_pgtemp(MonOpRequestRef op)
   size_t ignore_cnt = 0;
 
   // check caps
-  MonSession *session = m->get_session();
+  MonSession *session = op->get_session();
   if (!session)
     goto ignore;
   if (!session->is_capable("osd", MON_CAP_X)) {
@@ -3486,7 +3486,7 @@ bool OSDMonitor::preprocess_remove_snaps(MonOpRequestRef op)
   dout(7) << "preprocess_remove_snaps " << *m << dendl;
 
   // check privilege, ignore if failed
-  MonSession *session = m->get_session();
+  MonSession *session = op->get_session();
   if (!session)
     goto ignore;
   if (!session->caps.is_capable(
@@ -3565,9 +3565,8 @@ bool OSDMonitor::prepare_remove_snaps(MonOpRequestRef op)
 bool OSDMonitor::preprocess_beacon(MonOpRequestRef op)
 {
   op->mark_osdmon_event(__func__);
-  auto beacon = static_cast<MOSDBeacon*>(op->get_req());
   // check caps
-  auto session = beacon->get_session();
+  auto session = op->get_session();
   mon->no_reply(op);
   if (!session) {
     dout(10) << __func__ << " no monitor session!" << dendl;
@@ -4530,8 +4529,9 @@ bool OSDMonitor::preprocess_command(MonOpRequestRef op)
     return true;
   }
 
-  MonSession *session = m->get_session();
+  MonSession *session = op->get_session();
   if (!session) {
+    derr << __func__ << " no session" << dendl;
     mon->reply_command(op, -EACCES, "access denied", get_last_committed());
     return true;
   }
@@ -6087,7 +6087,7 @@ int OSDMonitor::prepare_new_pool(MonOpRequestRef op)
   op->mark_osdmon_event(__func__);
   MPoolOp *m = static_cast<MPoolOp*>(op->get_req());
   dout(10) << "prepare_new_pool from " << m->get_connection() << dendl;
-  MonSession *session = m->get_session();
+  MonSession *session = op->get_session();
   if (!session)
     return -EPERM;
   string erasure_code_profile;
@@ -8054,8 +8054,9 @@ bool OSDMonitor::prepare_command(MonOpRequestRef op)
     return true;
   }
 
-  MonSession *session = m->get_session();
+  MonSession *session = op->get_session();
   if (!session) {
+    derr << __func__ << " no session" << dendl;
     mon->reply_command(op, -EACCES, "access denied", get_last_committed());
     return true;
   }
@@ -12111,7 +12112,7 @@ bool OSDMonitor::enforce_pool_op_caps(MonOpRequestRef op)
   op->mark_osdmon_event(__func__);
 
   MPoolOp *m = static_cast<MPoolOp*>(op->get_req());
-  MonSession *session = m->get_session();
+  MonSession *session = op->get_session();
   if (!session) {
     _pool_op_reply(op, -EPERM, osdmap.get_epoch());
     return true;
diff --git a/src/mon/OSDMonitor.h b/src/mon/OSDMonitor.h
index 8c6ecb864da5..cb7ef1892ef1 100644
--- a/src/mon/OSDMonitor.h
+++ b/src/mon/OSDMonitor.h
@@ -358,7 +358,7 @@ public:
 private:
   void print_utilization(ostream &out, Formatter *f, bool tree) const;
 
-  bool check_source(PaxosServiceMessage *m, uuid_d fsid);
+  bool check_source(MonOpRequestRef op, uuid_d fsid);
  
   bool preprocess_get_osdmap(MonOpRequestRef op);