[PATCH v2 7/12] storage: Refactor and fix virStorageBackendCreateExecCommand

[John Ferlan <jferlan@xxxxxxxxxx>]

Refactor the code to handle the NETFS pool separately from other pool
types. When the original code was developed (commit id 'e1f27784')
the virCommandSetUmask did not exist (commit id '0e1a1a8c4'), so there
was no way to set the permissions for the creation. Now that it exists,
we can use it. The code currently fails to create a volume in a NETFS
pool from some other volume within the pool because the chmod done after
file creation fails.

So adjust the code to jump to cleanup once the file is successfully
created in the NETFS pool. If it fails or for non NETFS files, just
perform the create, chown, and chmod in order

Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
---
 src/storage/storage_backend.c | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index a375fe0..037d6d7 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -678,7 +678,6 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool,
     gid_t gid;
     uid_t uid;
     mode_t mode;
-    bool filecreated = false;
     int ret = -1;
 
     if ((pool->def->type == VIR_STORAGE_POOL_NETFS)
@@ -690,26 +689,29 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool,
 
         virCommandSetUID(cmd, vol->target.perms->uid);
         virCommandSetGID(cmd, vol->target.perms->gid);
+        mode = (vol->target.perms->mode == (mode_t) -1 ?
+            VIR_STORAGE_DEFAULT_VOL_PERM_MODE : vol->target.perms->mode);
+        virCommandSetUmask(cmd, 0777 - mode);
 
-        if (virCommandRun(cmd, NULL) == 0) {
-            /* command was successfully run, check if the file was created */
-            if (stat(vol->target.path, &st) >= 0)
-                filecreated = true;
+        if ((ret = virCommandRun(cmd, NULL)) == 0) {
+            /* command was successfully run, if the file was created
+             * then we are done */
+            if (virFileExists(vol->target.path))
+                goto cleanup;
         }
     }
 
-    /* don't change uid/gid if we retry */
+    /* don't change uid/gid/mode if we retry */
     virCommandSetUID(cmd, -1);
     virCommandSetGID(cmd, -1);
+    virCommandSetUmask(cmd, 0);
 
-    if (!filecreated) {
-        if (virCommandRun(cmd, NULL) < 0)
-            goto cleanup;
-        if (stat(vol->target.path, &st) < 0) {
-            virReportSystemError(errno,
-                                 _("failed to create %s"), vol->target.path);
-            goto cleanup;
-        }
+    if (virCommandRun(cmd, NULL) < 0)
+        goto cleanup;
+    if (stat(vol->target.path, &st) < 0) {
+        virReportSystemError(errno,
+                             _("failed to create %s"), vol->target.path);
+        goto cleanup;
     }
 
     uid = (vol->target.perms->uid != st.st_uid) ? vol->target.perms->uid
-- 
2.1.0

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