[RFC PATCH 0/2] logging: add service to cleanup internal log files
Hi, all. This is a RFC for cleanup service suggested in [1]. I argumented there that logrotate is not suitable in current form to cooperate with virtlogd and only peform cleanup function. There I thought we need to have file locking for cleanup service to cooperate nicely with virtlogd. In this patch series I used timestamps for that purpuse which seems to be more simple/reliable technique. The idea is to keep logs for active VMs fresh by touching them periodically for the case when log is not actually written for a long time. This is an RFC and so it misses a lot of pieces: - we only need to touch internal log files (and not log files of serial devices for example) - openrc support is missing - spec bits are missing I guess service itself need to be written in C. For a couple of reasons: - not sure if we have a policy not to use shell scripts in deployments (we still have libvirt-guests.sh) - cleaner time complexity in this version is O(N^2). It is not a big deal to write a O(N) version but looks like it will be ugly looking in shell script - I'd like to make at least max age to be configurable parameter in virtlogd.conf. And we have conf parser in C already. As a side note I'm not sure if we should make cleanup invocation period and touching period to be configurable (both are 1 day currently) There is also a need for drivers to add their cleanup paths to cleanup service configuration. I guess we can - add "cleanup_paths = []" to virtlogd.conf - drivers can add conf files to virtlogd.d/. For example qemu drivers will have in virtlogd.d/qemu.conf: cleanup_paths += "/var/log/libvirt/qemu/" On this way we need to support configure directories and += syntax for lists. [1] Re: removing VMs logs https://listman.redhat.com/archives/libvir-list/2022-February/msg00425.html Nikolay Shirokovskiy (2): logging: touch opened files periodically logging: add virtlogcleaner service src/logging/log_handler.c | 113 ++++++++++++++++++++++---- src/logging/meson.build | 15 ++++ src/logging/virtlogcleaner.service.in | 7 ++ src/logging/virtlogcleaner.sh | 9 ++ src/logging/virtlogcleaner.timer | 8 ++ src/logging/virtlogd.service.in | 1 + 6 files changed, 138 insertions(+), 15 deletions(-) create mode 100644 src/logging/virtlogcleaner.service.in create mode 100755 src/logging/virtlogcleaner.sh create mode 100644 src/logging/virtlogcleaner.timer -- 2.31.1
This will protect log files from being deleted by virtlogcleaner even if log is not being written active currently. --- src/logging/log_handler.c | 113 +++++++++++++++++++++++++++++++++----- 1 file changed, 98 insertions(+), 15 deletions(-) diff --git a/src/logging/log_handler.c b/src/logging/log_handler.c index 5c3df37415..fee4567911 100644 --- a/src/logging/log_handler.c +++ b/src/logging/log_handler.c @@ -34,6 +34,8 @@ #include <unistd.h> #include <fcntl.h> #include <poll.h> +#include <sys/types.h> +#include <utime.h> #include "configmake.h" @@ -43,6 +45,8 @@ VIR_LOG_INIT("logging.log_handler"); #define DEFAULT_MODE 0600 +#define LOG_HANDLER_TOUCH_TIMEOUT (24 * 3600 * 1000) + typedef struct _virLogHandlerLogFile virLogHandlerLogFile; struct _virLogHandlerLogFile { virRotatingFileWriter *file; @@ -65,6 +69,8 @@ struct _virLogHandler { virLogHandlerLogFile **files; size_t nfiles; + int timer; + virLogHandlerShutdownInhibitor inhibitor; void *opaque; }; @@ -102,6 +108,17 @@ virLogHandlerLogFileFree(virLogHandlerLogFile *file) } +static void +virLogHandlerCleanupTimer(virLogHandler *handler) +{ + if (handler->nfiles > 0 || handler->timer == 0) + return; + + virEventRemoveTimeout(handler->timer); + handler->timer = 0; +} + + static void virLogHandlerLogFileClose(virLogHandler *handler, virLogHandlerLogFile *file) @@ -115,6 +132,8 @@ virLogHandlerLogFileClose(virLogHandler *handler, break; } } + + virLogHandlerCleanupTimer(handler); } @@ -209,6 +228,30 @@ virLogHandlerNew(bool privileged, } +/* + * This helper aims to handle races with file deleting by log file cleaner. + * Cleaner can unlink file right after we open it for write. In this case + * let's just recreate it. + * + */ +static virRotatingFileWriter * +virLogHandlerNewWriter(const char *path, + off_t maxlen, + size_t maxbackup, + bool trunc, + mode_t mode) +{ + virRotatingFileWriter *writer; + + writer = virRotatingFileWriterNew(path, maxlen, maxbackup, trunc, mode); + if (virFileExists(path)) + return writer; + + virRotatingFileWriterFree(writer); + return virRotatingFileWriterNew(path, maxlen, maxbackup, trunc, mode); +} + + static virLogHandlerLogFile * virLogHandlerLogFilePostExecRestart(virLogHandler *handler, virJSONValue *object) @@ -253,11 +296,11 @@ virLogHandlerLogFilePostExecRestart(virLogHandler *handler, goto error; } - if ((file->file = virRotatingFileWriterNew(path, - handler->max_size, - handler->max_backups, - false, - DEFAULT_MODE)) == NULL) + if ((file->file = virLogHandlerNewWriter(path, + handler->max_size, + handler->max_backups, + false, + DEFAULT_MODE)) == NULL) goto error; if (virJSONValueObjectGetNumberInt(object, "pipefd", &file->pipefd) < 0) { @@ -280,6 +323,26 @@ virLogHandlerLogFilePostExecRestart(virLogHandler *handler, } +static void +virLogHandlerTimeout(int timer G_GNUC_UNUSED, + void *opaque) +{ + virLogHandler *handler = opaque; + size_t i; + + virObjectLock(handler); + + for (i = 0; i < handler->nfiles; i++) { + const char *path = virRotatingFileWriterGetPath(handler->files[i]->file); + + if (utime(path, NULL) < 0) + VIR_WARN("utime(%s) error: %s", path, g_strerror(errno)); + } + + virObjectUnlock(handler); +} + + virLogHandler * virLogHandlerNewPostExecRestart(virJSONValue *object, bool privileged, @@ -330,6 +393,11 @@ virLogHandlerNewPostExecRestart(virJSONValue *object, } } + if (handler->nfiles > 0 && + (handler->timer = virEventAddTimeout(LOG_HANDLER_TOUCH_TIMEOUT, + virLogHandlerTimeout, + handler, NULL) <= 0)) + goto error; return handler; @@ -349,7 +417,10 @@ virLogHandlerDispose(void *obj) handler->inhibitor(false, handler->opaque); virLogHandlerLogFileFree(handler->files[i]); } + g_free(handler->files); + handler->nfiles = 0; + virLogHandlerCleanupTimer(handler); } @@ -393,11 +464,21 @@ virLogHandlerDomainOpenLogFile(virLogHandler *handler, file->driver = g_strdup(driver); file->domname = g_strdup(domname); - if ((file->file = virRotatingFileWriterNew(path, - handler->max_size, - handler->max_backups, - trunc, - DEFAULT_MODE)) == NULL) + /* + * Touch log files every day to prevent from removing by log files + * cleaner. + */ + if (handler->nfiles == 0 && + (handler->timer = virEventAddTimeout(LOG_HANDLER_TOUCH_TIMEOUT, + virLogHandlerTimeout, + handler, NULL) <= 0)) + goto error; + + if ((file->file = virLogHandlerNewWriter(path, + handler->max_size, + handler->max_backups, + trunc, + DEFAULT_MODE)) == NULL) goto error; VIR_APPEND_ELEMENT_COPY(handler->files, handler->nfiles, file); @@ -418,6 +499,8 @@ virLogHandlerDomainOpenLogFile(virLogHandler *handler, return pipefd[1]; error: + virLogHandlerCleanupTimer(handler); + VIR_FORCE_CLOSE(pipefd[0]); VIR_FORCE_CLOSE(pipefd[1]); handler->inhibitor(false, handler->opaque); @@ -579,11 +662,11 @@ virLogHandlerDomainAppendLogFile(virLogHandler *handler, } if (!writer) { - if (!(newwriter = virRotatingFileWriterNew(path, - handler->max_size, - handler->max_backups, - false, - DEFAULT_MODE))) + if (!(newwriter = virLogHandlerNewWriter(path, + handler->max_size, + handler->max_backups, + false, + DEFAULT_MODE))) goto cleanup; writer = newwriter; -- 2.31.1
It will clean log files of qemu processes that are outdated. Here it means that VM where not active for given amount time (currently hardcoded to a month). --- src/logging/meson.build | 15 +++++++++++++++ src/logging/virtlogcleaner.service.in | 7 +++++++ src/logging/virtlogcleaner.sh | 9 +++++++++ src/logging/virtlogcleaner.timer | 8 ++++++++ src/logging/virtlogd.service.in | 1 + 5 files changed, 40 insertions(+) create mode 100644 src/logging/virtlogcleaner.service.in create mode 100755 src/logging/virtlogcleaner.sh create mode 100644 src/logging/virtlogcleaner.timer diff --git a/src/logging/meson.build b/src/logging/meson.build index 7066f16fad..d23f51b9fd 100644 --- a/src/logging/meson.build +++ b/src/logging/meson.build @@ -101,6 +101,21 @@ if conf.has('WITH_LIBVIRTD') 'name': 'virtlogd', 'in_file': files('virtlogd.init.in'), } + + if init_script == 'systemd' + systemd_unit_dir = prefix / 'lib' / 'systemd' / 'system' + + configure_file( + input: 'virtlogcleaner.service.in', + output: 'virtlogcleaner.service', + configuration: configuration_data({'sbindir': sbindir}), + install: true, + install_dir: systemd_unit_dir, + ) + + install_data('virtlogcleaner.timer', install_dir: systemd_unit_dir) + install_data('virtlogcleaner.sh', install_dir: sbindir) + endif endif log_inc_dir = include_directories('.') diff --git a/src/logging/virtlogcleaner.service.in b/src/logging/virtlogcleaner.service.in new file mode 100644 index 0000000000..1d1ff2b121 --- /dev/null +++ b/src/logging/virtlogcleaner.service.in @@ -0,0 +1,7 @@ +[Unit] +Description=Virtual machine log cleaner +Documentation=https://libvirt.org + +[Service] +Type=oneshot +ExecStart=@sbindir@/virtlogcleaner.sh diff --git a/src/logging/virtlogcleaner.sh b/src/logging/virtlogcleaner.sh new file mode 100755 index 0000000000..21ddefb15a --- /dev/null +++ b/src/logging/virtlogcleaner.sh @@ -0,0 +1,9 @@ +#!/bin/sh + +logdir=/var/log/libvirt/qemu + +cd "$logdir" +find . -mtime +30 -name "*.log" | while read file +do + find . -regex "$file.[0-9]*" -delete && rm "$file" +done diff --git a/src/logging/virtlogcleaner.timer b/src/logging/virtlogcleaner.timer new file mode 100644 index 0000000000..a916f05f87 --- /dev/null +++ b/src/logging/virtlogcleaner.timer @@ -0,0 +1,8 @@ +[Unit] +Description=Periodic cleanup of virtual machine logs +Documentation=https://libvirt.org + +[Timer] +OnCalendar=daily +AccuracySec=1h +Persistent=true diff --git a/src/logging/virtlogd.service.in b/src/logging/virtlogd.service.in index 8ab5478517..e1883b73be 100644 --- a/src/logging/virtlogd.service.in +++ b/src/logging/virtlogd.service.in @@ -2,6 +2,7 @@ Description=Virtual machine log manager Requires=virtlogd.socket Requires=virtlogd-admin.socket +Requires=virtlogcleaner.timer Before=libvirtd.service Documentation=man:virtlogd(8) Documentation=https://libvirt.org -- 2.31.1
participants (1)
-
Nikolay Shirokovskiy