[libvirt] [PATCH v2] logging: ensure virtlogd rollover takes priority over logrotate

Daniel P. Berrangé posted 1 patch 4 years, 9 months ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20190710171425.25262-1-berrange@redhat.com
src/logging/virtlogd.conf              |  6 ++++++
src/remote/libvirtd.libxl.logrotate.in |  2 +-
src/remote/libvirtd.lxc.logrotate.in   |  2 +-
src/remote/libvirtd.qemu.logrotate.in  | 10 +++++++++-
4 files changed, 17 insertions(+), 3 deletions(-)
[libvirt] [PATCH v2] logging: ensure virtlogd rollover takes priority over logrotate
Posted by Daniel P. Berrangé 4 years, 9 months ago
The virtlogd config is set to rollover logs every 2 MB.

Normally a logrotate config file is also installed to handle cases where
virtlogd is disabled. This is set to rollover weekly with no size
constraint.

As a result logrotate can interfere with virtlogd's, rolling over files
that virtlogd has already taken care of.

This changes logrotate configs to rollover based on a max size
constraint of 2 MB + 1 byte. When virtlogd is running the log files will
never get this large, making logrotate a no-op.

If the user changes the size in virtlogd's config to something larger,
they are responsible for also changing the logrotate config suitably.

The LXC driver doesn't use virtlogd, but its logrotate config is altered
to match the QEMU driver logrotate, just for the sake of consistency.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/logging/virtlogd.conf              |  6 ++++++
 src/remote/libvirtd.libxl.logrotate.in |  2 +-
 src/remote/libvirtd.lxc.logrotate.in   |  2 +-
 src/remote/libvirtd.qemu.logrotate.in  | 10 +++++++++-
 4 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/src/logging/virtlogd.conf b/src/logging/virtlogd.conf
index 72da7f0705..bc41edbc6b 100644
--- a/src/logging/virtlogd.conf
+++ b/src/logging/virtlogd.conf
@@ -90,6 +90,12 @@
 #admin_max_clients = 5
 
 # Maximum file size before rolling over. Defaults to 2 MB
+#
+# Beware that a logrotate config file might be installed too,
+# to handle cases where virtlogd is disabled. To ensure that
+# the logrotate config is a no-op when virtlogd is running,
+# make sure that max_size here is smaller than size listed
+# in the logrotate config.
 #max_size = 2097152
 
 # Maximum number of backup files to keep. Defaults to 3,
