[PATCH v3 1/6] storage: Fix setting mode in virStorageBackendCreateExecCommand

[John Ferlan <jferlan@xxxxxxxxxx>]

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.

Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
---

Change over v2:

  - Revert back to the original style of using filecreated in order
    to determine whether or not to run the second virCommandRun instead
    of checking status and virFileExists

  - This means the 'stat' will be run and the subsequent chown and chmod
    could be run, but more than likely won't since the values should match.

  - Use of S_IRWXUGO instead of 0777

  - Compare the current st_mode against the expected mode and only call
    chmod if we need to change.

 src/storage/storage_backend.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index a375fe0..2ba6e27 100644
--- 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);
-- 
2.1.0

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