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

[Alexandru Ardelean]

Hmm, very weird.
My settings for GMail are to send email in Plain Text Mode and I still
got email delivery failure to lede-dev (for this message).

===========================================
The error that the other server returned was:
550-Mailing lists do not accept HTML mail. See
550 http://david.woodhou.se/email.html
===========================================

---------- Forwarded message ----------
From: Alexandru Ardelean <ardeleanalex at gmail.com>
Date: Sat, May 14, 2016 at 11:05 AM
Subject: Re: [LEDE-DEV] [PATCH 2/2] [ubox] logd: add ubus reload method
To: David Lang <david at lang.hm>
Cc: Helmut Schaa <helmut.schaa at googlemail.com>,
lede-dev at lists.infradead.org, Bastian Bittorf <bittorf at bluebottle.com>


On Fri, May 13, 2016 at 11:40 PM, David Lang <david at lang.hm> 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


@Felix, so then to conclude/converge. We keep the -S cli param, and
add a "size" ubus method that can return the current log size (if no
arg specified), and if an arg is specified, then update the log size ?
Something like:
- to read: ubus call log size   ==> output: {  size: 1024 }
- to write: ubus call log size  '{  "size" : 2048  }'  ==>  output: {
size: 2048 }

This would work fine for us as well.
I'm open to other modifications/simplifications as long as we can at
least write the new log size.

@David: I did mention in an earlier message in this thread, that I am
open/flexible regarding the UCI C change; that I am fine with with
replacing it with (inherently dropping it for) a ubus arg to the
reload method.
Somehow that message seems to have been missed, or if it's an issue
with infraread & GMail, let's fix that.
We're more interested in getting the log size dynamically increased
(if possible).