From a0f43d820d7488b8b19dd17ca1ada87e034fb3bb Mon Sep 17 00:00:00 2001
From: Michal Privoznik <mprivozn@redhat.com>
Date: Wed, 7 Oct 2015 10:39:17 +0200
Subject: [PATCH] virSecurityDACSetOwnershipInternal: Don't chown so often

It's better if we stat() file that we are about to chown() at
first and check if there's something we need to change. Not that
it would make much difference, but for the upcoming patches we
need to be doing stat() anyway. Moreover, if we do things this
way, we can drop @chown_errno variable which will become
redundant.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/security/security_dac.c | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 0dfe570ba3..a1ab40abb4 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -242,7 +242,6 @@ virSecurityDACSetOwnershipInternal(virSecurityDACDataPtr priv,
                                    gid_t gid)
 {
     int rc;
-    int chown_errno;
 
     VIR_INFO("Setting DAC user and group on '%s' to '%ld:%ld'",
              NULLSTR(src ? src->path : path), (long) uid, (long) gid);
@@ -255,9 +254,6 @@ virSecurityDACSetOwnershipInternal(virSecurityDACDataPtr priv,
         /* on -2 returned an error was already reported */
         if (rc == -2)
             return -1;
-
-        /* on -1 only errno was set */
-        chown_errno = errno;
     } else {
         struct stat sb;
 
@@ -271,34 +267,34 @@ virSecurityDACSetOwnershipInternal(virSecurityDACDataPtr priv,
             path = src->path;
         }
 
-        rc = chown(path, uid, gid);
-        chown_errno = errno;
+        if (stat(path, &sb) < 0) {
+            virReportSystemError(errno, _("unable to stat: %s"), path);
+            return -1;
+        }
 
-        if (rc < 0 &&
-            stat(path, &sb) >= 0) {
-            if (sb.st_uid == uid &&
-                sb.st_gid == gid) {
-                /* It's alright, there's nothing to change anyway. */
-                return 0;
-            }
+        if (sb.st_uid == uid && sb.st_gid == gid) {
+            /* nothing to chown */
+            return 0;
         }
+
+        rc = chown(path, uid, gid);
     }
 
     if (rc < 0) {
-        if (chown_errno == EOPNOTSUPP || chown_errno == EINVAL) {
+        if (errno == EOPNOTSUPP || errno == EINVAL) {
             VIR_INFO("Setting user and group to '%ld:%ld' on '%s' not "
                      "supported by filesystem",
                      (long) uid, (long) gid, path);
-        } else if (chown_errno == EPERM) {
+        } else if (errno == EPERM) {
             VIR_INFO("Setting user and group to '%ld:%ld' on '%s' not "
                      "permitted",
                      (long) uid, (long) gid, path);
-        } else if (chown_errno == EROFS) {
+        } else if (errno == EROFS) {
             VIR_INFO("Setting user and group to '%ld:%ld' on '%s' not "
                      "possible on readonly filesystem",
                      (long) uid, (long) gid, path);
         } else {
-            virReportSystemError(chown_errno,
+            virReportSystemError(errno,
                                  _("unable to set user and group to '%ld:%ld' "
                                    "on '%s'"),
                                  (long) uid, (long) gid, path);
-- 
GitLab