9:06 a.m.
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
9:06 a.m.
New subject: [RFC PATCH 1/2] logging: touch opened files periodically
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
9:06 a.m.
New subject: [RFC PATCH 2/2] logging: add virtlogcleaner service
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(a)/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
1001
days inactive
1001
days old
2 comments
1 participants
participants (1)
-
Nikolay Shirokovskiy