IPL service not returning error on invalid isteps

Change-Id: Ie9d9873dee0b903c9deb107780eeda2a0d9375de
RTC: 70657
Reviewed-on: http://gfw160.austin.ibm.com:8080/gerrit/4499
Tested-by: Jenkins Server
Reviewed-by: Brian H. Horton <brianh@linux.ibm.com>
Reviewed-by: Van H. Lee <vanlee@us.ibm.com>
Reviewed-by: A. Patrick Williams III <iawillia@us.ibm.com>

2 files changed, 77 insertions, 85 deletions

diff --git a/src/usr/initservice/istepdispatcher/istepWorker.C b/src/usr/initservice/istepdispatcher/istepWorker.C
index e700f3ac7..1f67d6364 100644
--- a/src/usr/initservice/istepdispatcher/istepWorker.C
+++ b/src/usr/initservice/istepdispatcher/istepWorker.C
@@ -156,37 +156,37 @@ void iStepWorkerThread ( void * i_msgQ )
         substep = (theMsg->data[0] & 0xFF);
 
 
-        // Post the Progress Code
-        // TODO - Covered with RTC: 34046
-        InitService::getTheInstance().setProgressCode( progressCode );
-
         // Get the Task Info for this step
         const TaskInfo * theStep = findTaskInfo( istep,
                                                  substep );
 
-        if( prevStep != istep )
-        {
-            // unload the modules from the previous step
-            unLoadModules( prevStep );
-            // load modules for this step
-            loadModules( istep );
-            prevStep = istep;
-        }
-
-
         if( NULL != theStep )
         {
             TRACFCOMP( g_trac_initsvc,
-                    "IStepDispatcher (worker): Run Istep (%d), substep(%d), "
-                    "- %s",
+                    "IStepDispatcher (worker): "
+                    "Run Istep (%d), substep(%d), - %s",
                        istep, substep, theStep->taskname );
 
+            // Post the Progress Code
+            InitService::getTheInstance().setProgressCode( progressCode );
+
+            if( prevStep != istep )
+            {
+                // unload the modules from the previous step
+                unLoadModules( prevStep );
+                // load modules for this step
+                loadModules( istep );
+                prevStep = istep;
+            }
 
             err = InitService::getTheInstance().executeFn( theStep,
                                                            NULL );
 
+            //  flush contTrace immediately after each istep/substep  returns
+            TRAC_FLUSH_BUFFERS();
+
             // sync the attributes to fsp in single step mode but only
-            // after step 6 is complete to allow discoverTargets() to 
+            // after step 6 is complete to allow discoverTargets() to
             // run before the sync is done.
             if( step_mode && istep > 6 )
             {
@@ -199,8 +199,10 @@ void iStepWorkerThread ( void * i_msgQ )
 
                     if(l_errl)
                     {
-                        TRACFCOMP(g_trac_initsvc, "Attribute sync between steps failed, see"
-                                   "%x for details", l_errl->eid());
+                        TRACFCOMP(g_trac_initsvc,
+                        "Attribute sync between steps failed"
+                        "see %x for details",
+                        l_errl->eid());
 
                         errlCommit(l_errl, INITSVC_COMP_ID);
                     }
@@ -210,9 +212,10 @@ void iStepWorkerThread ( void * i_msgQ )
             if( err )
             {
                 TRACFCOMP( g_trac_initsvc,
-                           "IStepDipspatcher (worker): Istep %s returned "
-                           "errlog=%p",
-                           theStep->taskname, err );
+                           "IStepDipspatcher (worker): "
+                           "Istep %s return PLID=0x%x",
+                           theStep->taskname,
+                           err->plid() );
             }
             // Check for any attentions and invoke PRD for analysis
             else if ( true == theStep->taskflags.check_attn )
@@ -225,8 +228,8 @@ void iStepWorkerThread ( void * i_msgQ )
                 if ( err )
                 {
                     TRACFCOMP( g_trac_initsvc,
-                               "IStepDipspatcher (worker): Error returned "
-                               "from PRD analysis after Istep %s",
+                               "IStepDipspatcher (worker): "
+                               "ERROR return from PRD analysis after Istep %s",
                                theStep->taskname);
                 }
             }
