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

[Martin Kletzander]

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.
>

They are allocated and free'd.

>There is of course the issue with type erasure, which requires the cast,
>but that I consider the limitation of GHashTable API. Or should I never
>attempt to put a const value into a type-erased void*?
>
>Also, I see a number of tasks failed in a pipeline because of missing
>unlink definition. Probably I forgot #include <unistd.h>. Should have I
>tested the patch somehow else before submitting, apart from running test
>on the machine I had at hand, e.g., have my own GitLab pipeline setup?
>

It's definitely beneficial to test this.  There are various ways, we
have documented here:

https://libvirt.org/testing.html

I pushed it upstream.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20230206/4218314a/attachment.sig>