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