diff mbox series

suricatta: Improve handling of STATE_WAIT

Message ID 20201028185855.161915-1-sava.jakovljev@teufel.de
State Changes Requested
Headers show
Series suricatta: Improve handling of STATE_WAIT | expand

Commit Message

Sava Jakovljev Oct. 28, 2020, 6:58 p.m. UTC 
* Enable proper cancelation reply, allowing Hawkbit controller to reject
the cancelation request if it is not possible to cancel the upgrade.
* Make activation IPC better, thus allowing external programs to have better
control. This also fixes situations were an upgrade has been installed, but
not activated, which previosly lead to suricatta again processing the same
update action. Since external process is in charge when suricatta is in
STATE_WAIT, this change fixes that behavior.
* Fix deadlock in save_state() in activation_ipc.

Signed-off-by: Sava Jakovljev <sava.jakovljev@teufel.de>
---
 suricatta/server_hawkbit.c | 115 ++++++++++++++++++++++++++-----------
 1 file changed, 80 insertions(+), 35 deletions(-)

Comments

Stefano Babic Oct. 28, 2020, 8:06 p.m. UTC | #1 
Hi Sava,

this patch seems to address different issues. Each issue should be fixed 
by a separate patch and not by a cumulative patch, so please split this 
accordingly.

On 28.10.20 19:58, Sava Jakovljev wrote:
> * Enable proper cancelation reply, allowing Hawkbit controller to reject
> the cancelation request if it is not possible to cancel the upgrade.

I have not understood the issue.

> * Make activation IPC better, thus allowing external programs to have better
> control. This also fixes situations were an upgrade has been installed, but
> not activated, which previosly lead to suricatta again processing the same
> update action. Since external process is in charge when suricatta is in
> STATE_WAIT, this change fixes that behavior.

I have not understood this, and I cannot confirm that suricatta 
processes the same action. After an update, state go to installed and 
SWUpdate does not download again.

> * Fix deadlock in save_state() in activation_ipc.

Even not understood, which is the deadlock ?

