[Asterisk-code-review] voicemail: Move app voicemail / res mwi external conflict to... (asterisk[master])

George Joseph asteriskteam at digium.com
Wed Dec 30 11:21:11 CST 2015


George Joseph has uploaded a new change for review.

  https://gerrit.asterisk.org/1885

Change subject: voicemail: Move app_voicemail / res_mwi_external conflict to runtime
......................................................................

voicemail: Move app_voicemail / res_mwi_external conflict to runtime

The menuselect conflict between app_voicemail and res_mwi_external
makes it hard to package 1 version of Asterisk.  There no actual
build dependencies between the 2 so moving this check to runtime
seems like a better solution.

The ast_vm_register and ast_vm_greeter_register functions in app.c
were modified to return AST_MODULE_LOAD_SKIP if there was already
a voicemail module registered instead of -1. The modules' load_module
functions were then modified to check for SKIP and return SKIP
instead of the -1 in response to the load.  Since the modules were
originally returning the -1 on load, it was being interpreted as
AST_MODULE_LOAD_FAILURE which was causing Asterisk to stop so this
needed to be cleaned up anyway.

Now you can build both and use modules.conf to decide which voicemail
implementation to load.

The default menuselect options still build app_voicemail and not
res_mwi_external but if both ARE built, res_mwi_external will load
first and become the voicemail provider unless module.conf rules
prevent it.  This is noted in CHANGES.

Change-Id: I7d98d4e8a3b87b8df9e51c2608f0da6ddfb89247
---
M CHANGES
M apps/app_voicemail.c
M include/asterisk/app.h
M main/app.c
M res/res_mwi_external.c
5 files changed, 66 insertions(+), 13 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/85/1885/1

diff --git a/CHANGES b/CHANGES
index 9421f0b..e1694d0 100644
--- a/CHANGES
+++ b/CHANGES
@@ -205,6 +205,14 @@
 --- Functionality changes from Asterisk 13.6.0 to Asterisk 13.7.0 ------------
 ------------------------------------------------------------------------------
 
+Voicemail
+------------------
+ * app_voicemail and res_mwi_external can now be built together.  The default
+   remains to build app_voicemail and not res_mwi_external but if they are
+   both built, the load order will cause res_mwi_external to load first and
+   app_voicemail will be skipped.  Use 'preload=app_voicemail.so' in
+   modules.conf to force app_voicemail to be the voicemail provider.
+
 Codecs
 ------------------
  * Added format attribute negotiation for the VP8 video codec. Format attribute
diff --git a/apps/app_voicemail.c b/apps/app_voicemail.c
index f2f7bad..44ed152 100644
--- a/apps/app_voicemail.c
+++ b/apps/app_voicemail.c
@@ -48,7 +48,6 @@
 
 /*** MODULEINFO
 	<defaultenabled>yes</defaultenabled>
-	<conflict>res_mwi_external</conflict>
 	<use type="module">res_adsi</use>
 	<use type="module">res_smdi</use>
 	<support_level>core</support_level>
@@ -14702,10 +14701,16 @@
  *
  * Module loading including tests for configuration or dependencies.
  * This function can return AST_MODULE_LOAD_FAILURE, AST_MODULE_LOAD_DECLINE,
- * or AST_MODULE_LOAD_SUCCESS. If a dependency or environment variable fails
- * tests return AST_MODULE_LOAD_FAILURE. If the module can not load the 
- * configuration file or other non-critical problem return 
- * AST_MODULE_LOAD_DECLINE. On success return AST_MODULE_LOAD_SUCCESS.
+ * AST_MODULE_LOAD_SKIP or AST_MODULE_LOAD_SUCCESS.
+ *
+ * If a dependency or environment variable fails tests return AST_MODULE_LOAD_FAILURE.
+ *
+ * If the module can not load the configuration file or other non-critical problem return
+ * AST_MODULE_LOAD_DECLINE.
+ *
+ * If another module already registered for voicemail or greeter, return AST_MODULE_LOAD_SKIP.
+ *
+ * On success return AST_MODULE_LOAD_SUCCESS.
  */
 static int load_module(void)
 {
@@ -14724,8 +14729,10 @@
 		ast_log(AST_LOG_WARNING, "failed to reference mwi subscription taskprocessor.  MWI will not work\n");
 	}
 
-	if ((res = load_config(0)))
+	if ((res = load_config(0))) {
+		unload_module();
 		return res;
+	}
 
 	res = ast_register_application_xml(app, vm_exec);
 	res |= ast_register_application_xml(app2, vm_execmain);
@@ -14746,9 +14753,32 @@
 	res |= AST_TEST_REGISTER(test_voicemail_vm_info);
 #endif
 
