On Thu, Apr 09, 2026 at 01:02:06 -0400, Laine Stump wrote:
On 4/8/26 3:59 AM, Peter Krempa via Devel wrote:
On Wed, Apr 08, 2026 at 00:10:38 -0400, Laine Stump via Devel wrote:
From: Laine Stump <laine@redhat.com>
When running in session/unprivileged mode, nearly all paths are prefixed with the returns from one of glib's g_get_user_*_dir() functions, which in turn base their selected paths on the settings of a few items in the user's environment ($XDG_*, or a subdirectory of $HOME if the relevant $XDG_* isn't set).
This patch logs the settings of these directories in the log banner in an attempt to help diagnose the problem when a file/socket open/create fails.
An example of the banner:
libvirt version: 12.3.0, package: 1.fc43 (Unknown, 2026-04-07-22:43:30, vhost) hostname: 83be0e173e02, user: qemu, uid: 107 home dir: '/' (HOME='/') runtime dir: '/.cache' (XDG_RUNTIME_DIR='(unset)') config dir: '/.config' (XDG_CONFIG_HOME='(unset)') log dir: '/.cache' (XDG_CACHE_HOME='(unset)') libvirt: XML-RPC error : Cannot create user runtime directory '/.cache/libvirt': Permission denied
Resolves: https://redhat.atlassian.net/browse/RHEL-70222 Resolves: https://redhat.atlassian.net/browse/RHEL-105490 Signed-off-by: Laine Stump <laine@redhat.com> ---
We could obviously add more information here (or less); it's difficult to know where to draw the line. Also, the astute reviewer will notice that all this code is executed once for each log target - we could do it all once at a higher level and cache it if we really wanted to. I'm not sure if it's worth the trouble though).
Changes in V2: modified the format/labeling of the data as Peter suggested, and included an example in the commit log.
src/util/virlog.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+)
diff --git a/src/util/virlog.c b/src/util/virlog.c index ccdf66c396..d2882d16ee 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -540,6 +540,43 @@ virLogToOneTarget(virLogSource *source, g_get_host_name(), username, uid); virLogOneInitMsg(timestamp, hoststr, outputFunc, data); + /* This info is only relevant when running as something other than root */ + if (uid != 0) { + g_autofree char *envHOME = NULL; + g_autofree char *envXDG_RUNTIME_DIR = NULL; + g_autofree char *envXDG_CONFIG_HOME = NULL; + g_autofree char *envXDG_CACHE_HOME = NULL; + + g_autofree char *envstr1 = NULL; + g_autofree char *envstr2 = NULL; + g_autofree char *envstr3 = NULL; + g_autofree char *envstr4 = NULL; + + if (!(envHOME = g_strdup(g_getenv("HOME")))) + envHOME = g_strdup("(unset)"); + if (!(envXDG_RUNTIME_DIR = g_strdup(g_getenv("XDG_RUNTIME_DIR")))) + envXDG_RUNTIME_DIR = g_strdup("(unset)"); + if (!(envXDG_CONFIG_HOME = g_strdup(g_getenv("XDG_CONFIG_HOME")))) + envXDG_CONFIG_HOME = g_strdup("(unset)"); + if (!(envXDG_CACHE_HOME = g_strdup(g_getenv("XDG_CACHE_HOME")))) + envXDG_CACHE_HOME = g_strdup("(unset)"); + + envstr1 = g_strdup_printf("home dir: '%s' (HOME='%s')", + g_get_home_dir(), envHOME);
I wonder if the '(unset)' string is even required. The code could be simplified, if we decide that e.g. empty string is okay to denote we won't need the extra variables:
envstr1 = g_strdup_printf("home dir: '%s' (HOME='%s')", g_get_home_dir(), NULLSTR_EMPTY(g_getenv("HOME")));
I'd never noticed NULLSTR_EMPTY() before - that could be useful! In this case though, I think I need to keep the strdups anyway, so may as well leave in the "(unset)" to differentiate from "". The reason I think the environment needs to be strdup'ed is that the result of g_getenv() is a pointer to a cached string owned by glib that is only guaranteed to remaina unchanged until g_getenv() (or g_setenv()) is called again. Since the
I've seen that in the docs ...
g_get_*_dir() functions all examine multiple environment variables, we should assume they might call g_getenv(), which could invalidate the
... but my review-saturated brain didn't get to the conclusion that this can have the same side-effect by calling g_setenv internally.
g_getenv() return value that had just been put on the stack for the call to g_strdup_printf() that's building the log string. (I can't ever remember which order the args are evaluated in, but figure it's best to assume that it could be "either", and that way I'll always be safe :-))
Yeah, evaluation order is UB AFAIR
(if it wasn't for that, I would probably follow your suggestion and just use NULLSTR_EMPTY())
+ virLogOneInitMsg(timestamp, envstr1, outputFunc, data); + + envstr2 = g_strdup_printf("runtime dir: '%s' (XDG_RUNTIME_DIR='%s')", + g_get_user_runtime_dir(), envXDG_RUNTIME_DIR); + virLogOneInitMsg(timestamp, envstr2, outputFunc, data); + + envstr3 = g_strdup_printf("config dir: '%s' (XDG_CONFIG_HOME='%s')", + g_get_user_config_dir(), envXDG_CONFIG_HOME); + virLogOneInitMsg(timestamp, envstr3, outputFunc, data); + + envstr4 = g_strdup_printf("log dir: '%s' (XDG_CACHE_HOME='%s')", + g_get_user_cache_dir(), envXDG_CACHE_HOME); + virLogOneInitMsg(timestamp, envstr4, outputFunc, data); + } *needInit = false; }
However you decide about the '(unset)' vs '' for unset env variables:
That removes the way for you to decide :P
Reviewed-by: Peter Krempa <pkrempa@redhat.com>