> 
> Signed-off-by: Sava Jakovljev <sava.jakovljev@teufel.de>
> ---
>   suricatta/server_hawkbit.c | 115 ++++++++++++++++++++++++++-----------
>   1 file changed, 80 insertions(+), 35 deletions(-)
> 
> diff --git a/suricatta/server_hawkbit.c b/suricatta/server_hawkbit.c
> index 42e8569..3921148 100644
> --- a/suricatta/server_hawkbit.c
> +++ b/suricatta/server_hawkbit.c
> @@ -102,7 +102,8 @@ server_send_deployment_reply(channel_t *channel,
>   			     const int action_id, const int job_cnt_max,
>   			     const int job_cnt_cur, const char *finished,
>   			     const char *execution_status, int numdetails, const char *details[]);
> -server_op_res_t server_send_cancel_reply(channel_t *channel, const int action_id);
> +server_op_res_t server_send_cancel_reply(channel_t *channel, const int action_id ,
> +        bool cancelation_acknowledged);
>   static int get_target_data_length(void);
>   
>   server_hawkbit_t server_hawkbit = {.url = NULL,
> @@ -264,7 +265,8 @@ static void check_action_changed(int action_id, const char *update_action)
>   	}
>   }
>   
> -server_op_res_t server_send_cancel_reply(channel_t *channel, const int action_id)
> +server_op_res_t server_send_cancel_reply(channel_t *channel, const int action_id,
> +		bool cancelation_acknowledged)
>   {
>   	assert(server_hawkbit.url != NULL);
>   	assert(server_hawkbit.tenant != NULL);
> @@ -306,9 +308,13 @@ server_op_res_t server_send_cancel_reply(channel_t *channel, const int action_id
>   	(void)strftime(fdate, sizeof(fdate), "%Y%m%dT%H%M%S", localtime(&now));
>   	if (ENOMEM_ASPRINTF ==
>   	    asprintf(&json_reply_string, json_hawkbit_cancelation_feedback,
> -		     stop_id, fdate, reply_status_result_finished.success,
> -		     reply_status_execution.closed,
> -		     "cancellation acknowledged.")) {
> +		     stop_id, fdate,
> +				cancelation_acknowledged ? reply_status_result_finished.success :
> +					reply_status_result_finished.failure,
> +				cancelation_acknowledged ? reply_status_execution.closed :
> +					reply_status_execution.rejected,
> +				cancelation_acknowledged ? "cancellation acknowledged." :
> +					"cancelation not possible.")) {
>   		ERROR("hawkBit server reply cannot be sent because of OOM.");
>   		result = SERVER_EINIT;
>   		goto cleanup;
> @@ -327,17 +333,19 @@ cleanup:
>   		free(json_reply_string);
>   	}
>   
> -	/*
> -	 * Send always a notification
> -	 */
> -	char *notifybuf = NULL;
> -	if (ENOMEM_ASPRINTF ==
> -	    asprintf(&notifybuf, "{ \"id\" : \"%d\", \"stopId\" : \"%d\"}",
> -		     action_id, stop_id)) {
> -		notify(SUBPROCESS, CANCELUPDATE, INFOLEVEL, "Update cancelled");
> -	}  else {
> -		notify(SUBPROCESS, CANCELUPDATE, INFOLEVEL, notifybuf);
> -		free(notifybuf);
> +	if (cancelation_acknowledged == true) {
> +		/*
> +		 * Send a notification if we have acknowledged the cancelation.
> +		 */
> +		char *notifybuf = NULL;
> +		if (ENOMEM_ASPRINTF ==
> +			asprintf(&notifybuf, "{ \"id\" : \"%d\", \"stopId\" : \"%d\"}",
> +				 action_id, stop_id)) {
> +			notify(SUBPROCESS, CANCELUPDATE, INFOLEVEL, "Update cancelled");
> +		}  else {
> +			notify(SUBPROCESS, CANCELUPDATE, INFOLEVEL, notifybuf);
> +			free(notifybuf);
> +		}
>   	}
>   
>   	return result;
> @@ -709,7 +717,6 @@ static int server_check_during_dwl(void)
>   
>   server_op_res_t server_has_pending_action(int *action_id)
>   {
> -
>   	channel_data_t channel_data = channel_data_defaults;
>   	const char *update_action = NULL;
>   	server_op_res_t result =
> @@ -730,9 +737,15 @@ server_op_res_t server_has_pending_action(int *action_id)
>   		ERROR("JSON object should be freed but was not.");
>   	}
>   	if (result == SERVER_UPDATE_CANCELED) {
> -		DEBUG("Acknowledging cancelled update.");
> -		(void)server_send_cancel_reply(server_hawkbit.channel, *action_id);
> -		/* Inform the installer that a CANCEL was received */
> +		if (server_hawkbit.update_state != STATE_WAIT) {
> +			DEBUG("Acknowledging cancelled update.");
> +			(void)server_send_cancel_reply(server_hawkbit.channel, *action_id, true);
> +			/* Inform the installer that a CANCEL was received */
> +
> +			server_hawkbit.update_state = STATE_OK;
> +			save_state((char*)STATE_KEY, STATE_OK);
> +		}
> +
>   		return SERVER_OK;
>   	}
>   
> @@ -806,6 +819,11 @@ static server_op_res_t handle_feedback(int action_id, server_op_res_t result,
>   	case SERVER_EAGAIN:
>   		return result;
>   	case SERVER_UPDATE_AVAILABLE:
> +		if (state == STATE_INSTALLED) {
> +			DEBUG("Update installed, testing pending, skipping feedback.");
> +			return result;
> +		}
> +
>   		break;
>   	}
>   
> @@ -1358,7 +1376,7 @@ server_op_res_t server_install_update(void)
>   			/* Check if failed because it was cancelled */
>   			if (server_hawkbit.cancelDuringUpdate) {
>   				TRACE("Acknowledging cancelled update.");
> -				(void)server_send_cancel_reply(server_hawkbit.channel, action_id);
> +				(void)server_send_cancel_reply(server_hawkbit.channel, action_id, true);
>   				/* Inform the installer that a CANCEL was received */
>   			} else {
>   				/* TODO handle partial installations and rollback if
> @@ -1884,25 +1902,46 @@ static server_op_res_t server_activation_ipc(ipc_message *msg)
>   	result =
>   	    server_get_deployment_info(server_hawkbit.channel, &channel_data, &server_action_id);
>   
> -        if (result != SERVER_OK && result != SERVER_UPDATE_AVAILABLE &&
> -            result != SERVER_NO_UPDATE_AVAILABLE &&
> -            result != SERVER_UPDATE_CANCELED && result != SERVER_ID_REQUESTED) {
> -          DEBUG("Hawkbit is not accessible, bailing out (%d)", result);
> -          result = SERVER_EERR;
> -          goto cleanup;
> -        }
> -
> -        server_op_res_t response = SERVER_OK;
> +	if (result != SERVER_OK && result != SERVER_UPDATE_AVAILABLE &&
> +		result != SERVER_NO_UPDATE_AVAILABLE &&
> +		result != SERVER_UPDATE_CANCELED && result != SERVER_ID_REQUESTED) {
> +		DEBUG("Hawkbit is not accessible, bailing out (%d)", result);
> +		result = SERVER_EERR;
> +		goto cleanup;
> +	}
>   
>   	if (result == SERVER_UPDATE_CANCELED) {
> -		DEBUG("Acknowledging cancelled update.");
> -		(void)server_send_cancel_reply(server_hawkbit.channel, server_action_id);
> +		DEBUG("Acknowledging canceled update%s",
> +				update_state != STATE_TESTING ? "." : " not possible");
> +		(void)server_send_cancel_reply(server_hawkbit.channel, server_action_id,
> +				update_state != STATE_TESTING);
> +
> +		if (update_state == STATE_TESTING) {
> +			/*
> +			 * Upgrade is installed and activated, we rejected the cancellation
> +			 * request. We now have to send the actual feedback, and it can
> +			 * only be sent from UPDATE_AVAILABLE state.
> +			 */
> +			result = SERVER_UPDATE_AVAILABLE;
> +		} else if (update_state == STATE_INSTALLED) {
> +			/*
> +			 * Upgrade is installed, but not activated, so we are able to confirm
> +			 * the cancellation. There is no need to send any feedback, but we
> +			 * want to exit STATE_WAIT.
> +			 */
> +			server_hawkbit.update_state = STATE_OK;
> +			save_state((char*)STATE_KEY, STATE_OK);
> +
> +			result = SERVER_OK;
> +			goto cleanup;
> +		}
>   	}
>   
> +	server_op_res_t response = SERVER_OK;
>   	if (action_id != server_action_id) {
>   		TRACE("Deployment changed on server: our id %d, on server %d",
>   			action_id, server_action_id);
> -	} else {
> +	} else if (update_state != STATE_INSTALLED) {
>   		response = handle_feedback(action_id, result, update_state, reply_result,
>   					   reply_execution,
>   					   numdetails == 0 ? 1 : numdetails, details);
> @@ -1916,12 +1955,18 @@ static server_op_res_t server_activation_ipc(ipc_message *msg)
>   	if ((response != SERVER_UPDATE_AVAILABLE) && (response != SERVER_OK))
>   		result = SERVER_EERR;
>   	else {
> -		server_hawkbit.update_state = STATE_OK;
> +		/*
> +		 * If the update is installed and not activated, we don't want to start
> +		 * processing the same action again - thus we stay in STATE_INSTALLED.
> +		 */
> +		update_state_t new_update_state = update_state == STATE_INSTALLED ?
> +			STATE_INSTALLED : STATE_OK;
>   
> +		server_hawkbit.update_state = new_update_state;
>   		/*
>   		 * Save the state
>   		 */
> -		if ((result = save_state((char *)STATE_KEY, STATE_OK)) != SERVER_OK) {
> +		if ((result = save_state_string((char *)STATE_KEY, new_update_state)) != SERVER_OK) {
>   			ERROR("Error while resetting update state on persistent "
>   			"storage.\n");
>   		}
> 

Best regards,
Stefano Babic
Sava Jakovljev Oct. 29, 2020, 12:36 p.m. UTC | #2 
Hi Stefano,

Thanks, you're right, makes sense - I'll split them into 3 commits.

Cheers,
Sava Jakovljev

Sava Jakovljev schrieb am Mittwoch, 28. Oktober 2020 um 19:59:14 UTC+1:

> * Enable proper cancelation reply, allowing Hawkbit controller to reject
> the cancelation request if it is not possible to cancel the upgrade.
> * Make activation IPC better, thus allowing external programs to have 
> better
> control. This also fixes situations were an upgrade has been installed, but
> not activated, which previosly lead to suricatta again processing the same
> update action. Since external process is in charge when suricatta is in
> STATE_WAIT, this change fixes that behavior.
> * Fix deadlock in save_state() in activation_ipc.
>
> Signed-off-by: Sava Jakovljev <sava.ja...@teufel.de>
> ---
> suricatta/server_hawkbit.c | 115 ++++++++++++++++++++++++++-----------
> 1 file changed, 80 insertions(+), 35 deletions(-)
>
> diff --git a/suricatta/server_hawkbit.c b/suricatta/server_hawkbit.c
> index 42e8569..3921148 100644
> --- a/suricatta/server_hawkbit.c
> +++ b/suricatta/server_hawkbit.c
> @@ -102,7 +102,8 @@ server_send_deployment_reply(channel_t *channel,
> const int action_id, const int job_cnt_max,
> const int job_cnt_cur, const char *finished,
> const char *execution_status, int numdetails, const char *details[]);
> -server_op_res_t server_send_cancel_reply(channel_t *channel, const int 
> action_id);
> +server_op_res_t server_send_cancel_reply(channel_t *channel, const int 
> action_id ,
> + bool cancelation_acknowledged);
> static int get_target_data_length(void);
>
> server_hawkbit_t server_hawkbit = {.url = NULL,
> @@ -264,7 +265,8 @@ static void check_action_changed(int action_id, const 
> char *update_action)
> }
> }
>
> -server_op_res_t server_send_cancel_reply(channel_t *channel, const int 
> action_id)
> +server_op_res_t server_send_cancel_reply(channel_t *channel, const int 
> action_id,
> + bool cancelation_acknowledged)
> {
> assert(server_hawkbit.url != NULL);
> assert(server_hawkbit.tenant != NULL);
> @@ -306,9 +308,13 @@ server_op_res_t server_send_cancel_reply(channel_t 
> *channel, const int action_id
> (void)strftime(fdate, sizeof(fdate), "%Y%m%dT%H%M%S", localtime(&now));
> if (ENOMEM_ASPRINTF ==
> asprintf(&json_reply_string, json_hawkbit_cancelation_feedback,
> - stop_id, fdate, reply_status_result_finished.success,
> - reply_status_execution.closed,
> - "cancellation acknowledged.")) {
> + stop_id, fdate,
> + cancelation_acknowledged ? reply_status_result_finished.success :
> + reply_status_result_finished.failure,
> + cancelation_acknowledged ? reply_status_execution.closed :
> + reply_status_execution.rejected,
> + cancelation_acknowledged ? "cancellation acknowledged." :
> + "cancelation not possible.")) {
> ERROR("hawkBit server reply cannot be sent because of OOM.");
> result = SERVER_EINIT;
> goto cleanup;
> @@ -327,17 +333,19 @@ cleanup:
> free(json_reply_string);
> }
>
> - /*
> - * Send always a notification
> - */
> - char *notifybuf = NULL;
> - if (ENOMEM_ASPRINTF ==
> - asprintf(&notifybuf, "{ \"id\" : \"%d\", \"stopId\" : \"%d\"}",
> - action_id, stop_id)) {
> - notify(SUBPROCESS, CANCELUPDATE, INFOLEVEL, "Update cancelled");
> - } else {
> - notify(SUBPROCESS, CANCELUPDATE, INFOLEVEL, notifybuf);
> - free(notifybuf);
> + if (cancelation_acknowledged == true) {
> + /*
> + * Send a notification if we have acknowledged the cancelation.
> + */
> + char *notifybuf = NULL;
> + if (ENOMEM_ASPRINTF ==
> + asprintf(&notifybuf, "{ \"id\" : \"%d\", \"stopId\" : \"%d\"}",
> + action_id, stop_id)) {
> + notify(SUBPROCESS, CANCELUPDATE, INFOLEVEL, "Update cancelled");
> + } else {
> + notify(SUBPROCESS, CANCELUPDATE, INFOLEVEL, notifybuf);
> + free(notifybuf);
> + }
> }
>
> return result;
> @@ -709,7 +717,6 @@ static int server_check_during_dwl(void)
>
> server_op_res_t server_has_pending_action(int *action_id)
> {
> -
> channel_data_t channel_data = channel_data_defaults;
> const char *update_action = NULL;
> server_op_res_t result =
> @@ -730,9 +737,15 @@ server_op_res_t server_has_pending_action(int 
> *action_id)
> ERROR("JSON object should be freed but was not.");
> }
> if (result == SERVER_UPDATE_CANCELED) {
> - DEBUG("Acknowledging cancelled update.");
> - (void)server_send_cancel_reply(server_hawkbit.channel, *action_id);
> - /* Inform the installer that a CANCEL was received */
> + if (server_hawkbit.update_state != STATE_WAIT) {
> + DEBUG("Acknowledging cancelled update.");
> + (void)server_send_cancel_reply(server_hawkbit.channel, *action_id, true);
> + /* Inform the installer that a CANCEL was received */
> +
> + server_hawkbit.update_state = STATE_OK;
> + save_state((char*)STATE_KEY, STATE_OK);
> + }
> +
> return SERVER_OK;
> }
>
> @@ -806,6 +819,11 @@ static server_op_res_t handle_feedback(int action_id, 
> server_op_res_t result,
> case SERVER_EAGAIN:
> return result;
> case SERVER_UPDATE_AVAILABLE:
> + if (state == STATE_INSTALLED) {
> + DEBUG("Update installed, testing pending, skipping feedback.");
> + return result;
> + }
> +
> break;
> }
>
> @@ -1358,7 +1376,7 @@ server_op_res_t server_install_update(void)
> /* Check if failed because it was cancelled */
> if (server_hawkbit.cancelDuringUpdate) {
> TRACE("Acknowledging cancelled update.");
> - (void)server_send_cancel_reply(server_hawkbit.channel, action_id);
> + (void)server_send_cancel_reply(server_hawkbit.channel, action_id, true);
> /* Inform the installer that a CANCEL was received */
> } else {
> /* TODO handle partial installations and rollback if
> @@ -1884,25 +1902,46 @@ static server_op_res_t 
> server_activation_ipc(ipc_message *msg)
> result =
> server_get_deployment_info(server_hawkbit.channel, &channel_data, 
> &server_action_id);
>
> - if (result != SERVER_OK && result != SERVER_UPDATE_AVAILABLE &&
> - result != SERVER_NO_UPDATE_AVAILABLE &&
> - result != SERVER_UPDATE_CANCELED && result != SERVER_ID_REQUESTED) {
> - DEBUG("Hawkbit is not accessible, bailing out (%d)", result);
> - result = SERVER_EERR;
> - goto cleanup;
> - }
> -
> - server_op_res_t response = SERVER_OK;
> + if (result != SERVER_OK && result != SERVER_UPDATE_AVAILABLE &&
> + result != SERVER_NO_UPDATE_AVAILABLE &&
> + result != SERVER_UPDATE_CANCELED && result != SERVER_ID_REQUESTED) {
> + DEBUG("Hawkbit is not accessible, bailing out (%d)", result);
> + result = SERVER_EERR;
> + goto cleanup;
> + }
>
> if (result == SERVER_UPDATE_CANCELED) {
> - DEBUG("Acknowledging cancelled update.");
> - (void)server_send_cancel_reply(server_hawkbit.channel, server_action_id);
> + DEBUG("Acknowledging canceled update%s",
> + update_state != STATE_TESTING ? "." : " not possible");
> + (void)server_send_cancel_reply(server_hawkbit.channel, server_action_id,
> + update_state != STATE_TESTING);
> +
> + if (update_state == STATE_TESTING) {
> + /*
> + * Upgrade is installed and activated, we rejected the cancellation
> + * request. We now have to send the actual feedback, and it can
> + * only be sent from UPDATE_AVAILABLE state.
> + */
> + result = SERVER_UPDATE_AVAILABLE;
> + } else if (update_state == STATE_INSTALLED) {
> + /*
> + * Upgrade is installed, but not activated, so we are able to confirm
> + * the cancellation. There is no need to send any feedback, but we
> + * want to exit STATE_WAIT.
> + */
> + server_hawkbit.update_state = STATE_OK;
> + save_state((char*)STATE_KEY, STATE_OK);
> +
> + result = SERVER_OK;
> + goto cleanup;
> + }
> }
>
> + server_op_res_t response = SERVER_OK;
> if (action_id != server_action_id) {
> TRACE("Deployment changed on server: our id %d, on server %d",
> action_id, server_action_id);
> - } else {
> + } else if (update_state != STATE_INSTALLED) {
> response = handle_feedback(action_id, result, update_state, reply_result,
> reply_execution,
> numdetails == 0 ? 1 : numdetails, details);
> @@ -1916,12 +1955,18 @@ static server_op_res_t 
> server_activation_ipc(ipc_message *msg)
> if ((response != SERVER_UPDATE_AVAILABLE) && (response != SERVER_OK))
> result = SERVER_EERR;
> else {
> - server_hawkbit.update_state = STATE_OK;
> + /*
> + * If the update is installed and not activated, we don't want to start
> + * processing the same action again - thus we stay in STATE_INSTALLED.
> + */
> + update_state_t new_update_state = update_state == STATE_INSTALLED ?
> + STATE_INSTALLED : STATE_OK;
>
> + server_hawkbit.update_state = new_update_state;
> /*
> * Save the state
> */
> - if ((result = save_state((char *)STATE_KEY, STATE_OK)) != SERVER_OK) {
> + if ((result = save_state_string((char *)STATE_KEY, new_update_state)) != 
> SERVER_OK) {
> ERROR("Error while resetting update state on persistent "
> "storage.\n");
> }
> -- 
> 2.26.2
>
>
diff mbox series

Patch

diff --git a/suricatta/server_hawkbit.c b/suricatta/server_hawkbit.c
index 42e8569..3921148 100644
--- a/suricatta/server_hawkbit.c
+++ b/suricatta/server_hawkbit.c
@@ -102,7 +102,8 @@  server_send_deployment_reply(channel_t *channel,
 			     const int action_id, const int job_cnt_max,
 			     const int job_cnt_cur, const char *finished,
 			     const char *execution_status, int numdetails, const char *details[]);
-server_op_res_t server_send_cancel_reply(channel_t *channel, const int action_id);
+server_op_res_t server_send_cancel_reply(channel_t *channel, const int action_id ,
+        bool cancelation_acknowledged);
 static int get_target_data_length(void);
 
 server_hawkbit_t server_hawkbit = {.url = NULL,
@@ -264,7 +265,8 @@  static void check_action_changed(int action_id, const char *update_action)
 	}
 }
 
-server_op_res_t server_send_cancel_reply(channel_t *channel, const int action_id)
+server_op_res_t server_send_cancel_reply(channel_t *channel, const int action_id,
+		bool cancelation_acknowledged)
 {
 	assert(server_hawkbit.url != NULL);
 	assert(server_hawkbit.tenant != NULL);
@@ -306,9 +308,13 @@  server_op_res_t server_send_cancel_reply(channel_t *channel, const int action_id
 	(void)strftime(fdate, sizeof(fdate), "%Y%m%dT%H%M%S", localtime(&now));
 	if (ENOMEM_ASPRINTF ==
 	    asprintf(&json_reply_string, json_hawkbit_cancelation_feedback,
-		     stop_id, fdate, reply_status_result_finished.success,
-		     reply_status_execution.closed,
-		     "cancellation acknowledged.")) {
+		     stop_id, fdate,
+				cancelation_acknowledged ? reply_status_result_finished.success :
+					reply_status_result_finished.failure,
+				cancelation_acknowledged ? reply_status_execution.closed :
+					reply_status_execution.rejected,
+				cancelation_acknowledged ? "cancellation acknowledged." :
+					"cancelation not possible.")) {
 		ERROR("hawkBit server reply cannot be sent because of OOM.");
 		result = SERVER_EINIT;
 		goto cleanup;
@@ -327,17 +333,19 @@  cleanup:
 		free(json_reply_string);
 	}
 
-	/*
-	 * Send always a notification
-	 */
-	char *notifybuf = NULL;
-	if (ENOMEM_ASPRINTF ==
-	    asprintf(&notifybuf, "{ \"id\" : \"%d\", \"stopId\" : \"%d\"}",
-		     action_id, stop_id)) {
-		notify(SUBPROCESS, CANCELUPDATE, INFOLEVEL, "Update cancelled");
-	}  else {
-		notify(SUBPROCESS, CANCELUPDATE, INFOLEVEL, notifybuf);
-		free(notifybuf);
+	if (cancelation_acknowledged == true) {
+		/*
+		 * Send a notification if we have acknowledged the cancelation.
+		 */
+		char *notifybuf = NULL;
+		if (ENOMEM_ASPRINTF ==
+			asprintf(&notifybuf, "{ \"id\" : \"%d\", \"stopId\" : \"%d\"}",
+				 action_id, stop_id)) {
+			notify(SUBPROCESS, CANCELUPDATE, INFOLEVEL, "Update cancelled");
+		}  else {
+			notify(SUBPROCESS, CANCELUPDATE, INFOLEVEL, notifybuf);
+			free(notifybuf);
+		}
 	}
 
 	return result;
@@ -709,7 +717,6 @@  static int server_check_during_dwl(void)
 
 server_op_res_t server_has_pending_action(int *action_id)
 {
-
 	channel_data_t channel_data = channel_data_defaults;
 	const char *update_action = NULL;
 	server_op_res_t result =
@@ -730,9 +737,15 @@  server_op_res_t server_has_pending_action(int *action_id)
 		ERROR("JSON object should be freed but was not.");
 	}
 	if (result == SERVER_UPDATE_CANCELED) {
-		DEBUG("Acknowledging cancelled update.");
-		(void)server_send_cancel_reply(server_hawkbit.channel, *action_id);
-		/* Inform the installer that a CANCEL was received */
+		if (server_hawkbit.update_state != STATE_WAIT) {
+			DEBUG("Acknowledging cancelled update.");
+			(void)server_send_cancel_reply(server_hawkbit.channel, *action_id, true);
+			/* Inform the installer that a CANCEL was received */
+
+			server_hawkbit.update_state = STATE_OK;
+			save_state((char*)STATE_KEY, STATE_OK);
+		}
+
 		return SERVER_OK;
 	}
 
@@ -806,6 +819,11 @@  static server_op_res_t handle_feedback(int action_id, server_op_res_t result,
 	case SERVER_EAGAIN:
 		return result;
 	case SERVER_UPDATE_AVAILABLE:
+		if (state == STATE_INSTALLED) {
+			DEBUG("Update installed, testing pending, skipping feedback.");
+			return result;
+		}
+
 		break;
 	}
 
@@ -1358,7 +1376,7 @@  server_op_res_t server_install_update(void)
 			/* Check if failed because it was cancelled */
 			if (server_hawkbit.cancelDuringUpdate) {
 				TRACE("Acknowledging cancelled update.");
-				(void)server_send_cancel_reply(server_hawkbit.channel, action_id);
+				(void)server_send_cancel_reply(server_hawkbit.channel, action_id, true);
 				/* Inform the installer that a CANCEL was received */
 			} else {
 				/* TODO handle partial installations and rollback if
@@ -1884,25 +1902,46 @@  static server_op_res_t server_activation_ipc(ipc_message *msg)
 	result =
 	    server_get_deployment_info(server_hawkbit.channel, &channel_data, &server_action_id);
 
-        if (result != SERVER_OK && result != SERVER_UPDATE_AVAILABLE &&
-            result != SERVER_NO_UPDATE_AVAILABLE &&
-            result != SERVER_UPDATE_CANCELED && result != SERVER_ID_REQUESTED) {
-          DEBUG("Hawkbit is not accessible, bailing out (%d)", result);
-          result = SERVER_EERR;
-          goto cleanup;
-        }
-
-        server_op_res_t response = SERVER_OK;
+	if (result != SERVER_OK && result != SERVER_UPDATE_AVAILABLE &&
+		result != SERVER_NO_UPDATE_AVAILABLE &&
+		result != SERVER_UPDATE_CANCELED && result != SERVER_ID_REQUESTED) {
+		DEBUG("Hawkbit is not accessible, bailing out (%d)", result);
+		result = SERVER_EERR;
+		goto cleanup;
+	}
 
 	if (result == SERVER_UPDATE_CANCELED) {
-		DEBUG("Acknowledging cancelled update.");
-		(void)server_send_cancel_reply(server_hawkbit.channel, server_action_id);
+		DEBUG("Acknowledging canceled update%s",
+				update_state != STATE_TESTING ? "." : " not possible");
+		(void)server_send_cancel_reply(server_hawkbit.channel, server_action_id,
+				update_state != STATE_TESTING);
+
+		if (update_state == STATE_TESTING) {
+			/*
+			 * Upgrade is installed and activated, we rejected the cancellation
+			 * request. We now have to send the actual feedback, and it can
+			 * only be sent from UPDATE_AVAILABLE state.
+			 */
+			result = SERVER_UPDATE_AVAILABLE;
+		} else if (update_state == STATE_INSTALLED) {
+			/*
+			 * Upgrade is installed, but not activated, so we are able to confirm
+			 * the cancellation. There is no need to send any feedback, but we
+			 * want to exit STATE_WAIT.
+			 */
+			server_hawkbit.update_state = STATE_OK;
+			save_state((char*)STATE_KEY, STATE_OK);
+
+			result = SERVER_OK;
+			goto cleanup;
+		}
 	}
 
+	server_op_res_t response = SERVER_OK;
 	if (action_id != server_action_id) {
 		TRACE("Deployment changed on server: our id %d, on server %d",
 			action_id, server_action_id);
-	} else {
+	} else if (update_state != STATE_INSTALLED) {
 		response = handle_feedback(action_id, result, update_state, reply_result,
 					   reply_execution,
 					   numdetails == 0 ? 1 : numdetails, details);
@@ -1916,12 +1955,18 @@  static server_op_res_t server_activation_ipc(ipc_message *msg)
 	if ((response != SERVER_UPDATE_AVAILABLE) && (response != SERVER_OK))
 		result = SERVER_EERR;
 	else {
-		server_hawkbit.update_state = STATE_OK;
+		/*
+		 * If the update is installed and not activated, we don't want to start
+		 * processing the same action again - thus we stay in STATE_INSTALLED.
+		 */
+		update_state_t new_update_state = update_state == STATE_INSTALLED ?
+			STATE_INSTALLED : STATE_OK;
 
+		server_hawkbit.update_state = new_update_state;
 		/*
 		 * Save the state
 		 */
-		if ((result = save_state((char *)STATE_KEY, STATE_OK)) != SERVER_OK) {
+		if ((result = save_state_string((char *)STATE_KEY, new_update_state)) != SERVER_OK) {
 			ERROR("Error while resetting update state on persistent "
 			"storage.\n");
 		}