From 28c547ed6d3a42d3d838f294787e478b2126f1ed Mon Sep 17 00:00:00 2001
From: Cole Robinson <crobinso@redhat.com>
Date: Mon, 27 Apr 2015 16:48:05 -0400
Subject: [PATCH] storage: fs: Don't try to chown directory unless user
 requested

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 984c5713c5..8a06813925 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" */
+    /* 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 == (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();
 
-    if ((!(flags & VIR_DIR_CREATE_AS_UID))
-        || (geteuid() != 0)
-        || ((uid == 0) && (gid == 0))
-        || ((flags & VIR_DIR_CREATE_ALLOW_EXIST) && (stat(path, &st) >= 0))) {
-        return virDirCreateNoFork(path, mode, uid, gid, flags);
-    }
-
     ngroups = virGetGroupList(uid, gid, &groups);
     if (ngroups < 0)
         return -errno;