@@ -234,13 +237,12 @@ void iStepWorkerThread ( void * i_msgQ )
         }
         else
         {
-#if 1
-            //  Nothing to do for this istep.
+            //  Invalid istep sent from FSP or spless, return error.
             TRACFCOMP( g_trac_initsvc,
-                       INFO_MRK"Empty Istep, Nothing to do!" );
-#else
-            //  $$  please save, need to fix for
-            //  $$      IPL service not returning error on invalid isteps
+                       "istepWorker: ERROR: "
+                       "Invalid Istep %d, substep %d ",
+                       istep,
+                       substep );
             //  This istep should have not been sent here to run, make up
             //  an errorlog and return it.
             /*@
@@ -259,26 +261,20 @@ void iStepWorkerThread ( void * i_msgQ )
                                            substep  );
 
             TRACFCOMP( g_trac_initsvc,
-                       ERR_MRK"Invalid Istep %d.%d, return errlog plid=0x%x",
+                       ERR_MRK"istepWorker ERROR:"
+                       "Invalid Istep %d.%d, returned errlog PLID=0x%x",
                        istep,
                        substep,
                        err->plid()  );
-#endif
         }
 
 
-
-        //  flush contTrace after each istep/substep  returns
-        TRAC_FLUSH_BUFFERS();
-
         msg_free( theMsg );
         theMsg = NULL;
     }   // end while(1)
 
     // Something bad happened...  this thread should never exit once started
-    assert( 0 );
-    TRACDCOMP( g_trac_initsvc,
-               ENTER_MRK"iStepWorkerThread()" );
+    assert( false, "istepWorker:  should never get here." );
 }
 
 
diff --git a/src/usr/initservice/istepdispatcher/istepdispatcher.C b/src/usr/initservice/istepdispatcher/istepdispatcher.C
index 6b403f2b1..afaa9761e 100644
--- a/src/usr/initservice/istepdispatcher/istepdispatcher.C
+++ b/src/usr/initservice/istepdispatcher/istepdispatcher.C
@@ -253,13 +253,13 @@ void IStepDispatcher::init ( errlHndl_t &io_rtaskRetErrl )
                 // normal IPL mode
                 TRACFCOMP( g_trac_initsvc, "sync attributes to FSP");
 
-                errlHndl_t l_errl = TARGETING::syncAllAttributesToFsp();
+                errlHndl_t l_syncAttrErrl = TARGETING::syncAllAttributesToFsp();
 
-                if(l_errl)
+                if(l_syncAttrErrl)
                 {
                     TRACFCOMP(g_trac_initsvc, "Attribute sync failed, see"
-                            "%x for details", l_errl->eid());
-                    errlCommit(l_errl, INITSVC_COMP_ID);
+                            "%x for details", l_syncAttrErrl->eid());
+                    errlCommit(l_syncAttrErrl, INITSVC_COMP_ID);
                 }
 
                 break;
@@ -299,28 +299,25 @@ errlHndl_t IStepDispatcher::executeAllISteps ( void )
              istep < MaxISteps;
              istep++ )
         {
-            // Run until num items +1, to be sure we know the last step
-            // finished
             for( size_t substep = 0;
-                 substep < (g_isteps[istep].numitems + 1);
+                 substep < g_isteps[istep].numitems ;
                  substep++ )
             {
-#if 1
-                //  $$  please save, need to fix for
-                //  $$      IPL service not returning error on invalid isteps
-#else
-                //  If the istep/substep doesn't exist, skip it.
-                if ( findTaskInfo( istep, substep ) ==  NULL )
+
+                //  Check to see if this is a valid istep, if not, don't
+                //  send it to istepWorker.  IstepWorker treats invalid
+                //  isteps as an error.
+                if ( NULL   ==  findTaskInfo( istep, substep ))
                 {
                     TRACFCOMP( g_trac_initsvc,
-                               INFO_MRK"Empty istep %d.%d, continuing...",
-                               istep,
-                               substep  );
+                               "executeAllSteps: "
+                               "skipping empty istep %d, substep: %d",
+                               istep, substep );
                     continue;
                 }
-#endif
-                // Before we can do anything, we need to be sure the worker thread is
-                // ready to start
+
+                // Before we can do anything, we need to be sure that
+                //  the worker thread is ready to start
                 theMsg = msg_wait( iv_msgQ );
 
                 // check for sync msgs
@@ -336,21 +333,14 @@ errlHndl_t IStepDispatcher::executeAllISteps ( void )
                     continue;
                 }
 
-                // If we just got the msg that the last step finished, break
-                // out
-                if( substep == (g_isteps[istep].numitems + 1) )
-                {
-                    TRACFCOMP( g_trac_initsvc,
-                               INFO_MRK"Last Step, exit" );
-                    break;
-                }
-
                 // Look for an errlog in extra_data
                 if( NULL != theMsg->extra_data )
                 {
                     TRACFCOMP( g_trac_initsvc,
-                               ERR_MRK"Error returned from istep(%d), substep(%d)",
-                               prevIstep, prevSubStep );
+                               ERR_MRK"executeAllISteps: "
+                               "Error returned from istep(%d), substep(%d)",
+                               prevIstep,
+                               prevSubStep );
 
                     err = ((errlHndl_t)theMsg->extra_data);
 
@@ -358,9 +348,12 @@ errlHndl_t IStepDispatcher::executeAllISteps ( void )
                 }
 
                 TRACFCOMP( g_trac_initsvc,
-                           INFO_MRK"executeAllSteps(0x%08x), istep: %d, substep: %d",
+                           INFO_MRK"executeAllSteps: "
+                           "type: 0x%08x, istep: %d, substep: %d",
                            theMsg->type, istep, substep );
 
+
+
                 // Set the Istep info
                 prevIstep = istep;
                 prevSubStep = substep;
@@ -373,7 +366,7 @@ errlHndl_t IStepDispatcher::executeAllISteps ( void )
                              theMsg );
 
                 theMsg = NULL;
-            } // for substep < (g_isteps[istep].numitems + 1)
+            } // for substep
 
             // check to see if there were any delayed deconfigure callouts
             // call HWAS to have this processed
@@ -517,10 +510,11 @@ errlHndl_t IStepDispatcher::msgHndlr ( void )
                 if( iv_Msg )
                 {
                     TRACFCOMP( g_trac_initsvc,
-                               ERR_MRK"ERROR: IStep Dispatcher already has another "
-                               "Istep request for step: 0x%02x, substep: 0x%02x",
-                               ((iv_Msg->type & 0xFF00) >> 8),
-                               (iv_Msg->type & 0xFF) );
+                               ERR_MRK"msgHndlr: ERROR: "
+                               "IStep Dispatcher has another Istep request"
+                               "for step: %d, substep: %d",
+                               ((theMsg->type & 0xFF00) >> 8),
+                               (theMsg->type & 0xFF) );
 
                     /*@
                      * @errortype
@@ -533,11 +527,12 @@ errlHndl_t IStepDispatcher::msgHndlr ( void )
                      *                   and we are still working on a
                      *                   previous Istep.
                      */