-	res |= ast_vm_register(&vm_table);
-	res |= ast_vm_greeter_register(&vm_greeter_table);
 	if (res) {
+		unload_module();
+		return AST_MODULE_LOAD_DECLINE;
+	}
+
+	/* ast_vm_register may return SKIP if another module registered for vm */
+	res = ast_vm_register(&vm_table);
+	if (res == AST_MODULE_LOAD_SKIP) {
+		ast_log(LOG_WARNING, "Skipping.  Another voicemail provider is already registered\n");
+		unload_module();
+		return res;
+	} else if (res) {
+		ast_log(LOG_ERROR, "Failure registering as a voicemail provider\n");
+		unload_module();
+		return res;
+	}
+
+	/* ast_vm_greeter_register may return SKIP if another module registered as a greeter */
+	res = ast_vm_greeter_register(&vm_greeter_table);
+	if (res == AST_MODULE_LOAD_SKIP) {
+		ast_log(LOG_WARNING, "Skipping.  Another voicemail greeter is already registered\n");
+		unload_module();
+		return res;
+	} else if (res) {
+		ast_log(LOG_ERROR, "Failure registering as a voicemail greeter\n");
+		unload_module();
 		return res;
 	}
 
@@ -14762,7 +14792,7 @@
 	ast_realtime_require_field("voicemail", "uniqueid", RQ_UINTEGER3, 11, "password", RQ_CHAR, 10, SENTINEL);
 	ast_realtime_require_field("voicemail_data", "filename", RQ_CHAR, 30, "duration", RQ_UINTEGER3, 5, SENTINEL);
 
-	return res;
+	return AST_MODULE_LOAD_SUCCESS;
 }
 
 static int dialout(struct ast_channel *chan, struct ast_vm_user *vmu, char *num, char *outgoing_context) 
diff --git a/include/asterisk/app.h b/include/asterisk/app.h
index 3975fda..87f46c3 100644
--- a/include/asterisk/app.h
+++ b/include/asterisk/app.h
@@ -580,6 +580,7 @@
  *
  * \retval 0 on success.
  * \retval -1 on error.
+ * \retval -2 (AST_MODULE_LOAD_SKIP) if there's already another provider registered.
  */
 int __ast_vm_register(const struct ast_vm_functions *vm_table, struct ast_module *module);
 
@@ -648,6 +649,7 @@
  *
  * \retval 0 on success.
  * \retval -1 on error.
+ * \retval -2 (AST_MODULE_LOAD_SKIP) if there's already another greeter registered.
  */
 int __ast_vm_greeter_register(const struct ast_vm_greeter_functions *vm_table, struct ast_module *module);
 
diff --git a/main/app.c b/main/app.c
index dabf15d..68a7396 100644
--- a/main/app.c
+++ b/main/app.c
@@ -494,7 +494,7 @@
 	if (table) {
 		ast_log(LOG_WARNING, "Voicemail provider already registered by %s.\n",
 			table->module_name);
-		return -1;
+		return AST_MODULE_LOAD_SKIP ;
 	}
 
 	table = ao2_alloc_options(sizeof(*table), NULL, AO2_ALLOC_OPT_LOCK_NOLOCK);
@@ -605,7 +605,7 @@
 	if (table) {
 		ast_log(LOG_WARNING, "Voicemail greeter provider already registered by %s.\n",
 			table->module_name);
-		return -1;
+		return AST_MODULE_LOAD_SKIP;
 	}
 
 	table = ao2_alloc_options(sizeof(*table), NULL, AO2_ALLOC_OPT_LOCK_NOLOCK);
diff --git a/res/res_mwi_external.c b/res/res_mwi_external.c
index 9722822..fc34622 100644
--- a/res/res_mwi_external.c
+++ b/res/res_mwi_external.c
@@ -33,7 +33,6 @@
 
 /*** MODULEINFO
 	<defaultenabled>no</defaultenabled>
-	<conflict>app_voicemail</conflict>
 	<support_level>core</support_level>
  ***/
 
@@ -935,16 +934,30 @@
 
 static int load_module(void)
 {
+	int res;
+
 	if (mwi_sorcery_init()
 		|| ast_sorcery_observer_add(mwi_sorcery, MWI_MAILBOX_TYPE, &mwi_observers)
 #if defined(MWI_DEBUG_CLI)
 		|| ast_cli_register_multiple(mwi_cli, ARRAY_LEN(mwi_cli))
 #endif	/* defined(MWI_DEBUG_CLI) */
-		|| ast_vm_register(&vm_table)) {
+		) {
 		unload_module();
 		return AST_MODULE_LOAD_DECLINE;
 	}
 
+	/* ast_vm_register may return SKIP if another module registered for vm */
+	res = ast_vm_register(&vm_table);
+	if (res == AST_MODULE_LOAD_SKIP) {
+		ast_log(LOG_WARNING, "Skipping.  Another voicemail provider is already registered\n");
+		unload_module();
+		return res;
+	} else if (res) {
+		ast_log(LOG_ERROR, "Failure registering as a voicemail provider\n");
+		unload_module();
+		return res;
+	}
+
 	/* Post initial MWI count events. */
 	mwi_initial_events();
 

-- 
To view, visit https://gerrit.asterisk.org/1885
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7d98d4e8a3b87b8df9e51c2608f0da6ddfb89247
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: George Joseph <george.joseph at fairview5.com>



More information about the asterisk-code-review mailing list