storage: Fix setting mode in virStorageBackendCreateExecCommand

Currently the code does not handle the NFS root squash environment
properly since if the file gets created, then the subsequent chmod
will fail in a root squash environment where we're creating a file
in the pool with qemu tools, such as seen via:

   $ virsh vol-create-from $pool $file.xml file.img --inputpool $pool

assuming $file.xml is creating a file of "<format type='qcow2'"> from
an existing file.img in the pool of "<format type='raw'>".

This patch will utilize the virCommandSetUmask when creating the file
in the NETFS pool. The virCommandSetUmask API was added in commit id
'0e1a1a8c4', which was after the original code was developed in commit
id 'e1f27784' to attempt to handle the root squash environment.

Also, rather than blindly attempting to chmod, check to see if the
st_mode bits from the stat match what we're trying to set and only
make the chmod if they don't.

Also, a slight adjustment to the fallback algorithm to move the
virCommandSetUID/virCommandSetGID inside the if (!filecreated) since
they're only useful if we need to attempt to create the file again.

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index a375fe0f514477eaef999027dad6664168610bfa..2ba6e2774c71f8a6d1fe89009fa8f8370b8d51a2 100644 (file)
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -677,7 +677,9 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool,
     struct stat st;
     gid_t gid;
     uid_t uid;
-    mode_t mode;
+    mode_t mode = (vol->target.perms->mode == (mode_t) -1 ?
+                   VIR_STORAGE_DEFAULT_VOL_PERM_MODE :
+                   vol->target.perms->mode);
     bool filecreated = false;
     int ret = -1;
 
@@ -690,6 +692,7 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool,
 
         virCommandSetUID(cmd, vol->target.perms->uid);
         virCommandSetGID(cmd, vol->target.perms->gid);
+        virCommandSetUmask(cmd, S_IRWXUGO ^ mode);
 
         if (virCommandRun(cmd, NULL) == 0) {
             /* command was successfully run, check if the file was created */
@@ -698,11 +701,12 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool,
         }
     }
 
-    /* don't change uid/gid if we retry */
-    virCommandSetUID(cmd, -1);
-    virCommandSetGID(cmd, -1);
-
     if (!filecreated) {
+        /* don't change uid/gid/mode if we retry */
+        virCommandSetUID(cmd, -1);
+        virCommandSetGID(cmd, -1);
+        virCommandSetUmask(cmd, 0);
+
         if (virCommandRun(cmd, NULL) < 0)
             goto cleanup;
         if (stat(vol->target.path, &st) < 0) {
@@ -725,9 +729,8 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool,
         goto cleanup;
     }
 
-    mode = (vol->target.perms->mode == (mode_t) -1 ?
-            VIR_STORAGE_DEFAULT_VOL_PERM_MODE : vol->target.perms->mode);
-    if (chmod(vol->target.path, mode) < 0) {
+    if (mode != (st.st_mode & S_IRWXUGO) &&
+        chmod(vol->target.path, mode) < 0) {
         virReportSystemError(errno,
                              _("cannot set mode of '%s' to %04o"),
                              vol->target.path, mode);