On Wed, Jun 30, 2010 at 4:51 PM, Daniel-Constantin Mierla miconda@gmail.com wrote:

Hello,

On 6/30/10 10:17 PM, JR Richardson wrote:

...

Hi All,

I'm running kamailio 3.0.2.

hmm, checking the logs I see changes done lately that introduced some bugs: http://git.sip-router.org/cgi-bin/gitweb.cgi?p=sip-router;a=commitdiff;h=b00...

I could spot at least:  - db handle is initialed by mi child init and then checked to be null (the cause of error message printed)  - in some error cases, db handle is left initialized, so next times the command won't work

I will try to fix in the next days, meanwhile you can apply revert patch and everything should work ... the attempt by that commit is not really solving the problem it tried to fix.

Cheers, Daniel

Hi Daniel,

Well, you sure are a smart fella. I reverted the patch and reloading the address table works as expected now. For clarification I was testing with kamailio-3.0.2_src_2010-06-30_a6141a.tar.gz snapshot. Let me know when you have the updated fix and I'll do sime testing.

Thanks.

JR

Juha Heinanen writes:

reverting the patch thus most likely reintroduced this other bug. perhaps the bug that you saw has something to do with the last sentence of the commit.

jr,

did you try with db_mode=1? it works ok here.

-- juha

Hello Juha,

On 7/1/10 8:28 AM, Juha Heinanen wrote:

JR Richardson writes:

...

Well, you sure are a smart fella. I reverted the patch and reloading the address table works as expected now. For clarification I was testing with kamailio-3.0.2_src_2010-06-30_a6141a.tar.gz snapshot. Let me know when you have the updated fix and I'll do sime testing.

there was an address table reload bug that i tried to fix with the patch:

modules_k/permissions: fixed reloading of address table

• Database handle was not initialized when address table was reloaded. Now it is initialized/closed at each reload. Looks like this has never been tested before. Still don't know why mi_addr_child_init() exists and what it is supposed to do.

reverting the patch thus most likely reintroduced this other bug. perhaps the bug that you saw has something to do with the last sentence of the commit.

I guess you use this command via rpc interface, not via mi. Looking at the code, the mi init child is not done for rpc processes, resulting in some mi commands not working properly when called over rpc. The fix should be calling mi init child functions in mi_rpc module -- working on it today.

Cheers, Daniel

On 7/1/10 10:44 AM, Juha Heinanen wrote:

Daniel-Constantin Mierla writes:

...

I guess you use this command via rpc interface, not via mi.

yes, i use 'ctl mi address_reload', noticed that it didn't work and then went and fixed it.

...

Looking at the code, the mi init child is not done for rpc processes, resulting in some mi commands not working properly when called over rpc. The fix should be calling mi init child functions in mi_rpc module -- working on it today.

Juha, I just committed on master and tested over MI where works fine. Can you do some tests over rpc? If all ok then can be backported.

Cheers, Daniel

On Jul 01, 2010 at 13:38, Juha Heinanen jh@tutpro.com wrote:

Daniel-Constantin Mierla writes:

...

Juha, I just committed on master and tested over MI where works fine. Can you do some tests over rpc? If all ok then can be backported.

it would be nice if i could, but after trying to cherry-pick my msilo commit to sr_3.0, i got errors about debian-lenny files and after that my repo has been unusable:

jh@rautu:/usr/src/orig/sip-router$ eg switch master pkg/kamailio/debian-lenny/compat: needs merge pkg/kamailio/debian-lenny/control: needs merge pkg/kamailio/debian-lenny/copyright: needs merge pkg/kamailio/debian-lenny/kamailio.README.Debian: needs merge pkg/kamailio/debian-lenny/kamailio.default: needs merge pkg/kamailio/debian-lenny/kamailio.dirs: needs merge pkg/kamailio/debian-lenny/kamailio.examples: needs merge pkg/kamailio/debian-lenny/kamailio.init: needs merge pkg/kamailio/debian-lenny/kamailio.postinst: needs merge error: you need to resolve your current index first

this debian-lenny thing has been haunting me and it would be nice to get if somehow fixed. we cannot live with git repo that is broken.

The git repo is not broken, it the debian-lenny change just generates a conflict which it does not know how to solve automatically, and it must be solved by hand. I'll try to cherry-pick the debian-lenny change into sr_3.0 too, but until then you can always run:

git reset pkg/kamailio/debian-lenny

