Re: [PATCH] fix virLogCleanerParseFilename logic
On 5/21/24 11:11, David Negreira wrote:
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 ''
Ah, that helped. Thanks. IMO the proper fix is something among these lines:
diff --git i/src/logging/log_cleaner.c w/src/logging/log_cleaner.c
index 4ee91843aa..9f987b02b5 100644
--- i/src/logging/log_cleaner.c
+++ w/src/logging/log_cleaner.c
@@ -82,10 +82,8 @@ virLogCleanerParseFilename(const char *path,
*rotated_index = 0;
rotated_index_str = g_match_info_fetch(matchInfo, 3);
- if (!rotated_index_str)
- return chain_prefix;
-
- if (virStrToLong_i(rotated_index_str, NULL, 10, rotated_index) < 0) {
+ if (rotated_index_str && STRNEQ(rotated_index_str, "") &&
+ virStrToLong_i(rotated_index_str, NULL, 10, rotated_index) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to parse rotated index from '%1$s'"),
rotated_index_str);
If the path lacks ".N" suffix, then the third match group is empty and
g_match_info_fetch() returns an empty string. IOW, we should be parsing
the suffix iff it's a non-empty string.
Alternatively, we might use g_match_info_get_match_count() to find out
how many groups were matched.
What I also experimented with was passing G_REGEX_MATCH_NOTEMPTY flag to
g_regex_new() but that didn't work. Idea was to make
g_match_info_fetch(,3) return NULL, but I have no idea why it didn't
work.
Can you please send v2?
Michal