Hi Thuan

Ack with comments. See below [Lennart]
Still not tested

Thanks
Lennart

> -----Original Message-----
> From: thuan.tran <[email protected]>
> Sent: den 28 september 2018 06:00
> To: Lennart Lund <[email protected]>; Gary Lee
> <[email protected]>
> Cc: [email protected]; Thuan Tran
> <[email protected]>
> Subject: [PATCH 1/1] smf: campaign is executing forever until cluster reset
> [#1353]
> 
> The function getNodeDestination() reset elapsedTime to zero cause
> the node reboot timeout at waitForNodeDestination() never reach.
> If scenario that node reboot cannot come back then campaign is stuck
> in executing forever until cluster reset.
> ---
>  src/smf/smfd/SmfUpgradeStep.cc |  1 +
>  src/smf/smfd/SmfUtils.cc       | 28 ++++++++++++++--------------
>  src/smf/smfd/smfd.h            |  1 +
>  3 files changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/src/smf/smfd/SmfUpgradeStep.cc
> b/src/smf/smfd/SmfUpgradeStep.cc
> index 4c0ddd192..80da668de 100644
> --- a/src/smf/smfd/SmfUpgradeStep.cc
> +++ b/src/smf/smfd/SmfUpgradeStep.cc
> @@ -2399,6 +2399,7 @@ bool SmfUpgradeStep::nodeReboot() {
>        "SmfUpgradeStep::nodeReboot: Waiting to get node destination with
> increased UP counter");
> 
>    while (true) {
> +    elapsedTime = 0;
>      for (nodeIt = rebootedNodeList.begin(); nodeIt !=
> rebootedNodeList.end();) {
>        if (getNodeDestination((*nodeIt).node_name, &nodeDest,
> &elapsedTime,
>                               -1)) {
> diff --git a/src/smf/smfd/SmfUtils.cc b/src/smf/smfd/SmfUtils.cc
> index 915c086a5..ebc8b27ef 100644
> --- a/src/smf/smfd/SmfUtils.cc
> +++ b/src/smf/smfd/SmfUtils.cc
> @@ -95,9 +95,6 @@ bool getNodeDestination(const std::string &i_node,
> SmfndNodeDest *o_nodeDest,
> 
>    TRACE("Find destination for node '%s'", i_node.c_str());
> 
> -  if (elapsedTime)  // Initialize elapsedTime to zero.
> -    *elapsedTime = 0;
> -
>    /* It seems SaAmfNode objects can be stored, but the code
>     * indicates that SaClmNode's are expected. Anyway an attempt
>     * to go for it is probably faster that examining IMM classes
> @@ -131,12 +128,14 @@ bool getNodeDestination(const std::string
> &i_node, SmfndNodeDest *o_nodeDest,
>          LOG_NO("Failed to get node dest for clm node %s", i_node.c_str());
>          return false;
>        }
> -      struct timespec time = {2 * ONE_SECOND, 0};
> -      osaf_nanosleep(&time);
> -      timeout--;
> -      if (elapsedTime) *elapsedTime = *elapsedTime + 2 * ONE_SECOND;
> +      struct timespec two_seconds = {TWO_SECONDS, 0};
> +      osaf_nanosleep(&two_seconds);
> +      timeout -= TWO_SECONDS;
[Lennart]  Change if (elapsedTime) to if (elapsedTime != nullptr). The pointer 
elapsedTime is not a boolean
> +      if (elapsedTime) {
> +        *elapsedTime = *elapsedTime + TWO_SECONDS;
> +      }
>        if (maxWaitTime != -1) {
> -        if (*elapsedTime >= maxWaitTime) {
[Lennart]  Change if (elapsedTime) to if (elapsedTime != nullptr). The pointer 
elapsedTime is not a boolean 
> +        if ((elapsedTime) && (*elapsedTime >= maxWaitTime)) {
>            LOG_NO("Failed to get node dest for clm node %s", i_node.c_str());
>            return false;
>          }
> @@ -163,13 +162,14 @@ bool getNodeDestination(const std::string
> &i_node, SmfndNodeDest *o_nodeDest,
>          free(nodeName);
>          return false;
>        }
> -      struct timespec time = {2 * ONE_SECOND, 0};
> -      osaf_nanosleep(&time);
> -      timeout--;
> -      if (elapsedTime) *elapsedTime = *elapsedTime + 2 * ONE_SECOND;
> -
> +      struct timespec two_seconds = {TWO_SECONDS, 0};
> +      osaf_nanosleep(&two_seconds);
> +      timeout -= TWO_SECONDS;
[Lennart]  Change if (elapsedTime) to if (elapsedTime != nullptr). The pointer 
elapsedTime is not a boolean 
>+      if (elapsedTime) {
> +        *elapsedTime = *elapsedTime + TWO_SECONDS;
> +      }
>        if (maxWaitTime != -1) {
> -        if (*elapsedTime >= maxWaitTime) {
[Lennart]  Change if (elapsedTime) to if (elapsedTime != nullptr). The pointer 
elapsedTime is not a boolean 
> +        if ((elapsedTime) && (*elapsedTime >= maxWaitTime)) {
>            LOG_NO("Failed to get node dest for clm node %s", i_node.c_str());
>            free(nodeName);
>            return false;
> diff --git a/src/smf/smfd/smfd.h b/src/smf/smfd/smfd.h
> index 5e8ddeed8..fee7f1905 100644
> --- a/src/smf/smfd/smfd.h
> +++ b/src/smf/smfd/smfd.h
> @@ -67,6 +67,7 @@ extern "C" {
>   */
> 
>  #define ONE_SECOND 1
> +#define TWO_SECONDS 2
> 
>  /*
> ==========================================================
> ==============
>   *   DATA DECLARATIONS
> --
> 2.18.0



_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel

Reply via email to