Re: [PATCH] fix virLogCleanerParseFilename logic
Hi,
For the reproducer I have enabled the garbage collection and debugging
on virtlogd.conf file:
- grep -v ^# /etc/libvirt/virtlogd.conf
log_filters="1:logging 4:object 4:json 1:event 1:util"
log_outputs="1:syslog:virtlogd"
max_age_days = 1
log_root = "/var/log/libvirt"
- I have also patched the cleaner timer with the patch attached so
that I could debug it quickly and not have to wait a full day :-)
- Create a couple of VMs so that some logs are created.
- When you enable the above you could see on the logs the message:
"error : virLogCleanerParseFilename:89 : internal error: Failed to
parse rotated index from ''
Kind regards,
David Negreira
On Tue, May 21, 2024 at 10:42 AM Michal Prívozník <mprivozn(a)redhat.com> wrote:
On 5/19/24 17:40, David Negreira wrote:
> We should return the full filename path when we don't have a match on
> the third group of the regex.
>
> Signed-off-by: David Negreira <david.negreira(a)canonical.com>
> ---
> src/logging/log_cleaner.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/logging/log_cleaner.c b/src/logging/log_cleaner.c
> index 4ee91843aa..d8e6ce9cdd 100644
> --- a/src/logging/log_cleaner.c
> +++ b/src/logging/log_cleaner.c
> @@ -82,7 +82,7 @@ virLogCleanerParseFilename(const char *path,
> *rotated_index = 0;
> rotated_index_str = g_match_info_fetch(matchInfo, 3);
>
> - if (!rotated_index_str)
> + if (rotated_index_str)
> return chain_prefix;
>
> if (virStrToLong_i(rotated_index_str, NULL, 10, rotated_index) < 0) {
I'm not sure this is the right fix. If rotated_index_str is NOT NULL
chain_prefix is returned. Fair enough. But when it is NULL then it's
passed to virStrToLong_i() which does not seem right.
Also, do you have a minimalist reproducer?
Michal
--
David Negreira
Senior Sustaining Operations Engineer | Canonical
e: david.negreira(a)canonical.com
w: www.ubuntu.com | Support: support.canonical.com
Attachments:
- log_cleaner_timer.diff (text/x-patch — 541 bytes)