[PATCH v3 4/5] logging: add log cleanup for obsolete domains

[Daniel P. Berrangé]

On Mon, Feb 06, 2023 at 01:26:09PM +0600, Oleg Vasilev wrote:
> 
> 
> On 03.02.2023 19:45, Martin Kletzander wrote:
> > On Mon, Jan 30, 2023 at 09:00:01PM +0600, Oleg Vasilev wrote:
> > > Before, logs from deleted machines have been piling up, since there were
> > > no garbage collection mechanism. Now, virtlogd can be configured to
> > > periodically scan the log folder for orphan logs with no recent
> > > modifications
> > > and delete it.
> > > 
> > > A single chain of recent and rotated logs is deleted in a single
> > > transaction.
> > > 
> > > Signed-off-by: Oleg Vasilev <oleg.vasilev at virtuozzo.com>
> > > ---
> > > po/POTFILES               |   1 +
> > > src/logging/log_cleaner.c | 268 ++++++++++++++++++++++++++++++++++++++
> > > src/logging/log_cleaner.h |  29 +++++
> > > src/logging/log_handler.h |   2 +
> > > src/logging/meson.build   |   1 +
> > > 5 files changed, 301 insertions(+)
> > > create mode 100644 src/logging/log_cleaner.c
> > > create mode 100644 src/logging/log_cleaner.h
> > > 
> > > diff --git a/po/POTFILES b/po/POTFILES
> > > index 169e2a41dc..2fb6d18e27 100644
> > > --- a/po/POTFILES
> > > +++ b/po/POTFILES
> > > @@ -123,6 +123,7 @@ src/locking/lock_driver_lockd.c
> > > src/locking/lock_driver_sanlock.c
> > > src/locking/lock_manager.c
> > > src/locking/sanlock_helper.c
> > > +src/logging/log_cleaner.c
> > > src/logging/log_daemon.c
> > > src/logging/log_daemon_dispatch.c
> > > src/logging/log_handler.c
> > > diff --git a/src/logging/log_cleaner.c b/src/logging/log_cleaner.c
> > > new file mode 100644
> > > index 0000000000..3de54d0795
> > > --- /dev/null
> > > +++ b/src/logging/log_cleaner.c
> > > @@ -0,0 +1,268 @@
> > > +/*
> > > + * log_cleaner.c: cleans obsolete log files
> > > + *
> > > + * Copyright (C) 2022 Virtuozzo International GmbH
> > > + *
> > > + * This library is free software; you can redistribute it and/or
> > > + * modify it under the terms of the GNU Lesser General Public
> > > + * License as published by the Free Software Foundation; either
> > > + * version 2.1 of the License, or (at your option) any later version.
> > > + *
> > > + * This library is distributed in the hope that it will be useful,
> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > > + * Lesser General Public License for more details.
> > > + *
> > > + * You should have received a copy of the GNU Lesser General Public
> > > + * License along with this library;  If not, see
> > > + * <http://www.gnu.org/licenses/>.
> > > + */
> > > +
> > > +#include <config.h>
> > > +
> > > +#include "log_cleaner.h"
> > > +#include "log_handler.h"
> > > +
> > > +#include "virerror.h"
> > > +#include "virobject.h"
> > > +#include "virfile.h"
> > > +#include "viralloc.h"
> > > +#include "virlog.h"
> > > +#include "virrotatingfile.h"
> > > +#include "virstring.h"
> > > +
> > > +#define VIR_FROM_THIS VIR_FROM_LOGGING
> > > +
> > > +VIR_LOG_INIT("logging.log_cleaner");
> > > +
> > > +/* Cleanup log root (/var/log/libvirt) and all subfolders (e.g.
> > > /var/log/libvirt/qemu) */
> > > +#define CLEANER_LOG_DEPTH 1
> > > +#define CLEANER_LOG_TIMEOUT_MS (24 * 3600 * 1000) /* One day */
> > > +#define MAX_TIME ((time_t) G_MAXINT64)
> > > +
> > > +static GRegex *log_regex;
> > > +
> > > +typedef struct _virLogCleanerChain virLogCleanerChain;
> > > +struct _virLogCleanerChain {
> > > +    int rotated_max_index;
> > > +    time_t last_modified;
> > > +};
> > > +
> > > +typedef struct _virLogCleanerData virLogCleanerData;
> > > +struct _virLogCleanerData {
> > > +    virLogHandler *handler;
> > > +    time_t oldest_to_keep;
> > > +    GHashTable *chains;
> > > +};
> > > +
> > > +static const char*
> > 
> > This does not return a const char *, just char *, also the space is off.
> > 
> > > +virLogCleanerParseFilename(const char *path,
> > > +                           int *rotated_index)
> > > +{
> > > +    g_autoptr(GMatchInfo) matchInfo = NULL;
> > > +    g_autofree const char *rotated_index_str = NULL;
> > > +    g_autofree const char *clear_path = NULL;
> > > +    const char *chain_prefix = NULL;
> > 
> > None of these is const.
> 
> Just to educate myself, why are these not const? These are only set and not
> changed.

These strings are free()'d and that method doesn't accept 'const'.
It happens to be ok when using g_autofree because that discards
the const. It is surprising to reviewers since a 'const' declaration
is typically taken to indicate the variabled does not need free'ing.


With regards,
Daniel
-- 
|: 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 :|