On Tue, Apr 07, 2026 at 03:02:11PM -0400, Laine Stump wrote:
> On 4/7/26 4:37 AM, Daniel P. Berrangé via Devel wrote:
> > On Mon, Apr 06, 2026 at 11:29:19PM -0400, Laine Stump via Devel wrote:
> > > From: Laine Stump <[email protected]>
> > > 
> > > As libvirt is used more and more in unprivileged/session mode,
> > > file/socket permission errors have become more common. This patch adds
> > > an initial line to the log banner (the first thing sent to every log
> > > target after the process starts) stating whether the process is
> > > running privileged (as root) or unprivileged/session mode, and if the
> > > latter, it also provides the username and uid the process is running
> > > as.
> > > 
> > > The idea is to expend this to include more generally useful info about the
> > > environment we're running in. (We just need to remember that in this
> > > context we can't call anything that could lead to recursively calling
> > > the logging system (i.e. you can't call any code that reports an
> > > error, or a VIR_WARN, etc))
> > > 
> > > Signed-off-by: Laine Stump <[email protected]>
> > > ---
> > >   src/util/virlog.c | 11 +++++++++++
> > >   1 file changed, 11 insertions(+)
> > > 
> > > diff --git a/src/util/virlog.c b/src/util/virlog.c
> > > index 4e95af047b..d5bd216241 100644
> > > --- a/src/util/virlog.c
> > > +++ b/src/util/virlog.c
> > > @@ -525,6 +525,7 @@ virLogToOneTarget(virLogSource *source,
> > >                     bool *needInit)
> > >   {
> > >       if (needInit) {
> > > +        uid_t uid = geteuid();
> > >           g_autofree char *hoststr = NULL;
> > >           /* put some useful info at the top of the log. Avoid calling
> > > @@ -537,6 +538,16 @@ virLogToOneTarget(virLogSource *source,
> > >           hoststr = g_strdup_printf("hostname: %s", g_get_host_name());
> > >           virLogOneInitMsg(timestamp, hoststr, outputFunc, data);
> > > +        if (uid == 0) {
> > > +            virLogOneInitMsg(timestamp, "running in privileged/system 
> > > mode", outputFunc, data);
> > > +        } else {
> > > +            g_autofree char *username = virGetUserName(uid);
> > > +            g_autofree char *privstr = NULL;
> > > +
> > > +            privstr = g_strdup_printf("running in unprivileged/session 
> > > mode, user: %s, uid: %u",
> > > +                                      username, uid);
> > > +            virLogOneInitMsg(timestamp, privstr, outputFunc, data);
> > > +        }
> > 
> > The logs are used outside daemon context, so these messages are
> > somewhat too specific. I'd suggest just logging the user/group
> > alone without the leading 'running in ... mode'
> 
> Makes sense. I've just moved the two things up onto the same line with the
> hostname. What about the environment variables in patch 6? Too much? Not
> enough?

Hard to say, but I'm pretty sure we've seen bugs with libguestfs
where knowing the XDG_* env vars would have made diagnosis quicker,
so that probably points to including them.

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|

Reply via email to