[prev in list] [next in list] [prev in thread] [next in thread]
List: asterisk-dev
Subject: Re: [asterisk-dev] [Code Review] 3970: res_phoneprov: Refactor phoneprov to allow pluggable config p
From: "George Joseph" <reviewboard () asterisk ! org>
Date: 2014-09-30 20:23:33
Message-ID: 20140930202333.23528.26386 () sonic ! digium ! api
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
> On Sept. 30, 2014, 1:57 p.m., rmudgett wrote:
> > Minor nit
> >
> > I'm hesitant about this going into v12 and v13 at this late a date especially \
> > since v13 is a LTS and currently feature frozen.
I know but I think anyone who uses the current phoneprov implementation (including \
me) won't be able to migrate to pjsip without the refactor and the new module. I \
just didn't have a chance to work on it before the cutoff.
I'll fix the stray semicolon before I commit.
- George
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3970/#review13416
-----------------------------------------------------------
On Sept. 30, 2014, 1:41 p.m., George Joseph wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3970/
> -----------------------------------------------------------
>
> (Updated Sept. 30, 2014, 1:41 p.m.)
>
>
> Review request for Asterisk Developers.
>
>
> Repository: Asterisk
>
>
> Description
> -------
>
> The big piece missing for me to finally transition to pjsip was the ability to \
> mirror the auto provisioning features of res_phoneprov. The first step (this \
> patch) is to make res_phoneprov more modular so other modules (like pjsip) can \
> provide configuration information instead of res_phoneprov relying solely on \
> users.conf and sip.conf. To accomplish this a new ast_phoneprov public API is now \
> exposed which allows config providers to register themselves, set defaults (server \
> profile, etc) and add user extensions.
> ast_phoneprov_provider_register registers the provider and provides callbacks for \
> loading default settings and loading users. ast_phoneprov_provider_unregister \
> clears the defaults and users. ast_phoneprov_add_extension should be called once \
> for each user/extension by the provider's load_users callback to add them. \
> ast_phoneprov_delete_extension deletes one extension. \
> ast_phoneprov_delete_extensions deletes all extensions for the provider.
> res_phoneprov actually registers itself as the provider for sip/users and is always \
> available and is the default.
> Writing a new provider...
> Since res_phoneprov is also it's own provider, examples of what a new provider \
> would have to do are in load_users() in res_phoneprov.c. Those functions gather \
> the information from users.conf and sip.conf and call the ast_provider_register and \
> ast_phoneprov_add_extension apis.
> So...
> The provider creates a callback function which calls the \
> ast_phoneprov_add_extension api for each user. It then calls \
> ast_phoneprov_provider_register with the callback. res_phoneprov then calls the \
> callback to cause the actual load. During normal http server ops, all work is done \
> by res_phoneprov and the provider is never called again unless a reload is needed. \
> If the provider wants to reload it can simply unregister and reregister or it can \
> call its own load_users callback. If res_phoneprov wants to reload, it iterates \
> over its registry and calls the providers callback.
> NOTE: If res_phoneprov is actually unloaded, it has no way to know what providers \
> were registered (other than itself) so a subsequent load will have nothing but it's \
> own users.
> Additional changes...
> I added a few convenience functions to chanvars for creating lists and finding and \
> deleting entries. No existing code was touched.
> Next steps...
> A provider for res_pjsip.
>
>
> Diffs
> -----
>
> branches/12/res/res_phoneprov.exports.in PRE-CREATION
> branches/12/res/res_phoneprov.c 424175
> branches/12/main/chanvars.c 424175
> branches/12/include/asterisk/phoneprov.h PRE-CREATION
> branches/12/include/asterisk/chanvars.h 424175
> branches/12/configs/phoneprov.conf.sample 424175
>
> Diff: https://reviewboard.asterisk.org/r/3970/diff/
>
>
> Testing
> -------
>
> I ran through several scenarios including the use of PP_EACH_USER and \
> PP_EACH_EXTENSION to make sure that all existing functionality was preserved. I \
> actually use it with Grandstream phones and everything worked exactly as expected.
>
> Thanks,
>
> George Joseph
>
>
[Attachment #5 (text/html)]
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 \
solid;"> <tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/3970/">https://reviewboard.asterisk.org/r/3970/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <p style="margin-top: 0;">On September 30th, 2014, 1:57 p.m. MDT, \
<b>rmudgett</b> wrote:</p> <blockquote style="margin-left: 1em; border-left: 2px \
solid #d0d0d0; padding-left: 10px;"> <pre style="white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">Minor nit
I'm hesitant about this going into v12 and v13 at this late a date especially \
since v13 is a LTS and currently feature frozen.</pre> </blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I know but I think \
anyone who uses the current phoneprov implementation (including me) won't be able \
to migrate to pjsip without the refactor and the new module. I just didn't have \
a chance to work on it before the cutoff.
I'll fix the stray semicolon before I commit.
</pre>
<br />
<p>- George</p>
<br />
<p>On September 30th, 2014, 1:41 p.m. MDT, George Joseph wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('https://reviewboard.asterisk.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black \
solid;"> <tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By George Joseph.</div>
<p style="color: grey;"><i>Updated Sept. 30, 2014, 1:41 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">The big piece missing for me to finally transition to pjsip was the \
ability to mirror the auto provisioning features of res_phoneprov. The first step \
(this patch) is to make res_phoneprov more modular so other modules (like pjsip) can \
provide configuration information instead of res_phoneprov relying solely on \
users.conf and sip.conf. To accomplish this a new ast_phoneprov public API is now \
exposed which allows config providers to register themselves, set defaults (server \
profile, etc) and add user extensions.
ast_phoneprov_provider_register registers the provider and provides callbacks for \
loading default settings and loading users. ast_phoneprov_provider_unregister clears \
the defaults and users. ast_phoneprov_add_extension should be called once for each \
user/extension by the provider's load_users callback to add them. \
ast_phoneprov_delete_extension deletes one extension. ast_phoneprov_delete_extensions \
deletes all extensions for the provider.
res_phoneprov actually registers itself as the provider for sip/users and is always \
available and is the default.
Writing a new provider...
Since res_phoneprov is also it's own provider, examples of what a new provider \
would have to do are in load_users() in res_phoneprov.c. Those functions gather the \
information from users.conf and sip.conf and call the ast_provider_register and \
ast_phoneprov_add_extension apis.
So...
The provider creates a callback function which calls the ast_phoneprov_add_extension \
api for each user. It then calls ast_phoneprov_provider_register with the callback.
res_phoneprov then calls the callback to cause the actual load.
During normal http server ops, all work is done by res_phoneprov and the provider is \
never called again unless a reload is needed. If the provider wants to reload it can \
simply unregister and reregister or it can call its own load_users callback. If \
res_phoneprov wants to reload, it iterates over its registry and calls the providers \
callback.
NOTE: If res_phoneprov is actually unloaded, it has no way to know what providers \
were registered (other than itself) so a subsequent load will have nothing but \
it's own users.
Additional changes...
I added a few convenience functions to chanvars for creating lists and finding and \
deleting entries. No existing code was touched.
Next steps...
A provider for res_pjsip.
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">I ran through several scenarios including the use of PP_EACH_USER and \
PP_EACH_EXTENSION to make sure that all existing functionality was preserved. I \
actually use it with Grandstream phones and everything worked exactly as expected. \
</pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>branches/12/res/res_phoneprov.exports.in <span style="color: \
grey">(PRE-CREATION)</span></li>
<li>branches/12/res/res_phoneprov.c <span style="color: grey">(424175)</span></li>
<li>branches/12/main/chanvars.c <span style="color: grey">(424175)</span></li>
<li>branches/12/include/asterisk/phoneprov.h <span style="color: \
grey">(PRE-CREATION)</span></li>
<li>branches/12/include/asterisk/chanvars.h <span style="color: \
grey">(424175)</span></li>
<li>branches/12/configs/phoneprov.conf.sample <span style="color: \
grey">(424175)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3970/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
--
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic