Re: [pve-devel] [PATCH pve-cluster] Change log statements to debug

[Matthias Heiserer <m.heiserer@proxmox.com>]

On 28.05.2022 09:22, Thomas Lamprecht wrote:
> On 27/05/2022 11:37, Matthias Heiserer wrote:
>> They have been commented with //fixme for more than 11 years
>> and contain little information, so at least make them debug logs.
> 
> not really that of a good reason? Was there some actual event from a
> user report or similar to trigger this? As otherwise one could argue
> that they didn't really bother anyone in 11 years, so not much gained
> in removing them. If there where actual some complaints about noise,
> I'd at least also drop the fixme comment.
> 

Someone asked in the forum [0] about the meaning of these log entries.
I figured that changing it to a debug statement and leaving the comment 
allows for completely removing the statement later, while keeping it for 
now in case someone needed it.
But ofc you're right, there's not really a need to remove it.

If we keep the log statement as-is, because there is nothing to be 
fixed, I think we can remove the fixme comments altogether, would you agree?


[0] 
https://forum.proxmox.com/threads/was-bedeuten-die-eintr%C3%A4ge-pmxcfs-2817-status-notice-received-log.110026/

>>
>> Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
>> ---
>>   data/src/logger.c | 2 +-
>>   data/src/status.c | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/data/src/logger.c b/data/src/logger.c
>> index 619e1f6..c4fcdaa 100644
>> --- a/data/src/logger.c
>> +++ b/data/src/logger.c
>> @@ -626,7 +626,7 @@ clusterlog_insert(
>>   	if (dedup_lookup(cl->dedup, entry)) {
>>   		clog_copy(cl->base, entry);
>>   	} else {
>> -		cfs_message("ignore duplicate"); // fixme remove
>> +		cfs_debug("ignore duplicate"); // fixme remove
>>   	}
>>   
>>   	g_mutex_unlock(&cl->mutex);
>> diff --git a/data/src/status.c b/data/src/status.c
>> index 9bceaeb..5e39109 100644
>> --- a/data/src/status.c
>> +++ b/data/src/status.c
>> @@ -1668,7 +1668,7 @@ dfsm_deliver(
>>   			cfs_critical("cant parse update message");
>>   		}
>>   	} else if (msg_type == KVSTORE_MESSAGE_LOG) {
>> -		cfs_message("received log"); // fixme: remove
>> +		cfs_debug("received log"); // fixme: remove
>>   		const clog_entry_t *entry;
>>   		if ((entry = kvstore_parse_log_message(msg, msg_len))) {
>>   			clusterlog_insert(cfs_status.clusterlog, entry);
>