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 <[email protected]>
> > >
> > > 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 <[email protected]>
> > > ---
> > >
> > > 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 <[email protected]>
> >
>
