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

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

[Matthias Heiserer]

They have been commented with //fixme for more than 11 years
and contain little information, so at least make them debug logs.

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);
-- 
2.30.2

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

[Thomas Lamprecht]

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.

> 
> 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);

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

[Matthias Heiserer]

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);
>