BUG#18966 CHange in stop/shutdown behaviour

Fixes based on review by Tomas


ndb/src/mgmapi/mgmapi.cpp:
  Return immediately if ndb_mgm_get_version fails.
  correctly use new protocol for versions > 5.1
ndb/src/mgmsrv/Services.cpp:
  Only have 1 version of 'stop all' with reply being dependent on if the optional
  stop parameter is supplied.
ndb/src/mgmsrv/Services.hpp:
  Only 1 version of stopAll

ndb/src/mgmapi/mgmapi.cpp

@@ -870,18 +870,24 @@ ndb_mgm_stop3(NdbMgmHandle handle, int no_of_nodes, const int * node_list,
   if(handle->mgmd_version_build==-1)
   {
     char verstr[50];
-    ndb_mgm_get_version(handle,
+    if(!ndb_mgm_get_version(handle,
                         &(handle->mgmd_version_major),
                         &(handle->mgmd_version_minor),
                         &(handle->mgmd_version_build),
                         sizeof(verstr),
-                        verstr);
+                            verstr))
+    {
+      return -1;
+    }
   }
-  int use_v2= (handle->mgmd_version_major==5)
+  int use_v2= ((handle->mgmd_version_major==5)
     && (
         (handle->mgmd_version_minor==0 && handle->mgmd_version_build>=21)
         ||(handle->mgmd_version_minor==1 && handle->mgmd_version_build>=12)
-        );
+        ||(handle->mgmd_version_minor>1)
+        )
+               )
+    || (handle->mgmd_version_major>5);
 
   if(no_of_nodes < -1){
     SET_ERROR(handle, NDB_MGM_ILLEGAL_NUMBER_OF_NODES, 
@@ -900,7 +906,7 @@ ndb_mgm_stop3(NdbMgmHandle handle, int no_of_nodes, const int * node_list,
       args.put("stop", (no_of_nodes==-1)?"mgm,db":"db");
     const Properties *reply;
     if(use_v2)
-      reply = ndb_mgm_call(handle, stop_reply_v2, "stop all v2", &args);
+      reply = ndb_mgm_call(handle, stop_reply_v2, "stop all", &args);
     else
       reply = ndb_mgm_call(handle, stop_reply_v1, "stop all", &args);
     CHECK_REPLY(reply, -1);
@@ -1013,18 +1019,24 @@ ndb_mgm_restart3(NdbMgmHandle handle, int no_of_nodes, const int * node_list,
   if(handle->mgmd_version_build==-1)
   {
     char verstr[50];
-    ndb_mgm_get_version(handle,
+    if(!ndb_mgm_get_version(handle,
                         &(handle->mgmd_version_major),
                         &(handle->mgmd_version_minor),
                         &(handle->mgmd_version_build),
                         sizeof(verstr),
-                        verstr);
+                            verstr))
+    {
+      return -1;
+    }
   }
-  int use_v2= (handle->mgmd_version_major==5)
+  int use_v2= ((handle->mgmd_version_major==5)
     && (
         (handle->mgmd_version_minor==0 && handle->mgmd_version_build>=21)
         ||(handle->mgmd_version_minor==1 && handle->mgmd_version_build>=12)
-        );
+        ||(handle->mgmd_version_minor>1)
+        )
+               )
+    || (handle->mgmd_version_major>5);
 
   if(no_of_nodes < 0){
     SET_ERROR(handle, NDB_MGM_RESTART_FAILED, 

ndb/src/mgmsrv/Services.cpp

@@ -200,12 +200,9 @@ ParserRow<MgmApiSession> commands[] = {
     MGM_ARG("node", String, Mandatory, "Node"),
     MGM_ARG("abort", Int, Mandatory, "Node"),
 
-  MGM_CMD("stop all", &MgmApiSession::stopAll_v1, ""),
+  MGM_CMD("stop all", &MgmApiSession::stopAll, ""),
     MGM_ARG("abort", Int, Mandatory, "Node"),
-
-  MGM_CMD("stop all v2", &MgmApiSession::stopAll_v2, ""),
-    MGM_ARG("abort", Int, Mandatory, "Node"),
-    MGM_ARG("stop", String, Mandatory, "MGM/DB or both"),
+    MGM_ARG("stop", String, Optional, "MGM/DB or both"),
 
   MGM_CMD("enter single user", &MgmApiSession::enterSingleUser, ""),
     MGM_ARG("nodeId", Int, Mandatory, "Node"),
@@ -1071,31 +1068,26 @@ MgmApiSession::stop(Properties const &args, int version) {
   m_output->println("");
 }
 
-
-void
-MgmApiSession::stopAll_v1(Parser<MgmApiSession>::Context &,
-			      Properties const &args) {
-  stopAll(args,"db",1);
-}
-
 void
-MgmApiSession::stopAll_v2(Parser<MgmApiSession>::Context &,
-			      Properties const &args) {
-  BaseString tostop;
-  args.get("stop", tostop);
-  stopAll(args, tostop.c_str(), 2);
-}
-
-void
-MgmApiSession::stopAll(Properties const &args, const char* tostop, int ver) {
+MgmApiSession::stopAll(Parser<MgmApiSession>::Context &,
+                       Properties const &args) {
   int stopped[2] = {0,0};
   Uint32 abort;
   args.get("abort", &abort);
 
+  BaseString stop;
+  const char* tostop= "db";
+  int ver=1;
+  if (args.get("stop", stop))
+  {
+    tostop= stop.c_str();
+    ver= 2;
+  }
+
   int result= 0;
   if(strstr(tostop,"db"))
     result= m_mgmsrv.shutdownDB(&stopped[0], abort != 0);
-  if(strstr(tostop,"mgm"))
+  if(!result && strstr(tostop,"mgm"))
     result= m_mgmsrv.shutdownMGM(&stopped[1], abort!=0, &m_stopSelf);
 
   m_output->println("stop reply");

ndb/src/mgmsrv/Services.hpp

@@ -80,9 +80,7 @@ public:
   void stop_v1(Parser_t::Context &ctx, const class Properties &args);
   void stop_v2(Parser_t::Context &ctx, const class Properties &args);
   void stop(const class Properties &args, int version);
-  void stopAll_v1(Parser_t::Context &ctx, const class Properties &args);
-  void stopAll_v2(Parser_t::Context &ctx, const class Properties &args);
-  void stopAll(Properties const &args, const char* tostop, int ver);
+  void stopAll(Parser_t::Context &ctx, const class Properties &args);
   void start(Parser_t::Context &ctx, const class Properties &args);
   void startAll(Parser_t::Context &ctx, const class Properties &args);
   void bye(Parser_t::Context &ctx, const class Properties &args);