-                    err = new ERRORLOG::ErrlEntry( ERRORLOG::ERRL_SEV_UNRECOVERABLE,
-                                                   ISTEP_INITSVC_MOD_ID,
-                                                   ISTEP_MULTIPLE_ISTEP_REQ,
-                                                   ((iv_Msg->type & 0xFF00) >> 8),
-                                                   (iv_Msg->type & 0xFF) );
+                    err = new ERRORLOG::ErrlEntry(
+                        ERRORLOG::ERRL_SEV_UNRECOVERABLE,
+                        ISTEP_INITSVC_MOD_ID,
+                        ISTEP_MULTIPLE_ISTEP_REQ,
+                        ((theMsg->type & 0xFF00) >> 8),
+                        (theMsg->type & 0xFF) );
 
                     break;
                 }
@@ -593,7 +588,8 @@ void IStepDispatcher::waitForSyncPoint ( void )
         spLess() )
     {
         TRACFCOMP( g_trac_initsvc,
-                   INFO_MRK"Istep mode or SPless, no wait for sync point allowed" );
+                   INFO_MRK"Istep mode or SPless, "
+                   "no wait for sync point allowed" );
         return;
     }
 
@@ -691,8 +687,8 @@ errlHndl_t IStepDispatcher::sendIstepCompleteMsg ( void )
         else
         {
             TRACFCOMP( g_trac_initsvc,
-                       ERR_MRK"Request to send Istep complete, but no outstanding "
-                       "message from Fsp found!!" );
+                       ERR_MRK"Request to send Istep complete, "
+                       "but no outstanding message from Fsp found!!" );
 
             /*@
              * @errortype
@@ -701,8 +697,8 @@ errlHndl_t IStepDispatcher::sendIstepCompleteMsg ( void )
              * @moduleid         ISTEP_INITSVC_MOD_ID
              * @userdata1        Current Istep
              * @userdata2        Current SubStep
-             * @devdesc          Request to send Istep Complete msg to Fsp, but no
-             *                   outstanding message from Fsp found.
+             * @devdesc          Request to send Istep Complete msg to Fsp, but
+             *                   no outstanding message from Fsp found.
              */
             err = new ERRORLOG::ErrlEntry( ERRORLOG::ERRL_SEV_UNRECOVERABLE,
                                            ISTEP_INITSVC_MOD_ID,