diff --git a/src/remote/libvirtd.libxl.logrotate.in b/src/remote/libvirtd.libxl.logrotate.in
index cb7f07d846..1461c1efa1 100644
--- a/src/remote/libvirtd.libxl.logrotate.in
+++ b/src/remote/libvirtd.libxl.logrotate.in
@@ -1,5 +1,5 @@
 @localstatedir@/log/libvirt/libxl/*.log {
-        weekly
+        size 2097153
         missingok
         rotate 4
         compress
diff --git a/src/remote/libvirtd.lxc.logrotate.in b/src/remote/libvirtd.lxc.logrotate.in
index 2bb9dfba12..b88dabb58e 100644
--- a/src/remote/libvirtd.lxc.logrotate.in
+++ b/src/remote/libvirtd.lxc.logrotate.in
@@ -1,5 +1,5 @@
 @localstatedir@/log/libvirt/lxc/*.log {
-        weekly
+        size 2097153
         missingok
         rotate 4
         compress
diff --git a/src/remote/libvirtd.qemu.logrotate.in b/src/remote/libvirtd.qemu.logrotate.in
index cdb399ef23..78f2ca875e 100644
--- a/src/remote/libvirtd.qemu.logrotate.in
+++ b/src/remote/libvirtd.qemu.logrotate.in
@@ -1,5 +1,13 @@
 @localstatedir@/log/libvirt/qemu/*.log {
-        weekly
+        # The QEMU driver is configured to use virtlogd by
+        # default, which will perform log rollover.
+        # This logrotate config is still installed for cases
+        # where the user has switched off virtlogd.
+        #
+        # If virtlogd is active, ensure that size here is
+        # larger than 'max_size' in the virtlogd config
+        # so that logrotate becomes a no-op
+        size 2097153
         missingok
         rotate 4
         compress
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] logging: ensure virtlogd rollover takes priority over logrotate
Posted by Jim Fehlig 4 years, 9 months ago
On 7/10/19 11:14 AM, Daniel P. Berrangé  wrote:
> The virtlogd config is set to rollover logs every 2 MB.
> 
> Normally a logrotate config file is also installed to handle cases where
> virtlogd is disabled. This is set to rollover weekly with no size
> constraint.
> 
> As a result logrotate can interfere with virtlogd's, rolling over files
> that virtlogd has already taken care of.
> 
> This changes logrotate configs to rollover based on a max size
> constraint of 2 MB + 1 byte. When virtlogd is running the log files will
> never get this large, making logrotate a no-op.
> 
> If the user changes the size in virtlogd's config to something larger,
> they are responsible for also changing the logrotate config suitably.
> 
> The LXC driver doesn't use virtlogd, but its logrotate config is altered
> to match the QEMU driver logrotate, just for the sake of consistency.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   src/logging/virtlogd.conf              |  6 ++++++
>   src/remote/libvirtd.libxl.logrotate.in |  2 +-
>   src/remote/libvirtd.lxc.logrotate.in   |  2 +-
>   src/remote/libvirtd.qemu.logrotate.in  | 10 +++++++++-
>   4 files changed, 17 insertions(+), 3 deletions(-)

This patch looks good based on requirement of logrotate not interfering with 
virtlogd rotation, and for that

Reviewed-by: Jim Fehlig <jfehlig@suse.com>

But the problem of unpurged log files that Jan was attempting to solve remains. 
If I understood your response to to his V2 patch [0], virtlogd needs to gain 
support for purging log files for old transient VMs after some configurable time?

In the private bug (sorry) that initiated this discussion I suggested adjusting 
the logrotate config via cloud deployment tool, pushing rotation policy 
decisions off to the "admin". What is your opinion on fixing this in upstream 
libvirt vs configuration at deployment?

Regards,
Jim

[0] https://www.redhat.com/archives/libvir-list/2019-July/msg00368.html

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] logging: ensure virtlogd rollover takes priority over logrotate
Posted by Daniel P. Berrangé 4 years, 9 months ago
On Fri, Jul 12, 2019 at 02:55:25AM +0000, Jim Fehlig wrote:
> On 7/10/19 11:14 AM, Daniel P. Berrangé  wrote:
> > The virtlogd config is set to rollover logs every 2 MB.
> > 
> > Normally a logrotate config file is also installed to handle cases where
> > virtlogd is disabled. This is set to rollover weekly with no size
> > constraint.
> > 
> > As a result logrotate can interfere with virtlogd's, rolling over files
> > that virtlogd has already taken care of.
> > 
> > This changes logrotate configs to rollover based on a max size
> > constraint of 2 MB + 1 byte. When virtlogd is running the log files will
> > never get this large, making logrotate a no-op.
> > 
> > If the user changes the size in virtlogd's config to something larger,
> > they are responsible for also changing the logrotate config suitably.
> > 
> > The LXC driver doesn't use virtlogd, but its logrotate config is altered
> > to match the QEMU driver logrotate, just for the sake of consistency.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >   src/logging/virtlogd.conf              |  6 ++++++
> >   src/remote/libvirtd.libxl.logrotate.in |  2 +-
> >   src/remote/libvirtd.lxc.logrotate.in   |  2 +-
> >   src/remote/libvirtd.qemu.logrotate.in  | 10 +++++++++-
> >   4 files changed, 17 insertions(+), 3 deletions(-)
> 
> This patch looks good based on requirement of logrotate not interfering with 
> virtlogd rotation, and for that
> 
> Reviewed-by: Jim Fehlig <jfehlig@suse.com>
> 
> But the problem of unpurged log files that Jan was attempting to solve remains. 
> If I understood your response to to his V2 patch [0], virtlogd needs to gain 
> support for purging log files for old transient VMs after some configurable time?

I don't think it needs to be virtlogd really. I was suggesting a standalone
periodic task.

> In the private bug (sorry) that initiated this discussion I suggested adjusting 
> the logrotate config via cloud deployment tool, pushing rotation policy 
> decisions off to the "admin". What is your opinion on fixing this in upstream 
> libvirt vs configuration at deployment?

A local admin is of course free to change the configuration is any way
they desire.  Just that from upstream POV, having logrotate touch files
that are managed by virtlogd puts you in an unsupportable situation.

Regards,
Daniel

> [0] https://www.redhat.com/archives/libvir-list/2019-July/msg00368.html
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list