messages: avoid using mutable members in MMDSQuiesce*

Signed-off-by: Leonid Usov <leonid.usov@ibm.com>
(cherry picked from commit 2fbe40e72e8352a3bf47190d7bc8c80bb60eb7bd)

diff --git a/src/mds/MDSRankQuiesce.cc b/src/mds/MDSRankQuiesce.cc
index 820cd33fcffa83097a0323cbd307be9d01c62594..5d92c0bd818a38e0363bf522dc980d12adfe45c7 100644 (file)
--- a/src/mds/MDSRankQuiesce.cc
+++ b/src/mds/MDSRankQuiesce.cc
@@ -288,9 +288,9 @@ void MDSRank::quiesce_cluster_update() {
         }
         auto addrs = mdsmap->get_info_gid(leader).addrs;
 
-        auto ack_msg = make_message<MMDSQuiesceDbAck>(me);
+        auto ack_msg = make_message<MMDSQuiesceDbAck>();
         dout(10) << "sending ack " << ack << " to the leader " << leader << dendl;
-        ack_msg->diff_map = std::move(ack);
+        ack_msg->encode_payload_from(me, ack);
         return send_message_mds(ack_msg, addrs);
       }
     };
@@ -302,9 +302,9 @@ void MDSRank::quiesce_cluster_update() {
         return -ENOENT;
       }
       auto addrs = mdsmap->get_info_gid(to).addrs;
-      auto listing_msg = make_message<MMDSQuiesceDbListing>(me);
+      auto listing_msg = make_message<MMDSQuiesceDbListing>();
       dout(10) << "sending listing " << db << " to the peer " << to << dendl;
-      listing_msg->db_listing = std::move(db);
+      listing_msg->encode_payload_from(me, db);
       return send_message_mds(listing_msg, addrs);
     };
   }
