[Home]

Summary:ASTERISK-07102: [patch] asterisk should not fail on chan_zap config problems
Reporter:Tzafrir Cohen (tzafrir)Labels:
Date Opened:2006-06-05 18:23:43Date Closed:2011-06-07 14:07:49
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Channels/chan_zap
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) chanzap_noerr_85514.diff
( 1) chanzap_noerror.diff
Description:Currently chan_zap considers many of its configuration problems fata and fails its load. The result is a failiure of loading Asterisk.

The attached patch converts all of those to non-fatal errors (makes the module return 0).

****** ADDITIONAL INFORMATION ******

All of the changes were made in setup_zap() . I used kernel-style goto error-handling and thus this patch actually reduces the over-all lines-count of the chan_zap.c by 5.
Comments:By: Russell Bryant (russell) 2006-06-06 08:56:54

I would consider all of these errors cases that prevent chan_zap from functioning normally, which makes returning -1 the appropriate thing to do.  If you still feel it should be changed, I'd bring it up on the asterisk-dev mailing list and see what others think about it.
By: Tzafrir Cohen (tzafrir) 2007-05-12 09:10:59

Aparantly no objection on asterisk-dev to reopen this issue.

Three other issues I've noticed:

1.
The patch there is over-simplistic in that that it does not attempt to leave the partially configured channels in a "clean state". What should be such a "clean state"? Destroy channels? What about spans? (zap_restart actually needs exactly the same "destroy all of zaptel" operation).                                                                  

2.                                              
In 1.4 and trunk when there is an error while generating channels from users.conf, the error from process_zap is ignored.

3.
'zap destroy channel' and 'zap restart' don't seem to lock the mutex iflock.
By: jmls (jmls) 2007-09-12 16:31:26

now that it was reopened, is anything happening ?
By: Russell Bryant (russell) 2007-10-11 13:11:27

I went to commit this, but you have have some spaces vs. tabs issues in the patch
By: Tzafrir Cohen (tzafrir) 2007-10-12 07:19:48

Should we fail if there's no zapata.conf? What if the user wants to configure everything in users.conf?

Strange things happen, though, if we have zapata.conf but it has no [channels] section.

We also currently ignore errors from process_zap on users.conf sections (which may create channels)
By: Tzafrir Cohen (tzafrir) 2007-10-12 07:25:32

Added a reference to ASTERISK-9226, as when I updated it recently I noticed all osrts of issues regarding error handling. Those issues exist with the current code: ASTERISK-9226 simply uses the code for parsing users.conf to parse sections in zapata.conf .
By: Tilghman Lesher (tilghman) 2007-12-06 18:20:05.000-0600

This appears to already be fixed, by returning AST_MODULE_LOAD_DECLINE.  This is true for both 1.4 and trunk.