[PATCH] Allow root users to have their own configuration file

[Martin Kletzander <mkletzan@xxxxxxxxxx>]

Currently, we have two configuration file paths, one global (where
"global" means root-only and we're probably not changing this in near
future) and one per-user.  Unfortunately root user cannot use the
second option because until now we were choosing the file path
depending only on whether the user is root or not.

This patch modifies the mentioned behavior for root only, allowing him
to set his own configuration files without changing anything in
system-wide configuration folders.

This also makes the virsh-uriprecedence test pass its first test case
when ran as root.

Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx>
---

Notes:
    I'm playing along previously mentioned "proper behavior" in this
    patch.  However, IMNSHO, our "global" or "system-wide" configuration
    file (defaulting to '/etc/libvirt/libvirt.conf') should be accessible
    for all users since this has no security impact (security information
    may be in files 'libvirtd.conf' or 'qemu.conf').  This file should be
    also read and used for all users.  After that, settings in user
    configuration file (defaulting to '~/.config/libvirt/libvirt.conf')
    may override some of these settings for that user.
    
    This is how all sensible configurations are loaded and that's also
    what I'd prefer.  Unfortunately some developers feels this should be
    done in completely different way.

 src/libvirt.c | 56 ++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 36 insertions(+), 20 deletions(-)

diff --git a/src/libvirt.c b/src/libvirt.c
index 20a2d4c..bfc466b 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -957,28 +957,34 @@ error:
     return -1;
 }

-static char *
-virConnectGetConfigFilePath(void)
+/*
+ * Return code 0 means no error, but doesn't guarantee path != NULL.
+ */
+static int
+virConnectGetConfigFilePath(char **path, bool global)
 {
-    char *path;
-    if (geteuid() == 0) {
-        if (virAsprintf(&path, "%s/libvirt/libvirt.conf",
+    char *userdir = NULL;
+    int ret = -1;
+    *path = NULL;
+
+    /* Don't provide the global configuration file to non-root users */
+    if (geteuid() != 0 && global)
+        return 0;
+
+    if (global) {
+        if (virAsprintf(path, "%s/libvirt/libvirt.conf",
                         SYSCONFDIR) < 0)
-            return NULL;
+            goto cleanup;
     } else {
-        char *userdir = virGetUserConfigDirectory();
-        if (!userdir)
-            return NULL;
-
-        if (virAsprintf(&path, "%s/libvirt.conf",
-                        userdir) < 0) {
-            VIR_FREE(userdir);
-            return NULL;
-        }
-        VIR_FREE(userdir);
+        if (!(userdir = virGetUserConfigDirectory()) ||
+            virAsprintf(path, "%s/libvirt.conf", userdir) < 0)
+            goto cleanup;
     }

-    return path;
+    ret = 0;
+ cleanup:
+    VIR_FREE(userdir);
+    return ret;
 }

 static int
@@ -989,12 +995,22 @@ virConnectGetConfigFile(virConfPtr *conf)

     *conf = NULL;

-    if (!(filename = virConnectGetConfigFilePath()))
+    /* Try reading user configuration file unconditionally */
+    if (virConnectGetConfigFilePath(&filename, false) < 0)
         goto cleanup;

     if (!virFileExists(filename)) {
-        ret = 0;
-        goto cleanup;
+        /* and in case there is none, try the global one. */
+
+        VIR_FREE(filename);
+        if (virConnectGetConfigFilePath(&filename, true) < 0)
+            goto cleanup;
+
+        if (!filename ||
+            !virFileExists(filename)) {
+            ret = 0;
+            goto cleanup;
+        }
     }

     VIR_DEBUG("Loading config file '%s'", filename);
-- 
1.8.3.2

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list