[Home]

Summary:ASTERISK-03786: [patch] iax2 reload, fix counters for iax2 show peers
Reporter:Russell Bryant (russell)Labels:
Date Opened:2005-03-27 19:47:43.000-0600Date Closed:2008-01-15 15:29:47.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) iaxreload.txt
Description:This adds the cli command, "iax2 reload".

Also, if you used iax2 show peers with a regex, the counter would be inaccurate since it was incremented before checking the regex.

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

disclaimer is on file

russelb@clemson.edu
Comments:By: Mark Spencer (markster) 2005-03-27 21:14:17.000-0600

No need to add "iax reload" as you can already do "reload chan_iax2.so"....  Right?
By: Russell Bryant (russell) 2005-03-28 00:12:46.000-0600

Yeah, you can ... I personally think "reload chan_iax2.so" sounds like the equivalent of a "unload chan_iax2.so" and then a "load chan_iax2.so".

I know it's more code to have it this way, but it's a little more clear as to what it does.

I guess renaming it to something like "configreload chan_sip.so" would be more clear.
By: Mark Spencer (markster) 2005-03-28 01:16:36.000-0600

well, we *could* allow reload to simply intepret the second argument as either a ".so" file or the part between the first _ and the last ".so", e.g.:

reload iax2 ; would match chan_iax2.so
reload dundi ; would match pbx_dundi.so
reload sip ; would match chan_sip.so

and so on...
By: Clod Patry (junky) 2005-03-28 02:38:04.000-0600

just for reloading config files, why not just reload sip.conf ?
where .conf means reload the config file, and .so means reload the full module?
But reloading the module reload the config too.

what you think about the idea behind .so means the module and .conf means the config file?
By: Kevin P. Fleming (kpfleming) 2005-03-28 12:17:16.000-0600

That is no better than 'sip reload', it's still a command that has to be implemented by chan_sip.

I agree with drumkilla, I do not like the "reload chan_iax2.so" method at all, it just seems to imply much more is happening than just a config reload.
By: Russell Bryant (russell) 2005-03-28 16:33:06.000-0600

I kind of like mark's idea of being able to just do a "reload <module>" where the module is the part in between the '_' and '.so', but unfortunately, that wouldn't be consistent with the rest of the module commands.

Advantage:
You don't have to register a reload CLI command in every module.

Disadvantage:
The reload command isn't with the rest of the commands for the module.  All of the iax2 commands are "iax2 <command>", while reload would be "reload iax2".


Another idea:
What if we detected if a module had a reload function when it is loaded, and if so, we register the "<module> reload" CLI command automatically?
By: Kevin P. Fleming (kpfleming) 2005-03-28 16:39:52.000-0600

That's an interesting thought :-)

Here's another concern: should we do something now that means we cannot use that command to actually implement a full module reload (unload and reload) in a single operation, if we have a need to do that in the future? Most users will definitely think that 'reload chan_iax2.so' is doing exactly that (I certainly did when I started using Asterisk).

If the 'reload' function is only going to reconfigure the module, maybe it shouldn't even be called 'reload'?
By: Russell Bryant (russell) 2005-03-28 17:59:05.000-0600

Yeah, I used to think the same thing about the "reload <module>" command.  That's why I was thinking that at least renaming it to something along the lines of "configreload chan_iax2.so" would be a little more clear.

However, that still leaves the inconsistency between all of the commands for the same module.

I think my suggestion in my previous bugnote would be a nice compromise to get the best of all worlds.  We don't need to register the command inside of each module, and the reload command will be consistent with all commands for that module.  Any module with a reload function would get a CLI command automagically!  :)

I could probably work on this this weekend while I'm out of town if we come to some kind of agreement on how to proceed.

edited on: 03-28-05 18:17

edited on: 03-28-05 18:41
By: Kevin P. Fleming (kpfleming) 2005-03-28 20:02:33.000-0600

Well, I'd like to see something like 'iax2 reload config', if you want to have that command automatically registered based on the presence of a 'config_reload' function in the module. For now an automatic alias to 'iax2 reload' would provide backwards compatibility, but later could be removed or changed to actually reload the module.
By: Russell Bryant (russell) 2005-03-28 20:23:41.000-0600

I like the idea of automatically registering "<modulename> reload config" for reloading the config.  Then, there is room for "<modulename> reload <whatever>" if  there ever was a need.

I think "reload *.so" should be changed to automatically (soft) reload the module.  If someone tries to use it expecting the old behavior, they should notice the new behavior from the output.  We can stick it in the UPGRADE.txt file to notify everyone of the change, too.
By: Mark Spencer (markster) 2005-03-30 00:57:44.000-0600

reload <foo> lends itself to command completion :)
By: Kevin P. Fleming (kpfleming) 2005-03-31 14:48:21.000-0600

But so does 'iax2 reload config', since if someone wants to mess with the IAX2 module, they are more likely to look at 'iax2 ...' commands than 'reload ...'.
By: Kevin P. Fleming (kpfleming) 2005-03-31 22:57:07.000-0600

Committed to CVS, thsnks!
By: Kevin P. Fleming (kpfleming) 2005-03-31 22:57:53.000-0600

Not for stable
By: Digium Subversion (svnbot) 2008-01-15 15:29:47.000-0600

Repository: asterisk
Revision: 5337

U   trunk/channels/chan_iax2.c

------------------------------------------------------------------------
r5337 | kpfleming | 2008-01-15 15:29:46 -0600 (Tue, 15 Jan 2008) | 2 lines

Add 'iax2 reload' CLI command and fix peer counting with regex matches (bug ASTERISK-3786)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=5337