[Asterisk-code-review] loader: Rework load resource list. (asterisk[master])
Jenkins2
asteriskteam at digium.com
Mon Jan 22 16:31:29 CST 2018
Jenkins2 has submitted this change and it was merged. ( https://gerrit.asterisk.org/7987 )
Change subject: loader: Rework load_resource_list.
......................................................................
loader: Rework load_resource_list.
Use a single loop in a loop to scan the resource list attempting to
dlopen each module. The inner loop is repeated until it doesn't do any
work, then it is run one more time to allow printing of error messages.
Change-Id: I60c15cd57ff9680b62e2a94c7519401fa4a38e45
---
M main/loader.c
1 file changed, 35 insertions(+), 73 deletions(-)
Approvals:
Richard Mudgett: Looks good to me, but someone else must approve
George Joseph: Looks good to me, approved
Jenkins2: Approved for Submit
diff --git a/main/loader.c b/main/loader.c
index 7e629ca..ec8a184 100644
--- a/main/loader.c
+++ b/main/loader.c
@@ -1421,7 +1421,7 @@
*
* If the module_vector is not provided, the module's load function will be executed
* immediately */
-static enum ast_module_load_result load_resource(const char *resource_name, unsigned int suppress_logging, struct module_vector *resource_heap, int required)
+static enum ast_module_load_result load_resource(const char *resource_name, unsigned int suppress_logging, struct module_vector *module_priorities, int required)
{
struct ast_module *mod;
enum ast_module_load_result res = AST_MODULE_LOAD_SUCCESS;
@@ -1455,8 +1455,8 @@
mod->flags.declined = 0;
- if (resource_heap) {
- if (AST_VECTOR_ADD_SORTED(resource_heap, mod, module_vector_cmp)) {
+ if (module_priorities) {
+ if (AST_VECTOR_ADD_SORTED(module_priorities, mod, module_vector_cmp)) {
goto prestart_error;
}
res = AST_MODULE_LOAD_PRIORITY;
@@ -1635,115 +1635,77 @@
*/
static int load_resource_list(struct load_order *load_order, int *mod_count)
{
- struct module_vector resource_heap;
+ struct module_vector module_priorities;
struct load_order_entry *order;
- struct load_retries load_retries;
+ int attempt = 0;
int count = 0;
int res = 0;
- int i = 0;
-#define LOAD_RETRIES 4
+ int didwork;
+ int lasttry = 0;
- AST_LIST_HEAD_INIT_NOLOCK(&load_retries);
-
- if (AST_VECTOR_INIT(&resource_heap, 500)) {
+ if (AST_VECTOR_INIT(&module_priorities, 500)) {
ast_log(LOG_ERROR, "Failed to initialize module loader.\n");
return -1;
}
- /* first, add find and add modules to heap */
- AST_LIST_TRAVERSE_SAFE_BEGIN(load_order, order, entry) {
- enum ast_module_load_result lres;
+ while (!res) {
+ didwork = 0;
- /* Suppress log messages unless this is the last pass */
- lres = load_resource(order->resource, 1, &resource_heap, order->required);
- ast_debug(3, "PASS 0: %-46s %d\n", order->resource, lres);
- switch (lres) {
- case AST_MODULE_LOAD_SUCCESS:
- /* We're supplying a heap so SUCCESS isn't possible but we still have to test for it. */
- break;
- case AST_MODULE_LOAD_FAILURE:
- case AST_MODULE_LOAD_DECLINE:
- /*
- * DECLINE or FAILURE means there was an issue with dlopen or module_register
- * which might be retryable. LOAD_FAILURE only happens for required modules
- * but we're still going to retry. We need to remove the entry from the
- * load_order list and add it to the load_retries list.
- */
- AST_LIST_REMOVE_CURRENT(entry);
- AST_LIST_INSERT_TAIL(&load_retries, order, entry);
- break;
- case AST_MODULE_LOAD_SKIP:
- /*
- * SKIP means that dlopen worked but global_symbols was set and this module doesn't qualify.
- * Leave it in load_order for the next call of load_resource_list.
- */
- break;
- case AST_MODULE_LOAD_PRIORITY:
- /* load_resource worked and the module was added to the priority vector */
- AST_LIST_REMOVE_CURRENT(entry);
- ast_free(order->resource);
- ast_free(order);
- break;
- }
- }
- AST_LIST_TRAVERSE_SAFE_END;
-
- /* Retry the failures until the list is empty or we reach LOAD_RETRIES */
- for (i = 0; !AST_LIST_EMPTY(&load_retries) && i < LOAD_RETRIES; i++) {
- AST_LIST_TRAVERSE_SAFE_BEGIN(&load_retries, order, entry) {
+ AST_LIST_TRAVERSE_SAFE_BEGIN(load_order, order, entry) {
enum ast_module_load_result lres;
/* Suppress log messages unless this is the last pass */
- lres = load_resource(order->resource, (i < LOAD_RETRIES - 1), &resource_heap, order->required);
- ast_debug(3, "PASS %d %-46s %d\n", i + 1, order->resource, lres);
+ lres = load_resource(order->resource, !lasttry, &module_priorities, order->required);
+ ast_debug(3, "PASS %d: %-46s %d\n", attempt, order->resource, lres);
switch (lres) {
- /* These are all retryable. */
case AST_MODULE_LOAD_SUCCESS:
+ case AST_MODULE_LOAD_SKIP:
+ /* We're supplying module_priorities so SUCCESS isn't possible but we
+ * still have to test for it. SKIP is only used when we try to start a
+ * module that is missing dependencies. */
+ break;
case AST_MODULE_LOAD_DECLINE:
break;
case AST_MODULE_LOAD_FAILURE:
/* LOAD_FAILURE only happens for required modules */
- if (i == LOAD_RETRIES - 1) {
- /* This was the last chance to load a required module*/
+ if (lasttry) {
+ /* This run is just to print errors. */
ast_log(LOG_ERROR, "*** Failed to load module %s - Required\n", order->resource);
fprintf(stderr, "*** Failed to load module %s - Required\n", order->resource);
res = -2;
- goto done;
}
- break;;
- case AST_MODULE_LOAD_SKIP:
- /*
- * SKIP means that dlopen worked but global_symbols was set and this module
- * doesn't qualify. Put it back in load_order for the next call of
- * load_resource_list.
- */
- AST_LIST_REMOVE_CURRENT(entry);
- AST_LIST_INSERT_TAIL(load_order, order, entry);
break;
case AST_MODULE_LOAD_PRIORITY:
- /* load_resource worked and the module was added to the priority heap */
+ /* load_resource worked and the module was added to module_priorities */
AST_LIST_REMOVE_CURRENT(entry);
ast_free(order->resource);
ast_free(order);
+ didwork = 1;
break;
}
}
AST_LIST_TRAVERSE_SAFE_END;
+
+ if (!didwork) {
+ if (lasttry) {
+ break;
+ }
+ /* We know the next try is going to fail, it's only being performed
+ * so we can print errors. */
+ lasttry = 1;
+ }
+ attempt++;
}
- res = start_resource_list(&resource_heap, &count);
-
-done:
- while ((order = AST_LIST_REMOVE_HEAD(&load_retries, entry))) {
- ast_free(order->resource);
- ast_free(order);
+ if (!res) {
+ res = start_resource_list(&module_priorities, &count);
}
if (mod_count) {
*mod_count += count;
}
- AST_VECTOR_FREE(&resource_heap);
+ AST_VECTOR_FREE(&module_priorities);
return res;
}
--
To view, visit https://gerrit.asterisk.org/7987
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I60c15cd57ff9680b62e2a94c7519401fa4a38e45
Gerrit-Change-Number: 7987
Gerrit-PatchSet: 3
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180122/df49af75/attachment.html>
More information about the asterisk-code-review
mailing list