[Asterisk-code-review] loader: Fix startup issues. (asterisk[master])
Corey Farrell
asteriskteam at digium.com
Mon Jul 16 10:21:19 CDT 2018
Corey Farrell has uploaded this change for review. ( https://gerrit.asterisk.org/9446
Change subject: loader: Fix startup issues.
......................................................................
loader: Fix startup issues.
* Merge the preload and load stages, use load ordering to try preload's
first. This fixes an issue where `preload=res_config_curl` would fail
unless res_curl and func_curl were also preloaded. Now it is only
required that those modules be loaded during startup: autoload or
regular load is good enough.
* The configuration option `require` and `preload-require` were only
effective if the modules failed to load. These options will now abort
Asterisk startup if required modules fail to reach the 'Running'
state.
* Missing or invalid 'module.conf' did not prevent startup. Asterisk
doesn't do anything without modules so this a fatal error.
Change-Id: Ie4176699133f0e3a823b43f90c3348677e43a5f3
---
M include/asterisk/_private.h
M main/asterisk.c
M main/loader.c
3 files changed, 129 insertions(+), 53 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/46/9446/1
diff --git a/include/asterisk/_private.h b/include/asterisk/_private.h
index d19c589..89a8f54 100644
--- a/include/asterisk/_private.h
+++ b/include/asterisk/_private.h
@@ -20,7 +20,7 @@
void set_asterisk_conf_path(const char *path);
void set_socket_path(const char *path);
-int load_modules(unsigned int); /*!< Provided by loader.c */
+int load_modules(void); /*!< Provided by loader.c */
int modules_shutdown(void); /*!< Provided by loader.c */
int load_pbx(void); /*!< Provided by pbx.c */
int load_pbx_builtins(void); /*!< Provided by pbx_builtins.c */
diff --git a/main/asterisk.c b/main/asterisk.c
index 7ff167e..2fb32ac 100644
--- a/main/asterisk.c
+++ b/main/asterisk.c
@@ -4139,10 +4139,7 @@
check_init(ast_local_init(), "Local Proxy Channel Driver");
/* We should avoid most config loads before this point as they can't use realtime. */
- check_init(load_modules(1), "Module Preload");
-
- /* Load remaining modules */
- check_init(load_modules(0), "Module");
+ check_init(load_modules(), "Module");
/*
* This has to load after the dynamic modules load, as items in the media
diff --git a/main/loader.c b/main/loader.c
index bc78962..3fa6db9 100644
--- a/main/loader.c
+++ b/main/loader.c
@@ -181,6 +181,10 @@
unsigned int keepuntilshutdown:1;
/*! The module is built-in. */
unsigned int builtin:1;
+ /*! The admin has declared this module is required. */
+ unsigned int required:1;
+ /*! This module is marked for preload. */
+ unsigned int preload:1;
} flags;
AST_DLLIST_ENTRY(ast_module) entry;
char resource[0];
@@ -223,12 +227,20 @@
static int module_vector_cmp(struct ast_module *a, struct ast_module *b)
{
+ int preload_diff = (int)b->flags.preload - (int)a->flags.preload;
/* if load_pri is not set, default is 128. Lower is better */
int a_pri = ast_test_flag(a->info, AST_MODFLAG_LOAD_ORDER)
? a->info->load_pri : AST_MODPRI_DEFAULT;
int b_pri = ast_test_flag(b->info, AST_MODFLAG_LOAD_ORDER)
? b->info->load_pri : AST_MODPRI_DEFAULT;
+ if (preload_diff) {
+ /* -1 preload a but not b */
+ /* 0 preload both or neither */
+ /* 1 preload b but not a */
+ return preload_diff;
+ }
+
/*
* Returns comparison values for a vector sorted by priority.
* <0 a_pri < b_pri
@@ -1469,6 +1481,9 @@
break;
case AST_MODULE_LOAD_DECLINE:
mod->flags.declined = 1;
+ if (mod->flags.required) {
+ res = AST_MODULE_LOAD_FAILURE;
+ }
break;
case AST_MODULE_LOAD_FAILURE:
case AST_MODULE_LOAD_SKIP: /* modules should never return this value */
@@ -1494,7 +1509,8 @@
*
* 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 *module_priorities, 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, int preload)
{
struct ast_module *mod;
enum ast_module_load_result res = AST_MODULE_LOAD_SUCCESS;
@@ -1519,6 +1535,9 @@
}
}
+ mod->flags.required |= required;
+ mod->flags.preload |= preload;
+
if (inspect_module(mod)) {
goto prestart_error;
}
@@ -1554,7 +1573,7 @@
{
int res;
AST_DLLIST_LOCK(&module_list);
- res = load_resource(resource_name, 0, NULL, 0);
+ res = load_resource(resource_name, 0, NULL, 0, 0);
if (!res) {
ast_test_suite_event_notify("MODULE_LOAD", "Message: %s", resource_name);
}
@@ -1566,12 +1585,13 @@
struct load_order_entry {
char *resource;
int required;
+ int preload;
AST_LIST_ENTRY(load_order_entry) entry;
};
AST_LIST_HEAD_NOLOCK(load_order, load_order_entry);
-static struct load_order_entry *add_to_load_order(const char *resource, struct load_order *load_order, int required)
+static struct load_order_entry *add_to_load_order(const char *resource, struct load_order *load_order, int required, int preload)
{
struct load_order_entry *order;
size_t resource_baselen = resource_name_baselen(resource);
@@ -1581,20 +1601,26 @@
/* Make sure we have the proper setting for the required field
(we might have both load= and required= lines in modules.conf) */
order->required |= required;
- return NULL;
+ order->preload |= preload;
+ return order;
}
}
- if (!(order = ast_calloc(1, sizeof(*order))))
+ order = ast_calloc(1, sizeof(*order));
+ if (!order) {
+ ast_log(LOG_ERROR, "Allocation failure adding module to load priority list.\n");
return NULL;
+ }
order->resource = ast_strdup(resource);
if (!order->resource) {
+ ast_log(LOG_ERROR, "Allocation failure adding module to load priority list.\n");
ast_free(order);
return NULL;
}
order->required = required;
+ order->preload = preload;
AST_LIST_INSERT_TAIL(load_order, order, entry);
return order;
@@ -1745,7 +1771,7 @@
enum ast_module_load_result lres;
/* Suppress log messages unless this is the last pass */
- lres = load_resource(order->resource, !lasttry, &module_priorities, order->required);
+ lres = load_resource(order->resource, !lasttry, &module_priorities, order->required, order->preload);
ast_debug(3, "PASS %d: %-46s %d\n", attempt, order->resource, lres);
switch (lres) {
case AST_MODULE_LOAD_SUCCESS:
@@ -1799,24 +1825,9 @@
return res;
}
-int load_modules(unsigned int preload_only)
+static int loader_builtin_init(struct load_order *load_order)
{
- struct ast_config *cfg;
- struct load_order_entry *order;
- struct ast_variable *v;
- unsigned int load_count;
- struct load_order load_order;
- int res = 0;
- struct ast_flags config_flags = { 0 };
- int modulecount = 0;
- struct dirent *dirent;
- DIR *dir;
-
- ast_verb(1, "Asterisk Dynamic Loader Starting:\n");
-
- AST_LIST_HEAD_INIT_NOLOCK(&load_order);
-
- AST_DLLIST_LOCK(&module_list);
+ struct ast_module *mod;
/*
* All built-in modules have registered the first time, now it's time to complete
@@ -1829,42 +1840,79 @@
ast_module_register(resource_being_loaded->info);
}
- if (!preload_only) {
- struct ast_module *mod;
+ /* Add all built-in modules to the load order. */
+ AST_DLLIST_TRAVERSE(&module_list, mod, entry) {
+ if (!mod->flags.builtin) {
+ continue;
+ }
- /* Add all built-in modules to the load order. */
- AST_DLLIST_TRAVERSE(&module_list, mod, entry) {
- if (!mod->flags.builtin) {
- continue;
- }
-
- add_to_load_order(mod->resource, &load_order, 0);
+ /* Built-in modules are not preloaded, most have an early load priority. */
+ if (!add_to_load_order(mod->resource, load_order, 0, 0)) {
+ return -1;
}
}
+ return 0;
+}
+
+static int loader_config_init(struct load_order *load_order)
+{
+ int res = -1;
+ struct load_order_entry *order;
+ struct ast_config *cfg;
+ struct ast_variable *v;
+ struct ast_flags config_flags = { 0 };
+
cfg = ast_config_load2(AST_MODULE_CONFIG, "" /* core, can't reload */, config_flags);
if (cfg == CONFIG_STATUS_FILEMISSING || cfg == CONFIG_STATUS_FILEINVALID) {
- ast_log(LOG_WARNING, "No '%s' found, no modules will be loaded.\n", AST_MODULE_CONFIG);
- goto done;
+ ast_log(LOG_WARNING, "'%s' invalid or missing.\n", AST_MODULE_CONFIG);
+
+ return -1;
}
/* first, find all the modules we have been explicitly requested to load */
for (v = ast_variable_browse(cfg, "modules"); v; v = v->next) {
- if (!strcasecmp(v->name, preload_only ? "preload" : "load")) {
- add_to_load_order(v->value, &load_order, 0);
+ int required;
+ int preload = 0;
+
+ if (!strncasecmp(v->name, "preload", strlen("preload"))) {
+ preload = 1;
+ if (!strcasecmp(v->name, "preload")) {
+ required = 0;
+ } else if (!strcasecmp(v->name, "preload-require")) {
+ required = 1;
+ } else {
+ ast_log(LOG_ERROR, "Unknown configuration option '%s'", v->name);
+ goto done;
+ }
+ } else if (!strcasecmp(v->name, "load")) {
+ required = 0;
+ } else if (!strcasecmp(v->name, "require")) {
+ required = 1;
+ } else if (!strcasecmp(v->name, "noload") || !strcasecmp(v->name, "autoload")) {
+ continue;
+ } else {
+ ast_log(LOG_ERROR, "Unknown configuration option '%s'", v->name);
+ goto done;
}
- if (!strcasecmp(v->name, preload_only ? "preload-require" : "require")) {
- /* Add the module to the list and make sure it's required */
- add_to_load_order(v->value, &load_order, 1);
+
+ if (required) {
ast_debug(2, "Adding module to required list: %s (%s)\n", v->value, v->name);
}
+
+ if (!add_to_load_order(v->value, load_order, required, preload)) {
+ goto done;
+ }
}
/* check if 'autoload' is on */
- if (!preload_only && ast_true(ast_variable_retrieve(cfg, "modules", "autoload"))) {
+ if (ast_true(ast_variable_retrieve(cfg, "modules", "autoload"))) {
/* if we are allowed to load dynamic modules, scan the directory for
for all available modules and add them as well */
- if ((dir = opendir(ast_config_AST_MODULE_DIR))) {
+ DIR *dir = opendir(ast_config_AST_MODULE_DIR);
+ struct dirent *dirent;
+
+ if (dir) {
while ((dirent = readdir(dir))) {
int ld = strlen(dirent->d_name);
@@ -1881,14 +1929,16 @@
if (find_resource(dirent->d_name, 0))
continue;
- add_to_load_order(dirent->d_name, &load_order, 0);
+ if (!add_to_load_order(dirent->d_name, load_order, 0, 0)) {
+ closedir(dir);
+ goto done;
+ }
}
closedir(dir);
} else {
- if (!ast_opt_quiet)
- ast_log(LOG_WARNING, "Unable to open modules directory '%s'.\n",
- ast_config_AST_MODULE_DIR);
+ ast_log(LOG_ERROR, "Unable to open modules directory '%s'.\n", ast_config_AST_MODULE_DIR);
+ goto done;
}
}
@@ -1902,8 +1952,13 @@
}
baselen = resource_name_baselen(v->value);
- AST_LIST_TRAVERSE_SAFE_BEGIN(&load_order, order, entry) {
+ AST_LIST_TRAVERSE_SAFE_BEGIN(load_order, order, entry) {
if (!resource_name_match(v->value, baselen, order->resource)) {
+ if (order->required) {
+ ast_log(LOG_ERROR, "%s is configured with '%s' and 'noload', this is impossible.\n",
+ v->value, order->preload ? "preload-require" : "require");
+ goto done;
+ }
AST_LIST_REMOVE_CURRENT(entry);
ast_free(order->resource);
ast_free(order);
@@ -1912,10 +1967,34 @@
AST_LIST_TRAVERSE_SAFE_END;
}
- /* we are done with the config now, all the information we need is in the
- load_order list */
+ res = 0;
+done:
ast_config_destroy(cfg);
+ return res;
+}
+
+int load_modules(void)
+{
+ struct load_order_entry *order;
+ unsigned int load_count;
+ struct load_order load_order;
+ int res = 0;
+ int modulecount = 0;
+
+ ast_verb(1, "Asterisk Dynamic Loader Starting:\n");
+
+ AST_LIST_HEAD_INIT_NOLOCK(&load_order);
+ AST_DLLIST_LOCK(&module_list);
+
+ if ((res = loader_builtin_init(&load_order))) {
+ goto done;
+ }
+
+ if ((res = loader_config_init(&load_order))) {
+ goto done;
+ }
+
load_count = 0;
AST_LIST_TRAVERSE(&load_order, order, entry)
load_count++;
--
To view, visit https://gerrit.asterisk.org/9446
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie4176699133f0e3a823b43f90c3348677e43a5f3
Gerrit-Change-Number: 9446
Gerrit-PatchSet: 1
Gerrit-Owner: Corey Farrell <git at cfware.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180716/ec45b874/attachment-0001.html>
More information about the asterisk-code-review
mailing list