OSDMap: pay attention to the temp_primary in _get_temp_osds

Switch _get_temp_osds to use pointers instead of references, and force callers
to check the out params instead of relying on a return code for if anything
was set (trying to use the return code when there are two possible outputs
does not provide useable semantics). For the new temp_primary out param, fill it
in from temp_primary if set, or from the pg_temp list if it's set, or leave
it blank if neither are.

Also, don't use pointers to heap elements. Just put the ints and vectors on
the stack, and assign/swap the out parameters with them. This is less
confusing and should be a bit faster in general.

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

diff --git a/src/osd/OSDMap.cc b/src/osd/OSDMap.cc
index f5f4ce916d35b5ff84495de8b487addb3cf5ea9d..8d3bece58fa25a79d841b2d5e84a733978ad545b 100644 (file)
--- a/src/osd/OSDMap.cc
+++ b/src/osd/OSDMap.cc
@@ -1291,20 +1291,25 @@ void OSDMap::_raw_to_up_osds(pg_t pg, const vector<int>& raw,
   *primary = (up->empty() ? -1 : up->front());
 }
   
-bool OSDMap::_get_temp_osds(const pg_pool_t& pool, pg_t pg, vector<int>& temp) const
+void OSDMap::_get_temp_osds(const pg_pool_t& pool, pg_t pg,
+                            vector<int> *temp_pg, int *temp_primary) const
 {
   pg = pool.raw_pg_to_pg(pg);
   map<pg_t,vector<int> >::const_iterator p = pg_temp->find(pg);
+  temp_pg->clear();
   if (p != pg_temp->end()) {
-    temp.clear();
     for (unsigned i=0; i<p->second.size(); i++) {
       if (!exists(p->second[i]) || is_down(p->second[i]))
        continue;
-      temp.push_back(p->second[i]);
+      temp_pg->push_back(p->second[i]);
     }
-    return true;
   }
-  return false;
+  map<pg_t,int>::const_iterator pp = primary_temp->find(pg);
+  *temp_primary = -1;
+  if (pp != primary_temp->end())
+    *temp_primary = pp->second;
+  else if (p != pg_temp->end()) // apply pg_temp's primary
+    *temp_primary = p->second.front();
 }
 
 int OSDMap::pg_to_osds(pg_t pg, vector<int> *raw, int *primary) const
@@ -1334,18 +1339,25 @@ void OSDMap::_pg_to_up_acting_osds(pg_t pg, vector<int> *up, int *up_primary,
   if (!pool)
     return;
   vector<int> raw;
-  vector<int> *_up = (up ? up : new vector<int>);
-  int *_up_primary = (up_primary ? up_primary : new int);
-  _pg_to_osds(*pool, pg, &raw, _up_primary);
-  _raw_to_up_osds(pg, raw, _up, _up_primary);
-  if (acting && !_get_temp_osds(*pool, pg, *acting))
-    *acting = *_up;
+  vector<int> _up;
+  vector<int> _acting;
+  int _up_primary;
+  int _acting_primary;
+  _pg_to_osds(*pool, pg, &raw, &_up_primary);
+  _raw_to_up_osds(pg, raw, &_up, &_up_primary);
+  _get_temp_osds(*pool, pg, &_acting, &_acting_primary);
+  if (_acting.empty())
+    _acting = _up;
+  if (_acting_primary == -1)
+    _acting_primary = _up_primary;
+  if (up)
+    up->swap(_up);
+  if (up_primary)
+    *up_primary = _up_primary;
+  if (acting)
+    acting->swap(_acting);
   if (acting_primary)
-    *acting_primary = (acting->empty() ? -1 : acting->front());
-  if (_up != up)
-    delete _up;
-  if (_up_primary != up_primary)
-    delete _up_primary;
+    *acting_primary = _acting_primary;
 }
 
 int OSDMap::calc_pg_rank(int osd, vector<int>& acting, int nrep)

diff --git a/src/osd/OSDMap.h b/src/osd/OSDMap.h
index 95d7e0f1c229b5281a6b7d86317e3f388c63f0d1..194683cd594407c26af73c1977204ad0d34f150d 100644 (file)
--- a/src/osd/OSDMap.h
+++ b/src/osd/OSDMap.h
@@ -513,11 +513,17 @@ private:
   void _raw_to_up_osds(pg_t pg, const vector<int>& raw,
                        vector<int> *up, int *primary) const;
 
-  bool _get_temp_osds(const pg_pool_t& pool, pg_t pg, vector<int>& temp) const;
+  /**
+   * Get the pg and primary temp, if they are specified.
+   * @param temp_pg [out] Will be empty or contain the temp PG mapping on return
+   * @param temp_primary [out] Will be the value in primary_temp, or a value derived
+   * from the pg_temp (if specified), or -1 if you should use the calculated (up_)primary.
+   */
+  void _get_temp_osds(const pg_pool_t& pool, pg_t pg,
+                      vector<int> *temp_pg, int *temp_primary) const;
 
   /**
-   *  map to up and acting. Fills in whatever fields are non-NULL, but
-   *  the passed-in vectors must be empty.
+   *  map to up and acting. Fills in whatever fields are non-NULL.
    */
   void _pg_to_up_acting_osds(pg_t pg, vector<int> *up, int *up_primary,
                              vector<int> *acting, int *acting_primary) const;