osd: pass pg_log_t by lvalue reference

for better readability, as we intent to relinquish the ownership to the
callee.

Signed-off-by: Kefu Chai <kchai@redhat.com>

diff --git a/src/osd/PGLog.cc b/src/osd/PGLog.cc
index 0a01b8478a8f5185b1987a4c341c7965197db87e..b0f74f02d945eacd0545947597dc1df3908d33f6 100644 (file)
--- a/src/osd/PGLog.cc
+++ b/src/osd/PGLog.cc
@@ -356,7 +356,7 @@ void PGLog::rewind_divergent_log(eversion_t newhead,
   dirty_big_info = true;
 }
 
-void PGLog::merge_log(pg_info_t &oinfo, pg_log_t &olog, pg_shard_t fromosd,
+void PGLog::merge_log(pg_info_t &oinfo, pg_log_t&& olog, pg_shard_t fromosd,
                       pg_info_t &info, LogEntryHandler *rollbacker,
                       bool &dirty_info, bool &dirty_big_info)
 {
@@ -399,7 +399,7 @@ void PGLog::merge_log(pg_info_t &oinfo, pg_log_t &olog, pg_shard_t fromosd,
 
     // splice into our log.
     log.log.splice(log.log.begin(),
-                  olog.log, from, to);
+                  std::move(olog.log), from, to);
 
     info.log_tail = log.tail = olog.tail;
     changed = true;

diff --git a/src/osd/PGLog.h b/src/osd/PGLog.h
index 68075ca6d924ca384d5e44c771917a4f4e25f0fd..cea890a4c69116785785583b50e9bf04be960544 100644 (file)
--- a/src/osd/PGLog.h
+++ b/src/osd/PGLog.h
@@ -1245,7 +1245,7 @@ public:
                             bool &dirty_big_info);
 
   void merge_log(pg_info_t &oinfo,
-                pg_log_t &olog,
+                pg_log_t&& olog,
                 pg_shard_t from,
                 pg_info_t &info, LogEntryHandler *rollbacker,
                 bool &dirty_info, bool &dirty_big_info);

diff --git a/src/osd/PeeringState.cc b/src/osd/PeeringState.cc
index b9578277ac6bca3b32b3f0e6eb43059a34614f3b..c600431600c49fc72a0f076a4c12bcd9e9a4567b 100644 (file)
--- a/src/osd/PeeringState.cc
+++ b/src/osd/PeeringState.cc
@@ -2672,12 +2672,13 @@ void PeeringState::share_pg_info()
 }
 
 void PeeringState::merge_log(
-  ObjectStore::Transaction& t, pg_info_t &oinfo, pg_log_t &olog,
+  ObjectStore::Transaction& t, pg_info_t &oinfo, pg_log_t&& olog,
   pg_shard_t from)
 {
   PGLog::LogEntryHandlerRef rollbacker{pl->get_log_handler(t)};
   pg_log.merge_log(
-    oinfo, olog, from, info, rollbacker.get(), dirty_info, dirty_big_info);
+    oinfo, std::move(olog), from, info, rollbacker.get(),
+    dirty_info, dirty_big_info);
 }
 
 void PeeringState::rewind_divergent_log(
@@ -2714,7 +2715,7 @@ void PeeringState::proc_primary_info(
 
 void PeeringState::proc_master_log(
   ObjectStore::Transaction& t, pg_info_t &oinfo,
-  pg_log_t &olog, pg_missing_t& omissing, pg_shard_t from)
+  pg_log_t&& olog, pg_missing_t&& omissing, pg_shard_t from)
 {
   psdout(10) << "proc_master_log for osd." << from << ": "
             << olog << " " << omissing << dendl;
@@ -2724,7 +2725,7 @@ void PeeringState::proc_master_log(
   // make any adjustments to their missing map; we are taking their
   // log to be authoritative (i.e., their entries are by definitely
   // non-divergent).
-  merge_log(t, oinfo, olog, from);
+  merge_log(t, oinfo, std::move(olog), from);
   peer_info[from] = oinfo;
   psdout(10) << " peer osd." << from << " now " << oinfo
             << " " << omissing << dendl;
@@ -2743,13 +2744,13 @@ void PeeringState::proc_master_log(
   ceph_assert(cct->_conf->osd_find_best_info_ignore_history_les ||
         info.last_epoch_started >= info.history.last_epoch_started);
 
-  peer_missing[from].claim(omissing);
+  peer_missing[from].claim(std::move(omissing));
 }
 
 void PeeringState::proc_replica_log(
   pg_info_t &oinfo,
   const pg_log_t &olog,
-  pg_missing_t& omissing,
+  pg_missing_t&& omissing,
   pg_shard_t from)
 {
   psdout(10) << "proc_replica_log for osd." << from << ": "
@@ -2769,7 +2770,7 @@ void PeeringState::proc_replica_log(
               << " need " << i->second.need
               << " have " << i->second.have << dendl;
   }
-  peer_missing[from].claim(omissing);
+  peer_missing[from].claim(std::move(omissing));
 }
 
 void PeeringState::fulfill_info(
@@ -5740,7 +5741,7 @@ boost::statechart::result PeeringState::Active::react(const MLogRec& logevt)
   psdout(10) << "searching osd." << logevt.from
                     << " log for unfound items" << dendl;
   ps->proc_replica_log(
-    logevt.msg->info, logevt.msg->log, logevt.msg->missing, logevt.from);
+    logevt.msg->info, logevt.msg->log, std::move(logevt.msg->missing), logevt.from);
   bool got_missing = ps->search_for_missing(
     ps->peer_info[logevt.from],
     ps->peer_missing[logevt.from],
@@ -6042,7 +6043,7 @@ boost::statechart::result PeeringState::ReplicaActive::react(const MLogRec& loge
   DECLARE_LOCALS;
   psdout(10) << "received log from " << logevt.from << dendl;
   ObjectStore::Transaction &t = context<PeeringMachine>().get_cur_transaction();
-  ps->merge_log(t, logevt.msg->info, logevt.msg->log, logevt.from);
+  ps->merge_log(t, logevt.msg->info, std::move(logevt.msg->log), logevt.from);
   ceph_assert(ps->pg_log.get_head() == ps->info.last_update);
   if (logevt.msg->lease) {
     ps->proc_lease(*logevt.msg->lease);
@@ -6148,7 +6149,7 @@ boost::statechart::result PeeringState::Stray::react(const MLogRec& logevt)
 
     ps->pg_log.reset_backfill();
   } else {
-    ps->merge_log(t, msg->info, msg->log, logevt.from);
+    ps->merge_log(t, msg->info, std::move(msg->log), logevt.from);
   }
   if (logevt.msg->lease) {
     ps->proc_lease(*logevt.msg->lease);
@@ -6580,7 +6581,7 @@ boost::statechart::result PeeringState::GetLog::react(const GotLog&)
   if (msg) {
     psdout(10) << "processing master log" << dendl;
     ps->proc_master_log(context<PeeringMachine>().get_cur_transaction(),
-                       msg->info, msg->log, msg->missing,
+                       msg->info, std::move(msg->log), std::move(msg->missing),
                        auth_log_shard);
   }
   ps->start_flush(context< PeeringMachine >().get_cur_transaction());
@@ -6892,7 +6893,10 @@ boost::statechart::result PeeringState::GetMissing::react(const MLogRec& logevt)
   DECLARE_LOCALS;
 
   peer_missing_requested.erase(logevt.from);
-  ps->proc_replica_log(logevt.msg->info, logevt.msg->log, logevt.msg->missing, logevt.from);
+  ps->proc_replica_log(logevt.msg->info,
+                      logevt.msg->log,
+                      std::move(logevt.msg->missing),
+                      logevt.from);
 
   if (peer_missing_requested.empty()) {
     if (ps->need_up_thru) {
@@ -6965,7 +6969,7 @@ boost::statechart::result PeeringState::WaitUpThru::react(const MLogRec& logevt)
 {
   DECLARE_LOCALS;
   psdout(10) << "Noting missing from osd." << logevt.from << dendl;
-  ps->peer_missing[logevt.from].claim(logevt.msg->missing);
+  ps->peer_missing[logevt.from].claim(std::move(logevt.msg->missing));
   ps->peer_info[logevt.from] = logevt.msg->info;
   return discard_event();
 }

diff --git a/src/osd/PeeringState.h b/src/osd/PeeringState.h
index 710a77e95679e6bd9f04ae73c3bb538bbf2874c1..56f76081bf861ff8940cc5150787d01ba9cdc4d3 100644 (file)
--- a/src/osd/PeeringState.h
+++ b/src/osd/PeeringState.h
@@ -1577,14 +1577,14 @@ public:
   void rewind_divergent_log(ObjectStore::Transaction& t, eversion_t newhead);
   void merge_log(
     ObjectStore::Transaction& t, pg_info_t &oinfo,
-    pg_log_t &olog, pg_shard_t from);
+    pg_log_t&& olog, pg_shard_t from);
 
   void proc_primary_info(ObjectStore::Transaction &t, const pg_info_t &info);
   void proc_master_log(ObjectStore::Transaction& t, pg_info_t &oinfo,
-                      pg_log_t &olog, pg_missing_t& omissing,
+                      pg_log_t&& olog, pg_missing_t&& omissing,
                       pg_shard_t from);
   void proc_replica_log(pg_info_t &oinfo, const pg_log_t &olog,
-                       pg_missing_t& omissing, pg_shard_t from);
+                       pg_missing_t&& omissing, pg_shard_t from);
 
   void calc_min_last_complete_ondisk() {
     eversion_t min = last_complete_ondisk;

diff --git a/src/osd/osd_types.h b/src/osd/osd_types.h
index d523bd3d91ade8a14c51eeb2ab192d560e5fd99b..f1bf86e445c2d3e5c7aac0888a9d2184abdc9e83 100644 (file)
--- a/src/osd/osd_types.h
+++ b/src/osd/osd_types.h
@@ -4705,10 +4705,10 @@ public:
     return it->second.need;
   }
 
-  void claim(pg_missing_set& o) {
+  void claim(pg_missing_set&& o) {
     static_assert(!TrackChanges, "Can't use claim with TrackChanges");
-    missing.swap(o.missing);
-    rmissing.swap(o.rmissing);
+    missing = std::move(o.missing);
+    rmissing = std::move(o.rmissing);
   }
 
   /*

diff --git a/src/test/osd/TestPGLog.cc b/src/test/osd/TestPGLog.cc
index 99e22f82df9688cd11c7469969e6490611a5028e..3c45d1212e459a6fcb111230dd15f7746eb70d79 100644 (file)
--- a/src/test/osd/TestPGLog.cc
+++ b/src/test/osd/TestPGLog.cc
@@ -289,7 +289,7 @@ public:
     bool dirty_info = false;
     bool dirty_big_info = false;
     merge_log(
-      oinfo, olog, pg_shard_t(1, shard_id_t(0)), info,
+      oinfo, std::move(olog), pg_shard_t(1, shard_id_t(0)), info,
       &h, dirty_info, dirty_big_info);
 
     ASSERT_EQ(info.last_update, oinfo.last_update);
@@ -890,7 +890,7 @@ TEST_F(PGLogTest, merge_log) {
     EXPECT_FALSE(dirty_big_info);
 
     TestHandler h(remove_snap);
-    merge_log(oinfo, olog, fromosd, info, &h,
+    merge_log(oinfo, std::move(olog), fromosd, info, &h,
               dirty_info, dirty_big_info);
 
     EXPECT_FALSE(missing.have_missing());
@@ -940,7 +940,7 @@ TEST_F(PGLogTest, merge_log) {
     EXPECT_FALSE(dirty_big_info);
 
     TestHandler h(remove_snap);
-    merge_log(oinfo, olog, fromosd, info, &h,
+    merge_log(oinfo, std::move(olog), fromosd, info, &h,
               dirty_info, dirty_big_info);
 
     EXPECT_FALSE(missing.have_missing());
@@ -1045,7 +1045,7 @@ TEST_F(PGLogTest, merge_log) {
     EXPECT_FALSE(dirty_big_info);
 
     TestHandler h(remove_snap);
-    merge_log(oinfo, olog, fromosd, info, &h,
+    merge_log(oinfo, std::move(olog), fromosd, info, &h,
               dirty_info, dirty_big_info);
 
     EXPECT_FALSE(missing.have_missing());
@@ -1154,7 +1154,7 @@ TEST_F(PGLogTest, merge_log) {
     EXPECT_FALSE(dirty_big_info);
 
     TestHandler h(remove_snap);
-    merge_log(oinfo, olog, fromosd, info, &h,
+    merge_log(oinfo, std::move(olog), fromosd, info, &h,
               dirty_info, dirty_big_info);
 
     /* When the divergent entry is a DELETE and the authoritative
@@ -1273,7 +1273,7 @@ TEST_F(PGLogTest, merge_log) {
 
     TestHandler h(remove_snap);
     missing.may_include_deletes = false;
-    merge_log(oinfo, olog, fromosd, info, &h,
+    merge_log(oinfo, std::move(olog), fromosd, info, &h,
               dirty_info, dirty_big_info);
 
     /* When the divergent entry is a DELETE and the authoritative
@@ -1375,7 +1375,7 @@ TEST_F(PGLogTest, merge_log) {
 
     TestHandler h(remove_snap);
     missing.may_include_deletes = false;
-    merge_log(oinfo, olog, fromosd, info, &h,
+    merge_log(oinfo, std::move(olog), fromosd, info, &h,
               dirty_info, dirty_big_info);
 
     EXPECT_FALSE(missing.have_missing());

diff --git a/src/test/osd/types.cc b/src/test/osd/types.cc
index bed33c02d61f53add319794712fc2ede11f5b194..c452176bcfc31df01f57f85cda559ede1b1aed4a 100644 (file)
--- a/src/test/osd/types.cc
+++ b/src/test/osd/types.cc
@@ -996,7 +996,7 @@ TEST(pg_missing_t, claim)
   pg_missing_t other;
   EXPECT_FALSE(other.have_missing());
 
-  other.claim(missing);
+  other.claim(std::move(missing));
   EXPECT_TRUE(other.have_missing());
 }