[LEDE-DEV] [PATCH 2/2] [ubox] logd: add ubus reload method

[John Crispin]

On 13/05/2016 22:40, David Lang wrote:
> On Fri, 13 May 2016, Alexandru Ardelean wrote:
> 
>> On Fri, May 13, 2016 at 8:55 PM, David Lang <david at lang.hm> wrote:
>>> On Fri, 13 May 2016, Helmut Schaa wrote:
>>>> I was thinking of a different use-case:
>>>> Increasing the buffer size on the fly comes in handy during a debug
>>>> session
>>>> where you'd like to not interrupt logging (and thus potentially lose
>>>> some
>>>> parts
>>>> of the syslog when restarting logd).
>>>>
>>>> Independent of how the implementation looks like I think the
>>>> functionality
>>>> would be quite useful.
>>>
>>>
>>> I don't think it's very valuable. If you are debugging, you really don't
>>> want to be tweaking anything in the middle of trying to capture data.
>>> you
>>> want to set everything up and let it run, then analyze the data.
>>>
>>> I don't see that changing the log size in the middle of a capture run
>>> is a
>>> good idea, let alone one that is common enough to have to introduce uci
>>> specific knowledge into the logd daemon.
>>>
>>> You are better off sending to a remote logserver anyway.
>>>
>>> David Lang
>>>
>>>
>>> _______________________________________________
>>> Lede-dev mailing list
>>> Lede-dev at lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/lede-dev
>>
>> First of all, let's agree on a few things:
>> 1) the patch " [ubox] syslog: use realloc to change log buffer size ",
>> which precedes this, is an improvement over the existing code in logd
>> ; the initial code of logd, includes a logic to dynamically increase
>> the log buffer (using malloc() + memcpy()) ; there are 2 logical
>> options regarding that code:
>>   1.1) make it work (as that patch does)
>>   1.2) remove it
>> 2) there are people that don't need this ability to dynamically
>> increase the log buffer ; we do need it, but are not blocked by not
>> having it ; it would be neat to have in upstream ubox/logd, given that
>> logd was initially written with this ability partially intended;
>>
>> I don't know if this pushback is also amplified by my snappy reply to
>> KarlP.
>> If it is, well, c'est la vie :) .  I lost an argument because of a
>> snappy come-back that upset some people. Wouldn't be the first time.
>>
>> I feel that this change [to dynamically increase the log-size] can be
>> achieved with minimal impact on code/binary size and logd behavior
>> (given point 1) ).
>> Normal operation should not be affected (or the patchset can be
>> adapted to less affect normal operation).
>> And then, if it's in, people can choose to use it or not.
>> Or, if it's too intrusive/bothersome, we can drop the patchset
>> altogether.
>>
>> I'm still unclear yet how patch submission works in LEDE, and how
>> patches are accepted/voted, or who has the final go.
>> At least this process can shed some on light on that (for us).
> 
> People don't object to the ability to resize the buffer if the code
> impact is small, but when you start saying that you want to have logd
> understand/parse UCI in the binary (as opposed to the script that starts
> the binary), the code impact is not that small any longer.
> 
> The use case isn't non-existant, but it is marginal.
> 
> David Lang


tl;dr - just noticed 100 mails in this thread

the way deamons are expected to work is that they load with a default
configuration, this is usually loaded from uci and for small daemons
passed as the cmdline. if the config changes there are 2 ways to handle this

1) restart the daemon with a new cmdline
2) use the config.change trigger notification and send a new config to
the daemon via ubus.

this patch does 3) hook uci into the daemon etc pp which is i am afraid
not an option.

the difference between 1) and 2) is 1) is less code and 2) preserves the
data in the buffer. personally i would got for 1) but if there is a
patch for 2) that is clean i'll happily merge it. i'll close the series
as "Changes requested" and wait for a resend of 1) or 2) which ever it
might be.

	John