patches/sysrepo: add support for non-zero exit code from syrepocfg

This local patch of sysrepo adds support for detecting non-zero return
code from sysrepo callbacks during SR_EV_DONE, which upstream sysrepo
currently, as of v2.2.105, discards.

With this change failure to load startup-config results in the system
detecting the failure applying failure-config instead.

Fixes #429

Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
This commit is contained in:
Joachim Wiberg
2024-05-13 11:42:27 +02:00
committed by Tobias Waldekranz
parent d0cfe429c4
commit 9df97d9a1b
5 changed files with 156 additions and 4 deletions
@@ -1,7 +1,7 @@
From 49c660c412342e1af76735e75cd3f286ae655b82 Mon Sep 17 00:00:00 2001
From: Joachim Wiberg <troglobit@gmail.com>
Date: Tue, 28 Mar 2023 10:37:53 +0200
Subject: [PATCH 1/4] sysrepo-plugind: add support for running in foreground
Subject: [PATCH 1/5] sysrepo-plugind: add support for running in foreground
with syslog
Organization: Addiva Elektronik
@@ -1,7 +1,7 @@
From 87224ddce4ed1a118b3efb24e0dc7167f8754c22 Mon Sep 17 00:00:00 2001
From: Joachim Wiberg <troglobit@gmail.com>
Date: Wed, 7 Jun 2023 15:19:13 +0200
Subject: [PATCH 2/4] Refactor _sr_install_modules() to use
Subject: [PATCH 2/5] Refactor _sr_install_modules() to use
ly_ctx_load_module()
Organization: Addiva Elektronik
@@ -1,7 +1,7 @@
From 2d3b9524b1448dd6a8ef83e4e101e509d2552dcc Mon Sep 17 00:00:00 2001
From: Joachim Wiberg <troglobit@gmail.com>
Date: Fri, 16 Jun 2023 12:13:16 +0200
Subject: [PATCH 3/4] Add support for loading /etc/sysrepo/factory-default
Subject: [PATCH 3/5] Add support for loading /etc/sysrepo/factory-default
Organization: Addiva Elektronik
Follow-up to 3d07270 which adds support for compile-time defaults, this
@@ -1,7 +1,7 @@
From 7726017815243818f07936e4ff3bb979e9291f04 Mon Sep 17 00:00:00 2001
From: Joachim Wiberg <troglobit@gmail.com>
Date: Fri, 16 Jun 2023 16:17:27 +0200
Subject: [PATCH 4/4] sysrepoctl: add support for -Cfactory -d running
Subject: [PATCH 4/5] sysrepoctl: add support for -Cfactory -d running
Organization: Addiva Elektronik
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
@@ -0,0 +1,152 @@
From 49b558067ab7c6f59ec84f14cc89201b4a7bde57 Mon Sep 17 00:00:00 2001
From: Joachim Wiberg <troglobit@gmail.com>
Date: Tue, 7 May 2024 15:41:53 +0200
Subject: [PATCH 5/5] Allow SR_EV_DONE to return any error to sysrepocfg
Organization: Addiva Elektronik
Importing a system configuration with sysrepocfg the model callbacks do
their best to validate the configuration and prepare the transaction in
SR_EV_CHANGE. In cases when dealing with a complex system, like adding
a route, address, or other change using iproute2, we may still get into
trouble when actually sending off the transaction in SR_EV_DONE.
If something does go wrong we'd like to know this by the exit code of
sysrepocfg, not by inspecting logs or expected system state.
This patch is a clumsy way of forcing the (first) error to bubble up to
the surface and cause a non-zero exit code from sysrepocfg.
Signed-off-by: Joachim Wiberg <troglobit@gmail.com>
---
src/shm_sub.c | 36 ++++++++++++++++++++++++++++--------
src/shm_sub.h | 2 +-
src/sysrepo.c | 2 +-
3 files changed, 30 insertions(+), 10 deletions(-)
diff --git a/src/shm_sub.c b/src/shm_sub.c
index 99b2be37..298d7dd3 100644
--- a/src/shm_sub.c
+++ b/src/shm_sub.c
@@ -1586,7 +1586,7 @@ cleanup:
sr_error_info_t *
sr_shmsub_change_notify_change_done(struct sr_mod_info_s *mod_info, const char *orig_name, const void *orig_data,
- uint32_t timeout_ms)
+ uint32_t timeout_ms, sr_error_info_t **cb_err_info)
{
sr_error_info_t *err_info = NULL;
struct sr_mod_info_mod_s *mod = NULL;
@@ -1717,12 +1717,19 @@ sr_shmsub_change_notify_change_done(struct sr_mod_info_s *mod_info, const char *
sr_rwunlock(&nsub->sub_shm->lock, 0, SR_LOCK_WRITE, cid, __func__);
nsub->lock = SR_LOCK_NONE;
- /* we do not care about an error */
- sr_errinfo_free(&nsub->cb_err_info);
-
SR_LOG_INF("EV ORIGIN: \"%s\" \"%s\" ID %" PRIu32 " priority %" PRIu32 " succeeded.",
nsub->mod->ly_mod->name, sr_ev2str(SR_SUB_EV_DONE), nsub->mod->request_id, nsub->cur_priority);
+ /*
+ * unexpected critical error, merge and cleanup, let
+ * sysrepocfg caller handle undefined system state.
+ */
+ if (nsub->cb_err_info) {
+ sr_errinfo_merge(cb_err_info, nsub->cb_err_info);
+ nsub->cb_err_info = NULL;
+ goto cleanup;
+ }
+
nsub->pending_event = 0;
}
} while (1);
@@ -3167,7 +3174,7 @@ sr_shmsub_change_listen_check_update_edit(sr_session_ctx_t *ev_sess, const char
sr_error_info_t *
sr_shmsub_change_listen_process_module_events(struct modsub_change_s *change_subs, sr_conn_ctx_t *conn)
{
- sr_error_info_t *err_info = NULL;
+ sr_error_info_t *err_info = NULL, *err_tmp;
uint32_t i, data_len = 0, valid_subscr_count;
char *data = NULL, *shm_data_ptr;
int ret = SR_ERR_OK;
@@ -3289,6 +3296,11 @@ process_event:
}
break;
}
+ } else if (sub_info.event == SR_SUB_EV_DONE) {
+ if (ret && err_code == SR_ERR_OK) {
+ /* unexpected, callback actually failed, save for later. */
+ err_code = ret;
+ }
}
/* subscription processed this event */
@@ -3330,6 +3342,11 @@ process_event:
}
break;
case SR_SUB_EV_DONE:
+ if (err_code) {
+ /* prepare unexepected error from session to be written to SHM */
+ sr_errinfo_new(&err_info, err_code, "Oups, error detected in SR_EV_DONE");
+ }
+ break;
case SR_SUB_EV_ABORT:
/* nothing to do */
break;
@@ -3349,14 +3366,17 @@ process_event:
/* SUB WRITE URGE LOCK */
if (sr_shmsub_change_listen_relock(multi_sub_shm, SR_LOCK_WRITE_URGE, &sub_info, change_sub, change_subs->module_name,
- ret, ev_sess, &err_info)) {
+ ret, ev_sess, &err_tmp)) {
+ if (err_tmp)
+ err_info = err_tmp;
goto cleanup;
}
sub_lock = SR_LOCK_WRITE_URGE;
/* finish event */
- if ((err_info = sr_shmsub_multi_listen_write_event(multi_sub_shm, valid_subscr_count, err_code, &shm_data_sub, data,
+ if ((err_tmp = sr_shmsub_multi_listen_write_event(multi_sub_shm, valid_subscr_count, err_code, &shm_data_sub, data,
data_len, change_subs->module_name, err_code ? "fail" : "success"))) {
+ err_info = err_tmp;
goto cleanup;
}
@@ -3886,7 +3906,7 @@ finish_iter:
sr_errinfo_free(&cb_err_info);
/* publish "done" event */
- if ((err_info = sr_shmsub_change_notify_change_done(&mod_info, NULL, NULL, SR_CHANGE_CB_TIMEOUT))) {
+ if ((err_info = sr_shmsub_change_notify_change_done(&mod_info, NULL, NULL, SR_CHANGE_CB_TIMEOUT, &cb_err_info))) {
goto cleanup_unlock;
}
diff --git a/src/shm_sub.h b/src/shm_sub.h
index 3056e4c5..460ad927 100644
--- a/src/shm_sub.h
+++ b/src/shm_sub.h
@@ -144,7 +144,7 @@ sr_error_info_t *sr_shmsub_change_notify_change(struct sr_mod_info_s *mod_info,
* @return err_info, NULL on success.
*/
sr_error_info_t *sr_shmsub_change_notify_change_done(struct sr_mod_info_s *mod_info, const char *orig_name,
- const void *orig_data, uint32_t timeout_ms);
+ const void *orig_data, uint32_t timeout_ms, sr_error_info_t **cb_err_info);
/**
* @brief Notify about (generate) a change "abort" event.
diff --git a/src/sysrepo.c b/src/sysrepo.c
index ca2a20b7..e7c1fd0e 100644
--- a/src/sysrepo.c
+++ b/src/sysrepo.c
@@ -3801,7 +3801,7 @@ store:
}
/* publish "done" event, all changes were applied */
- if ((err_info = sr_shmsub_change_notify_change_done(mod_info, orig_name, orig_data, timeout_ms))) {
+ if ((err_info = sr_shmsub_change_notify_change_done(mod_info, orig_name, orig_data, timeout_ms, cb_err_info))) {
goto cleanup;
}
--
2.34.1