@@ -374,38 +374,59 @@ void MDSRank::quiesce_cluster_update() {
 }
 
 bool MDSRank::quiesce_dispatch(const cref_t<Message> &m) {
-  switch(m->get_type()) {
-    case MSG_MDS_QUIESCE_DB_LISTING:
-    {
-      const auto& req = ref_cast<MMDSQuiesceDbListing>(m);
-      if (quiesce_db_manager) {
-        dout(10) << "got " << req->db_listing << " from peer " << req->gid << dendl;
-        int result = quiesce_db_manager->submit_listing_from(req->gid, std::move(req->db_listing));
-        if (result != 0) {
-          dout(3) << "error (" << result << ") submitting " << req->db_listing << " from peer " << req->gid << dendl;
+  try {
+    switch(m->get_type()) {
+      case MSG_MDS_QUIESCE_DB_LISTING:
+      {
+        const auto& req = ref_cast<MMDSQuiesceDbListing>(m);
+        mds_gid_t gid;
+        QuiesceDbListing db_listing;
+        req->decode_payload_into(gid, db_listing);
+        if (quiesce_db_manager) {
+          dout(10) << "got " << db_listing << " from peer " << gid << dendl;
+          int result = quiesce_db_manager->submit_listing_from(gid, std::move(db_listing));
+          if (result != 0) {
+            dout(3) << "error (" << result << ") submitting " << db_listing << " from peer " << gid << dendl;
+          }
+        } else {
+          dout(5) << "no db manager to process " << db_listing << dendl;
         }
-      } else {
-        dout(5) << "no db manager to process " << req->db_listing << dendl;
+        return true;
       }
-      return true;
-    }
-    case MSG_MDS_QUIESCE_DB_ACK:
-    {
-      const auto& req = ref_cast<MMDSQuiesceDbAck>(m);
-      if (quiesce_db_manager) {
-        dout(10) << "got ack " << req->diff_map << " from peer " << req->gid << dendl;
-        int result = quiesce_db_manager->submit_ack_from(req->gid, std::move(req->diff_map));
-        if (result != 0) {
-          dout(3) << "error (" << result << ") submitting an ack from peer " << req->gid << dendl;
+      case MSG_MDS_QUIESCE_DB_ACK:
+      {
+        const auto& req = ref_cast<MMDSQuiesceDbAck>(m);
+        mds_gid_t gid;
+        QuiesceMap diff_map;
+        req->decode_payload_into(gid, diff_map);
+        if (quiesce_db_manager) {
+          dout(10) << "got ack " << diff_map << " from peer " << gid << dendl;
+          int result = quiesce_db_manager->submit_ack_from(gid, std::move(diff_map));
+          if (result != 0) {
+            dout(3) << "error (" << result << ") submitting an ack from peer " << gid << dendl;
+          }
+        } else {
+          dout(5) << "no db manager to process an ack: " << diff_map << dendl;
         }
-      } else {
-        dout(5) << "no db manager to process an ack: " << req->diff_map << dendl;
+        return true;
+      }
+      default: break;
+    }
+  }
+  catch (const ceph::buffer::error &e) {
+    if (cct) {
+      dout(-1) << "failed to decode message of type " << m->get_type()
+                 << " v" << m->get_header().version
+                 << ": " << e.what() << dendl;
+      dout(10) << "dump: \n";
+      m->get_payload().hexdump(*_dout);
+      *_dout << dendl;
+      if (cct->_conf->ms_die_on_bad_msg) {
+        ceph_abort();
       }
-      return true;
     }
-    default:
-      return false;
   }
+  return false;
 }
 
 void MDSRank::quiesce_agent_setup() {

diff --git a/src/messages/MMDSQuiesceDbAck.h b/src/messages/MMDSQuiesceDbAck.h
index 907db239bd18fa351372aee4ec247837b749589f..1d56451e89bc1b566ae02d9a9522abfbdec918dc 100644 (file)
--- a/src/messages/MMDSQuiesceDbAck.h
+++ b/src/messages/MMDSQuiesceDbAck.h
@@ -19,13 +19,8 @@
 #include "mds/QuiesceDbEncoding.h"
 
 class MMDSQuiesceDbAck final : public MMDSOp {
-public:
-  mds_gid_t gid;
-  mutable QuiesceMap diff_map;
-
 protected:
-  MMDSQuiesceDbAck(mds_gid_t gid) : MMDSOp{MSG_MDS_QUIESCE_DB_ACK}, gid(gid) {}
-  MMDSQuiesceDbAck() : MMDSQuiesceDbAck(MDS_GID_NONE) {}
+  MMDSQuiesceDbAck() : MMDSOp{MSG_MDS_QUIESCE_DB_ACK} {}
   ~MMDSQuiesceDbAck() final {}
 
 public:
@@ -35,6 +30,11 @@ public:
   }
 
   void encode_payload(uint64_t features) override
+  {
+    // noop to prevent unnecessary overheads
+  }
+
+  void encode_payload_from(mds_gid_t const&gid, QuiesceMap const&diff_map)
   {
     using ceph::encode;
 
@@ -47,6 +47,11 @@ public:
   }
 
   void decode_payload() override {
+    // noop to prevent unnecessary overheads
+  }
+
+  void decode_payload_into(mds_gid_t &gid, QuiesceMap &diff_map) const
+  {
     using ceph::decode;
     auto p = payload.cbegin();
     DECODE_START(1, p);

diff --git a/src/messages/MMDSQuiesceDbListing.h b/src/messages/MMDSQuiesceDbListing.h
index 9d3ce20fdab4837ca12aaef7c34b8b0b69909e77..f57de50e22fb52c049295467de40ed97f293fb2d 100644 (file)
--- a/src/messages/MMDSQuiesceDbListing.h
+++ b/src/messages/MMDSQuiesceDbListing.h
@@ -19,13 +19,8 @@
 #include "mds/QuiesceDbEncoding.h"
 
 class MMDSQuiesceDbListing final : public MMDSOp {
-public:
-  mds_gid_t gid;
-  mutable QuiesceDbListing db_listing;
-
 protected:
-  MMDSQuiesceDbListing(mds_gid_t gid) : MMDSOp{MSG_MDS_QUIESCE_DB_LISTING}, gid(gid) {}
-  MMDSQuiesceDbListing() : MMDSQuiesceDbListing(MDS_GID_NONE) {}
+  MMDSQuiesceDbListing() : MMDSOp{MSG_MDS_QUIESCE_DB_LISTING} {}
   ~MMDSQuiesceDbListing() final {}
 
 public:
@@ -34,7 +29,11 @@ public:
 
   }
 
-  void encode_payload(uint64_t features) override
+  void encode_payload(uint64_t features) override { 
+    // noop to prevent unnecessary overheads
+  }
+
+  void encode_payload_from(mds_gid_t const& gid, QuiesceDbListing const& db_listing)
   {
     using ceph::encode;
 
@@ -47,6 +46,11 @@ public:
   }
 
   void decode_payload() override {
+    // noop to prevent unnecessary overheads
+  }
+
+  void decode_payload_into(mds_gid_t &gid, QuiesceDbListing &db_listing) const
+  {
     using ceph::decode;
     auto p = payload.cbegin();
     DECODE_START(1, p);