code review changes

Signed-off-by: Leonid Chernin <leonidc@il.ibm.com>

diff --git a/src/mon/NVMeofGwMap.cc b/src/mon/NVMeofGwMap.cc
index cd25200c0ca23922f3a6a286fe47cf833b9adfd4..a06fd5c98cf3396b0fe503f5a1d25d1d243f5e0f 100755 (executable)
--- a/src/mon/NVMeofGwMap.cc
+++ b/src/mon/NVMeofGwMap.cc
@@ -318,16 +318,17 @@ int NVMeofGwMap::cfg_admin_state_change(const NvmeGwId &gw_id,
           skip_failovers_for_group(group_key, 5);
           process_gw_map_gw_down(gw_id, group_key, propose_pending);
         }
+        st.gw_admin_state = state;
         propose_pending = true;
       }
     } else if (state == gw_admin_state_t::GW_ADMIN_ENABLED) {
       if (st.gw_admin_state == gw_admin_state_t::GW_ADMIN_DISABLED) {
         dout(4) << "GW-id set admin Enabled " << group_key
                 << " " << gw_id << dendl;
+        st.gw_admin_state = state;
         propose_pending = true;
       }
     }
-    st.gw_admin_state = state;
   } else {
      dout(4) << "GW-id not created yet " << group_key << " " << gw_id << dendl;
      return -EINVAL;
@@ -408,6 +409,7 @@ bool NVMeofGwMap::is_last_gw_in_location(const NvmeGwId &gw_id,
 bool NVMeofGwMap::is_location_in_disaster(const NvmeGroupKey& group_key,
            NvmeLocation& location, bool &cleanup_in_process) {
   auto grp_it = disaster_locations.find(group_key);
+  cleanup_in_process = false;
   if (grp_it != disaster_locations.end()) {
     auto &loc_states = grp_it->second;
     if (loc_states.find(location) != loc_states.end()) {
@@ -457,7 +459,7 @@ int NVMeofGwMap::cfg_location_disaster_set(
             << group_key << dendl;
     return -EINVAL;
   }
-  bool cleanup_in_process;
+  bool cleanup_in_process = false;
   bool location_exists = false;
   auto& gws_states = created_gws[group_key];
   if (is_location_in_disaster(group_key, location, cleanup_in_process)) {
@@ -501,7 +503,7 @@ int NVMeofGwMap::cfg_location_disaster_clear(
   }
   auto& gws_states = created_gws[group_key];
   bool accept = false;
-  bool cleanup_in_process;
+  bool cleanup_in_process = false;
   // for all the gateways of the subsystem
   if (!is_location_in_disaster(group_key, location, cleanup_in_process)) {
       dout(4) << "command cannot be accepted: in a group " << group_key
@@ -939,7 +941,7 @@ void NVMeofGwMap::set_failover_gw_for_ANA_group(
 {
   NvmeGwMonState& gw_state = created_gws[group_key][gw_id];
   NvmeGwMonState& failed_gw_state = created_gws[group_key][failed_gw_id];
-  epoch_t epoch;
+  epoch_t epoch = 0;
   dout(10) << "Found failover GW " << gw_id
           << " for ANA group " << (int)ANA_groupid << dendl;
   if (failed_gw_state.availability == gw_availability_t::GW_CREATED) {
@@ -971,7 +973,7 @@ void NVMeofGwMap::find_failback_gw(
   auto& gw_state = created_gws[group_key][gw_id];
   bool do_failback = false;
   bool allow_inter_location = true;
-  bool cleanup_in_process;
+  bool cleanup_in_process = false;
   if (is_location_in_disaster(group_key, gw_state.location, cleanup_in_process)) {
     if (!cleanup_in_process) {
       allow_inter_location = false;
@@ -1058,7 +1060,7 @@ int  NVMeofGwMap::find_failover_gw_logic(const NvmeGroupKey& group_key,
           (ignore_locations || st.location == location)) {
        num_gws ++;
        active_ana_groups_in_gw = 0;
-       bool cleanup_in_process;
+       bool cleanup_in_process = false;
        if (is_location_in_disaster(group_key, st.location, cleanup_in_process)) {
          continue;
        }
@@ -1394,7 +1396,7 @@ void NVMeofGwMap::fsm_handle_to_expired(
        }
        else {
       st.standby_state(grpid);
-      dout(10) << "GW failed durind failback/relocation persistency interval"
+      dout(10) << "GW failed during failback/relocation persistency interval"
                << gw_state.first << dendl;
     }
       }

diff --git a/src/mon/NVMeofGwMon.cc b/src/mon/NVMeofGwMon.cc
index ba9b925c90a7e682ee7eb846f0ae1ded6cf760b2..16d3424af0d1c3518a035ba181d57c2c03a7992f 100644 (file)
--- a/src/mon/NVMeofGwMon.cc
+++ b/src/mon/NVMeofGwMon.cc
@@ -748,7 +748,7 @@ bool NVMeofGwMon::prepare_command(MonOpRequestRef op)
       if (rc == -EEXIST) {
         sstrm.str("command already set please wait until completed");
       }
-      if (rc == EINVAL) {
+      if (rc == -EINVAL) {
         sstrm.str("command cannot be executed");
       }
     }
@@ -756,13 +756,13 @@ bool NVMeofGwMon::prepare_command(MonOpRequestRef op)
       response = true;
     }
   } else if (prefix == "nvme-gw disaster-clear") {
-      std::string id, pool, group, location;
+      std::string pool, group, location;
       bool propose = false;
       cmd_getval(cmdmap, "pool", pool);
       cmd_getval(cmdmap, "group", group);
       cmd_getval(cmdmap, "location", location);
       auto group_key = std::make_pair(pool, group);
-      dout(10) << id <<" pool "<< pool << " group "<< group
+      dout(10) << " pool "<< pool << " group "<< group
                <<" location "<< location << dendl;
       rc = pending_map.cfg_location_disaster_clear(group_key,
                        location, propose);
@@ -772,7 +772,7 @@ bool NVMeofGwMon::prepare_command(MonOpRequestRef op)
         if (rc == -EEXIST) {
           sstrm.str("command already set please wait until completed");
         }
-        if (rc == EINVAL) {
+        if (rc == -EINVAL) {
           sstrm.str("command cannot be executed");
         }
       }

diff --git a/src/mon/NVMeofGwSerialize.h b/src/mon/NVMeofGwSerialize.h
index 64752ac1d6e5068b7863f2f8be9938f720991535..5926fd8521d0f9b27c8486a6df7c3d9f2e2cd197 100755 (executable)
--- a/src/mon/NVMeofGwSerialize.h
+++ b/src/mon/NVMeofGwSerialize.h
@@ -711,48 +711,13 @@ inline void decode(std::map<NvmeGroupKey, epoch_t>& gw_epoch,
   DECODE_FINISH(bl);
 }
 
-inline void encode(
-    const std::map<NvmeGroupKey, LocationStates> &disaster_locations, ceph::bufferlist &bl) {
-  ENCODE_START(1, 1, bl);
-  encode ((uint32_t)disaster_locations.size(), bl); // number of groups
-  for (auto& group_disaster: disaster_locations) {
-     auto& group_key = group_disaster.first;
-     encode(group_key.first, bl); // pool
-     encode(group_key.second, bl); // group
-     const LocationStates &locations = group_disaster.second;
-     encode((uint32_t)locations.size(), bl);
-     for( auto &locations_it: locations) {
-       NvmeLocation location = locations_it.first;
-       encode(location, bl);
-       encode(locations_it.second.failbacks_in_process, bl);
-     }
-  }
-  ENCODE_FINISH(bl);
+inline void encode(const LocationState &locationstate, ceph::bufferlist &bl) {
+  encode(locationstate.failbacks_in_process, bl);
 }
 
-inline void decode(
-    std::map<NvmeGroupKey, LocationStates> &disaster_locations,
-    ceph::buffer::list::const_iterator &bl) {
-  disaster_locations.clear();
-  uint32_t ngroups;
-  DECODE_START(1, bl);
-  decode(ngroups, bl);
-  for (uint32_t i = 0; i<ngroups; i++) {
-    std::string pool, group;
-    decode(pool, bl);
-    decode(group, bl);
-    uint32_t nlocations;
-    decode(nlocations, bl);
-    for (uint32_t i = 0; i<nlocations; i++) {
-     NvmeLocation location;
-     bool failback_in_progress;
-     decode(location, bl);
-     decode(failback_in_progress, bl);
-     disaster_locations[std::make_pair(pool, group)][location]
-                .failbacks_in_process = failback_in_progress;
-    }
-  }
-  DECODE_FINISH(bl);
+inline void decode(LocationState &locationstate,
+                  ceph::buffer::list::const_iterator &bl) {
+  decode(locationstate.failbacks_in_process, bl);
 }
 
 inline void encode(