From 380ebf0087aa90f1417b5f99384e296135a84fd1 Mon Sep 17 00:00:00 2001 From: Leonid Chernin Date: Wed, 11 Feb 2026 15:16:49 +0200 Subject: [PATCH] code review changes Signed-off-by: Leonid Chernin --- src/mon/NVMeofGwMap.cc | 16 +++++++------ src/mon/NVMeofGwMon.cc | 10 ++++----- src/mon/NVMeofGwSerialize.h | 45 +++++-------------------------------- 3 files changed, 19 insertions(+), 52 deletions(-) diff --git a/src/mon/NVMeofGwMap.cc b/src/mon/NVMeofGwMap.cc index cd25200c0ca..a06fd5c98cf 100755 --- 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 345929d0d4e..acc1b75fca0 100644 --- a/src/mon/NVMeofGwMon.cc +++ b/src/mon/NVMeofGwMon.cc @@ -733,7 +733,7 @@ bool NVMeofGwMon::prepare_command(MonOpRequestRef op) << " " << pool << " " << group << " rc " << rc << dendl; sstrm.str(""); if (rc == -EEXIST) { - sstrm.str("old location removed but exist namespaces assigned to it"); + sstrm.str("The location is already set"); } } // propose pending would be generated by the PaxosService @@ -757,7 +757,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"); } } @@ -765,13 +765,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); @@ -781,7 +781,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 64752ac1d6e..5926fd8521d 100755 --- a/src/mon/NVMeofGwSerialize.h +++ b/src/mon/NVMeofGwSerialize.h @@ -711,48 +711,13 @@ inline void decode(std::map& gw_epoch, DECODE_FINISH(bl); } -inline void encode( - const std::map &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 &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