when you get this conflict during cherry pick, and then git commit -c id_of_cherry_picked_commit.

Andrei

On 7/1/10 1:18 PM, Juha Heinanen wrote:

Daniel-Constantin Mierla writes:

...

Juha, I just committed on master and tested over MI where works fine. Can you do some tests over rpc? If all ok then can be backported.

daniel,

i deleted whole sr git repo and downloaded a new one. after that i did address_reload tests. it works from command line:

# sip-proxy_ctl mi address_reload #

but not via xmlrpc:

Jul 1 14:12:29 localhost /usr/sbin/sip-proxy[11821]: INFO: Handling XMLRPC POST from<127.0.0.1> with body<<?xml version="1.0" ?><methodCall><methodName>mi</methodName><params><param><value><string>address_reload</string></value></param></params></methodCall>> Jul 1 14:12:29 localhost /usr/sbin/sip-proxy[11821]: ERROR:<core> [db.c:421]: invalid parameter value Jul 1 14:12:29 localhost /usr/sbin/sip-proxy[11821]: ERROR: permissions [address.c:84]: failed to use table

this is another issue ... the mi commands were designed to be run from special processes, created by mi transport modules (mi_fifo, mi_datagram, etc). There are special init functions called for these processes. However, with 3.0 xmlrpc module can call these functions but the module re-uses SIP tcp processes, meaning that what is called in mi init child function should be called in child init functions.

There are two options: - call all mi child init functions in mi_rpc child_init even for SIP worker process - in the previous commit I added execution only for PROC_RPC. But then each module exporting mi commands has to be reviewed to see if there is no overlapping (usually is about opening db connection, thus make sure it is not opened twice in same process) - execute per module mi init fuction specific for that module. This will ensure that mi command become available to xmlrpc module gradually but safe. It has a drawback of initializing MI specific stuff for each process even when mi_rpc is not loaded, thus such things are not needed at runtime

Both options requires check over the mi cmd modules. Right now I think option 1 is better, because it does not initialize db connections when not needed, other opinions?

Cheers, Daniel

On 7/1/10 4:39 PM, Juha Heinanen wrote:

Daniel-Constantin Mierla writes:

...

this is another issue ... the mi commands were designed to be run from special processes, created by mi transport modules (mi_fifo, mi_datagram, etc). There are special init functions called for these processes. However, with 3.0 xmlrpc module can call these functions but the module re-uses SIP tcp processes, meaning that what is called in mi init child function should be called in child init functions.

mi trusted_reload from the same module works fine using both crt and xmlrpc:

yes, checking the sources I see trusted and address operations use different db handlers. The one for trusted is initialized from child_init, while the other one is not. Probably because the trusted can work in non-cache mode and need the connection for sip worker.

# sip-proxy_ctl mi trusted_reload #

Jul 1 17:35:35 localhost /usr/sbin/sip-proxy[11824]: INFO: Handling XMLRPC POST from<127.0.0.1> with body<<?xml version="1.0" ?><methodCall><methodName>mi</methodName><params><param><value><string>trusted_reload</string></value></param></params></methodCall>>

so may the same kind of implementation could be used also for address_realod.

...

• call all mi child init functions in mi_rpc child_init even for SIP

worker process - in the previous commit I added execution only for PROC_RPC. But then each module exporting mi commands has to be reviewed to see if there is no overlapping (usually is about opening db connection, thus make sure it is not opened twice in same process)

• execute per module mi init fuction specific for that module. This will

ensure that mi command become available to xmlrpc module gradually but safe. It has a drawback of initializing MI specific stuff for each process even when mi_rpc is not loaded, thus such things are not needed at runtime

Both options requires check over the mi cmd modules. Right now I think option 1 is better, because it does not initialize db connections when not needed, other opinions?

another opinion is: replace permissions module mi commands with rpc ones.

it is an option, somehow in the global to-do list to replace mi with rpc. Right now, this issue is not only for permissions module, is also affecting other mi cmds which are going to be used over xmlrpc module -- using mi commands via rpc interface with sercmd is different and there is no issue with that.

Cheers, Daniel

Daniel-Constantin Mierla writes:

No time to do real tests, if you can run some and report back, it would be great. Once some testing is done, can be ported to 3.0.

daniel,

i tested address_reload, trusted_reload, and sht_reload mi commands over xmlrpc and they all worked fine. also 'ctl mi address_reload' worked ok.

-- juha