From 9df97d9a1ba42d672ac4576eaa7e45229de8374a Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Wed, 8 May 2024 09:31:50 +0200 Subject: [PATCH] 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 --- ...add-support-for-running-in-foregroun.patch | 2 +- ...tall_modules-to-use-ly_ctx_load_modu.patch | 2 +- ...-loading-etc-sysrepo-factory-default.patch | 2 +- ...l-add-support-for-Cfactory-d-running.patch | 2 +- ...NE-to-return-any-error-to-sysrepocfg.patch | 152 ++++++++++++++++++ 5 files changed, 156 insertions(+), 4 deletions(-) create mode 100644 patches/sysrepo/2.2.105/0005-Allow-SR_EV_DONE-to-return-any-error-to-sysrepocfg.patch diff --git a/patches/sysrepo/2.2.105/0001-sysrepo-plugind-add-support-for-running-in-foregroun.patch b/patches/sysrepo/2.2.105/0001-sysrepo-plugind-add-support-for-running-in-foregroun.patch index 708d3bf5..414649fb 100644 --- a/patches/sysrepo/2.2.105/0001-sysrepo-plugind-add-support-for-running-in-foregroun.patch +++ b/patches/sysrepo/2.2.105/0001-sysrepo-plugind-add-support-for-running-in-foregroun.patch @@ -1,7 +1,7 @@ From 49c660c412342e1af76735e75cd3f286ae655b82 Mon Sep 17 00:00:00 2001 From: Joachim Wiberg 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 diff --git a/patches/sysrepo/2.2.105/0002-Refactor-_sr_install_modules-to-use-ly_ctx_load_modu.patch b/patches/sysrepo/2.2.105/0002-Refactor-_sr_install_modules-to-use-ly_ctx_load_modu.patch index 2ac20ae2..0d1362d8 100644 --- a/patches/sysrepo/2.2.105/0002-Refactor-_sr_install_modules-to-use-ly_ctx_load_modu.patch +++ b/patches/sysrepo/2.2.105/0002-Refactor-_sr_install_modules-to-use-ly_ctx_load_modu.patch @@ -1,7 +1,7 @@ From 87224ddce4ed1a118b3efb24e0dc7167f8754c22 Mon Sep 17 00:00:00 2001 From: Joachim Wiberg 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 diff --git a/patches/sysrepo/2.2.105/0003-Add-support-for-loading-etc-sysrepo-factory-default.patch b/patches/sysrepo/2.2.105/0003-Add-support-for-loading-etc-sysrepo-factory-default.patch index 744336de..2bf649c7 100644 --- a/patches/sysrepo/2.2.105/0003-Add-support-for-loading-etc-sysrepo-factory-default.patch +++ b/patches/sysrepo/2.2.105/0003-Add-support-for-loading-etc-sysrepo-factory-default.patch @@ -1,7 +1,7 @@ From 2d3b9524b1448dd6a8ef83e4e101e509d2552dcc Mon Sep 17 00:00:00 2001 From: Joachim Wiberg 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 diff --git a/patches/sysrepo/2.2.105/0004-sysrepoctl-add-support-for-Cfactory-d-running.patch b/patches/sysrepo/2.2.105/0004-sysrepoctl-add-support-for-Cfactory-d-running.patch index 1c7236c8..cf322594 100644 --- a/patches/sysrepo/2.2.105/0004-sysrepoctl-add-support-for-Cfactory-d-running.patch +++ b/patches/sysrepo/2.2.105/0004-sysrepoctl-add-support-for-Cfactory-d-running.patch @@ -1,7 +1,7 @@ From 7726017815243818f07936e4ff3bb979e9291f04 Mon Sep 17 00:00:00 2001 From: Joachim Wiberg 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 diff --git a/patches/sysrepo/2.2.105/0005-Allow-SR_EV_DONE-to-return-any-error-to-sysrepocfg.patch b/patches/sysrepo/2.2.105/0005-Allow-SR_EV_DONE-to-return-any-error-to-sysrepocfg.patch new file mode 100644 index 00000000..7fa48ece --- /dev/null +++ b/patches/sysrepo/2.2.105/0005-Allow-SR_EV_DONE-to-return-any-error-to-sysrepocfg.patch @@ -0,0 +1,152 @@ +From 49b558067ab7c6f59ec84f14cc89201b4a7bde57 Mon Sep 17 00:00:00 2001 +From: Joachim Wiberg +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 +--- + 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 +