[libvirt] [PATCH 4/6] storage: fs: Don't try to chown directory unless user requested

[Cole Robinson]

Currently we try to chown any directory passed to virDirCreate,
even if the user didn't request any explicit owner/group via the
pool/vol XML.

This causes issues with qemu:///session: try to build a pool of
a root owned directory like /tmp, and it fails trying to chown the
directory to the session user. Instead it should just leave things
as they are, unless the user requests changing permissions via
the pool XML.

Similarly this is annoying if creating a storage pool via system
libvirtd of an existing directory in user $HOME, it's now owned
by root.

The virDirCreate function is pretty convoluted, since it needs to
fork off in certain specific cases. Try to document that, to make
it clear where exactly we are changing behavior.
---
 src/util/virfile.c | 32 +++++++++++++++++++++++---------
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/src/util/virfile.c b/src/util/virfile.c
index 23a1655..676e7b4 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -2303,7 +2303,8 @@ virDirCreateNoFork(const char *path,
         virReportSystemError(errno, _("stat of '%s' failed"), path);
         goto error;
     }
-    if (((st.st_uid != uid) || (st.st_gid != gid))
+    if (((uid != (uid_t) -1 && st.st_uid != uid) ||
+         (gid != (gid_t) -1 && st.st_gid != gid))
         && (chown(path, uid, gid) < 0)) {
         ret = -errno;
         virReportSystemError(errno, _("cannot chown '%s' to (%u, %u)"),
@@ -2335,19 +2336,32 @@ virDirCreate(const char *path,
     gid_t *groups;
     int ngroups;
 
-    /* allow using -1 to mean "current value" */
-    if (uid == (uid_t) -1)
-        uid = geteuid();
-    if (gid == (gid_t) -1)
-        gid = getegid();
-
+    /* Everything after this check is crazyness to allow setting uid/gid
+     * on directories that are on root-squash NFS shares. We only want
+     * to go that route if the follow conditions are true:
+     *
+     * 1) VIR_DIR_CREATE_AS_UID was passed, currently only used when
+     *    directory is being created for a NETFS pool
+     * 2) We are running as root, since that's when the root-squash
+     *    workaround is required.
+     * 3) An explicit uid/gid was requested
+     * 4) The directory doesn't already exist and the ALLOW_EXIST flag
+     *    wasn't passed.
+     *
+     * If any of those conditions are _not_ met, ignore the fork crazyness
+     */
     if ((!(flags & VIR_DIR_CREATE_AS_UID))
         || (geteuid() != 0)
-        || ((uid == 0) && (gid == 0))
-        || ((flags & VIR_DIR_CREATE_ALLOW_EXIST) && (stat(path, &st) >= 0))) {
+        || ((uid == (uid_t) -1) && (gid == (gid_t) -1))
+        || ((flags & VIR_DIR_CREATE_ALLOW_EXIST) && virFileExists(path))) {
         return virDirCreateNoFork(path, mode, uid, gid, flags);
     }
 
+    if (uid == (uid_t) -1)
+        uid = geteuid();
+    if (gid == (gid_t) -1)
+        gid = getegid();
+
     ngroups = virGetGroupList(uid, gid, &groups);
     if (ngroups < 0)
         return -errno;
-- 
2.3.6