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

[Oleg Vasilev]

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 
---
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 00..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
+ * .
+ */
+
+#include 
+
+#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.


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


Thanks,
Oleg




+
+    clear_path = realpath(path, NULL);
+    if (!clear_path) {
+    VIR_WARN("Failed to resolve path %s: %s", path, 
g_strerror(errno));

+    return NULL;
+    }
+
+    if (!g_regex_match(log_regex, path, 0, ))
+    return NULL;
+
+    chain_prefix = g_match_info_fetch(matchInfo, 1);
+    if (!rotated_index)
+    return chain_prefix;
+
+    *rotated_index = 0;
+    rotated_index_str = g_match_info_fetch(matchInfo, 3);
+
+    if (!rotated_index_str)
+    return chain_prefix;
+
+    if (virStrToLong_i(rotated_index_str, NULL, 10, rotated_index) < 
0) {

+    virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Failed to parse rotated index from '%s'"),
+   rotated_index_str);
+    return NULL;
+    }
+    return chain_prefix;
+}
+
+static void
+virLogCleanerProcessFile(virLogCleanerData *data,
+ const char *path,
+ struct stat *sb)
+{
+ 

Re: [RFC PATCH] docs/about/deprecated: Deprecate 32-bit host systems

[Mark Cave-Ayland]

On 30/01/2023 20:45, Alex Bennée wrote:


Daniel P. Berrangé  writes:


On Mon, Jan 30, 2023 at 11:47:02AM +, Peter Maydell wrote:

On Mon, 30 Jan 2023 at 11:44, Thomas Huth  wrote:


Testing 32-bit host OS support takes a lot of precious time during the QEMU
contiguous integration tests, and considering that many OS vendors stopped
shipping 32-bit variants of their OS distributions and most hardware from
the past >10 years is capable of 64-bit


True for x86, not necessarily true for other architectures.
Are you proposing to deprecate x86 32-bit, or all 32-bit?
I'm not entirely sure about whether we're yet at a point where
I'd want to deprecate-and-drop 32-bit arm host support.


Do we have a feeling on which aspects of 32-bit cause us the support
burden ? The boring stuff like compiler errors from mismatched integer
sizes is mostly quick & easy to detect simply through a cross compile.

I vaguely recall someone mentioned problems with atomic ops in the past,
or was it 128-bit ints, caused implications for the codebase ?


Atomic operations on > TARGET_BIT_SIZE and cputlb when
TCG_OVERSIZED_GUEST is set. Also the core TCG code and a bunch of the
backends have TARGET_LONG_BITS > TCG_TARGET_REG_BITS ifdefs peppered
throughout.


I am one of an admittedly small group of people still interested in using KVM-PR on 
ppc32 to boot MacOS, although there is some interest on using 64-bit KVM-PR to run 
super-fast MacOS on modern Talos hardware.


From my perspective losing the ability to run 64-bit guests on 32-bit hardware with 
TCG wouldn't be an issue, as long as it were still possible to use qemu-system-ppc on 
32-bit hardware using both TCG and KVM to help debug the remaining issues.



ATB,

Mark.