[Home]

Summary:ASTERISK-06686: [patch] Allow contexts in regexten so that extensions can be added to multiple contexts when peer registers
Reporter:Bradley Watkins (marquis)Labels:
Date Opened:2006-04-03 10:01:16Date Closed:2006-05-18 09:08:34
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Channels/chan_sip/Registration
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) regexten_with_contexts_04-05-2006.diff
( 1) regexten_with_contexts_04-10-2006.diff
( 2) regexten_with_contexts.diff
( 3) sip.conf.sample_regexten_with_contexts.diff
Description:Modifies build_peer and register_peer_exten to parse exten@context so that regexten can apply to more than one context.

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

build_peer parses regexten variable and creates contexts to ensure they exist when register_peer_exten creates the extension.

Possible limitation is that the regexten variable in the sip_peer struct only allocates 80 characters.  In practice, this should still allow for a reasonable number of extensions (with their contexts) to be configured.

Note that this patch maintains the existing behavior of regexten so that contexts are not required and will use the global_regcontext variable if none is specified, also allowing mixed exten/exten@context definitions separated by '&'.
Comments:By: Olle Johansson (oej) 2006-04-03 12:00:42

What about not creating in build_peer, but allowing several regcontexts in general? That way, it would be easier to keep track of them and possibly delete them at reloads, if they are not needed any more.

By: Bradley Watkins (marquis) 2006-04-03 14:08:03

Do you mean something like:

regcontext=default&internal&external&whateverelse

And then parse them out and create them within reload_config?

Would you still maintain the semantics of my patch for the regexten variable on a per-peer basis, or would you intend to somehow associate individual contexts in regcontext with entries in regexten?

That is, currently this patch is looking for:

regexten=1234@default&12345@internal&123456@whateverelse

and so forth.

Would you want to keep that and just force users to have the list of contexts they intend to have listed in regexten match regcontext, or would you want something like (for lack of a better syntax at the moment):

regexten=1234&12345&&123456

be equivalent (note the double ampersand to skip the 'external' context listed in the above regcontext) such that the list of regextens is associative to the regcontexts?

The former would make sense to me and I'm happy to change the patch to do it that way if it's preferable, but the latter seems a bit weird.

Alternatively, of course, I'm rambling on about something other than you intend... ;)

Am I off-base, or moving in the right direction?

By: Olle Johansson (oej) 2006-04-03 14:55:40

I like the domains to be in the regexten, as you had from first, but only allow domains specified in regcontext.

We need to create a list of those domains and make sure we delete domains at reload if they are no longer needed or used. It's easier to maintain that list and compare it at reload if it's only specified in one place.

By: Bradley Watkins (marquis) 2006-04-05 16:50:07

Here is an update to this patch with the contexts being created in reload_config, and a function added to destroy stale contexts when the config is reloaded.  I also added a check to see if the context exists before an attempt to create the extension and logs a warning if it is not.

By: Olle Johansson (oej) 2006-04-09 14:04:16

You have a lot of strange ast_cli messages, propably for your own debugging. If you need debug messages, these need to be ast_log(LOG_DEBUG as others...

I have just taken a brief look, will go into more detail laters. Looks promising. THanks for updating!

By: Bradley Watkins (marquis) 2006-04-09 15:40:45

Yes, the ast_cli calls were for some debugging I was doing (amazing how strange things happen when you copy a pointer that later gets set to '\0' instead of copying the string).  They are entirely superfluous, and I'll post a patch tomorrow (when I get back to my development system at work) with those removed.

By: Bradley Watkins (marquis) 2006-04-10 05:12:15

Here is the new patch, sans superfluous ast_cli calls.

By: Olle Johansson (oej) 2006-04-13 08:11:56

This seems to be the way forward. Have you tested it?

By: Bradley Watkins (marquis) 2006-04-13 10:11:05

Yes, it has been tested on a couple of different boxes in my lab, including one that is 'semi-production' (it has its own PRI and DIDs that I use on a regular basis).

I also spent some time beating it up adding and removing contexts and extensions, and it never caused any issues in my testing.

By: Bradley Watkins (marquis) 2006-05-01 08:30:44

I was just thinking about this patch again, and was wondering if you might have had any time to look at it.

In addition, I should probably note that the changes I made at your behest have made it so mixed exten and exten@context syntax doesn't work anymore (nor does it make any sense to try and make it so).  The original single-context, single- or multiple-extension syntax does still work.

At some point perhaps the old syntax could be deprecated and register_peer_exten could be restructured a bit to be cleaner.

By: Joshua C. Colp (jcolp) 2006-05-18 09:08:33

Now in trunk as of revision 28168. Hopefully this will get some more people using/testing it! Thankies!