Re: [PATCH] http admin commands + userguide

Hi again,

Am 08.07.2009 um 18:28 schrieb Alejandro Guerrieri:

> Alex,
>
> I've already fixed the warnings (two variables declared but not used).
>
> Regarding the example you've given, that's why I've noted that you  
> shouldn't play with the id/admin-id's.
>
> Under my understanding, restart is meant to be used when you need to  
> modify some parameters. Modifying the number of binds qualifies for  
> a remove-smsc/add-smsc.
>
> On the scenario you depict, if you remove-smsc A and then add-smsc  
> A, it'll do as expected.
>
> A possible approach would be to replace restart-smsc for:
>
> get lock
> remove-smsc
> add-smsc
> release lock
>
> Or just leave it as it is and document it better?

this is bug and bugs should not be documented their should be fixed ;)
Before your patch was applied it was not possible to provoke such  
situation now it's possible and
easy fixable. I would like to see patch to fix it or I will do this  
myself :)

Should I fix it or you?

I just already see user complains :)

>
> What do you think?
> --
> Alejandro Guerrieri
> [email protected]
>
>
>
> On 08/07/2009, at 17:40, Alexander Malysh wrote:
>
>> Hi Alex,
>>
>> sorry I have not seen this before:
>> gw/bb_smscconn.c: In function ‘smsc2_remove_smsc’:
>> gw/bb_smscconn.c:839: warning: unused variable ‘smscid’
>> gw/bb_smscconn.c: In function ‘smsc2_add_smsc’:
>> gw/bb_smscconn.c:870: warning: unused variable ‘smsc_type’
>>
>> Please fix these warnings. Could you please test compile at your  
>> devel host with
>> 	./configure --enable-warnings ...
>> then you will see these :)
>>
>> And I think that now, with config reload, smsc2_restart_smsc  
>> function don't work as expected.
>> There is example:
>>
>> group = smsc
>> smsc-id = A
>>
>> group = smsc
>> smsc-id = A
>>
>> group = smsc
>> smsc-id = A
>>
>> -> start bearerbox
>>
>> -> reconfigure
>>
>> group = smsc
>> smsc-id = A
>>
>> -> restart_smsc(A)
>>
>> first found smsc-id = A will be restarted but the second will not  
>> found in the config
>> and error will be logged, leaving the second and third instances.  
>> This is not a expected behavior.
>> I would expect that all 3 instances will be shutdown and only one  
>> will be running after restarting.
>>
>> I propose to change it in following sequence:
>> 	- get lock
>> 	- shutdown all instances with this id
>> 	- start all found instances from new config
>> 	- release lock
>>
>> Thanks,
>> Alex
>>
>> Am 08.07.2009 um 16:30 schrieb Alejandro Guerrieri:
>>
>>> Commited to CVS.
>>>
>>> Regards,
>>> --
>>> Alejandro Guerrieri
>>> [email protected]
>>>
>>>
>>>
>>> On 08/07/2009, at 15:12, Alexander Malysh wrote:
>>>
>>>> no objections from me...
>>>>
>>>> Thanks,
>>>> Alex
>>>>
>>>> Am 08.07.2009 um 14:52 schrieb Alejandro Guerrieri:
>>>>
>>>>> Here's the patch with the userguide part.
>>>>>
>>>>> If no objections, I'll commit it later.
>>>>>
>>>>> Regards,
>>>>> --
>>>>> Alejandro Guerrieri
>>>>> [email protected]
>>>>>
>>>>>
>>>>> <kannel-http-admin-ug.patch>
>>>>>
>>>>
>>>
>>
>