[libvirt] [PATCH 1/5] storage: split virStorageBackendCreateExecCommand in two functions

Dmitry Andreev dandreev at virtuozzo.com
Tue Dec 8 14:11:11 UTC 2015


This patch introduces virStorageBackendCreateExec function
that has simple type arguments so it can be used without
'virStorageXXX' structures.

---
 src/storage/storage_backend.c | 130 +++++++++++++++++++++++-------------------
 1 file changed, 70 insertions(+), 60 deletions(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 194736b..2e14af9 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -675,92 +675,89 @@ virStorageGenerateQcowEncryption(virConnectPtr conn,
     return ret;
 }
 
-static int
-virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool,
-                                   virStorageVolDefPtr vol,
-                                   virCommandPtr cmd)
+static
+int virStorageBackendCreateExec(virCommandPtr cmd, const char* path,
+                                uid_t uid, gid_t gid, mode_t mode)
 {
     struct stat st;
-    gid_t gid;
-    uid_t uid;
-    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;
 
-    if ((pool->def->type == VIR_STORAGE_POOL_NETFS)
-        && (((geteuid() == 0)
-             && (vol->target.perms->uid != (uid_t) -1)
-             && (vol->target.perms->uid != 0))
-            || ((vol->target.perms->gid != (gid_t) -1)
-                && (vol->target.perms->gid != getegid())))) {
-
-        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 */
-            if (stat(vol->target.path, &st) >= 0) {
-                filecreated = true;
-
-                /* seems qemu-img disregards umask and open/creates using 0644.
-                 * If that doesn't match what we expect, then let's try to
-                 * re-open the file and attempt to force the mode change.
-                 */
-                if (mode != (st.st_mode & S_IRWXUGO)) {
-                    int fd = -1;
-                    int flags = VIR_FILE_OPEN_FORK | VIR_FILE_OPEN_FORCE_MODE;
-
-                    if ((fd = virFileOpenAs(vol->target.path, O_RDWR, mode,
-                                            vol->target.perms->uid,
-                                            vol->target.perms->gid,
-                                            flags)) >= 0) {
-                        /* Success - means we're good */
-                        VIR_FORCE_CLOSE(fd);
-                        ret = 0;
-                        goto cleanup;
-                    }
-                }
-            }
+    /* check is there a need to set uid/gid */
+    if ((uid == (uid_t) -1 || geteuid() == uid)
+        && (gid == (gid_t) -1 || geteuid() == gid))
+        goto retry;
+
+    /* first try - create with uid/gid/umask
+     * Originaly this try was added for NETFS
+     * that doesn't support chmod.
+     */
+    virCommandSetUID(cmd, uid);
+    virCommandSetGID(cmd, gid);
+    virCommandSetUmask(cmd, S_IRWXUGO ^ mode);
+
+    if (virCommandRun(cmd, NULL) < 0)
+        goto retry;
+
+    if (stat(path, &st) < 0)
+        goto retry;
+
+    filecreated = true;
+
+    /* seems qemu-img disregards umask and open/creates using 0644.
+     * If that doesn't match what we expect, then let's try to
+     * re-open the file and attempt to force the mode change.
+     */
+    if (mode != (st.st_mode & S_IRWXUGO)) {
+        int fd = -1;
+        int flags = VIR_FILE_OPEN_FORK | VIR_FILE_OPEN_FORCE_MODE;
+
+        if ((fd = virFileOpenAs(path, O_RDWR, mode,
+                                uid, gid, flags)) >= 0) {
+            /* Success - means we're good */
+            VIR_FORCE_CLOSE(fd);
+            ret = 0;
+            virReportSystemError(errno, _("no need to retry %s"), path);
+            goto cleanup;
         }
     }
 
+ retry:
+    /* second try - set uid/gid/umask after creation */
     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) {
+
+        if (stat(path, &st) < 0) {
             virReportSystemError(errno,
-                                 _("failed to create %s"), vol->target.path);
+                                 _("failed to create %s"), path);
             goto cleanup;
         }
+
         filecreated = true;
     }
 
-    uid = (vol->target.perms->uid != st.st_uid) ? vol->target.perms->uid
-        : (uid_t) -1;
-    gid = (vol->target.perms->gid != st.st_gid) ? vol->target.perms->gid
-        : (gid_t) -1;
+    uid = (uid != st.st_uid) ? uid : (uid_t) -1;
+    gid = (gid != st.st_gid) ? gid : (gid_t) -1;
+
     if (((uid != (uid_t) -1) || (gid != (gid_t) -1))
-        && (chown(vol->target.path, uid, gid) < 0)) {
+        && (chown(path, uid, gid) < 0)) {
         virReportSystemError(errno,
                              _("cannot chown %s to (%u, %u)"),
-                             vol->target.path, (unsigned int) uid,
+                             path, (unsigned int) uid,
                              (unsigned int) gid);
         goto cleanup;
     }
 
     if (mode != (st.st_mode & S_IRWXUGO) &&
-        chmod(vol->target.path, mode) < 0) {
+        chmod(path, mode) < 0) {
         virReportSystemError(errno,
                              _("cannot set mode of '%s' to %04o"),
-                             vol->target.path, mode);
+                             path, mode);
         goto cleanup;
     }
 
@@ -768,11 +765,24 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool,
 
  cleanup:
     if (ret < 0 && filecreated)
-        virFileRemove(vol->target.path, vol->target.perms->uid,
-                      vol->target.perms->gid);
+        virFileRemove(path, uid, gid);
     return ret;
 }
 
+static int
+virStorageBackendCreateExecCommand(virStorageVolDefPtr vol,
+                                   virCommandPtr cmd)
+{
+    mode_t mode = (vol->target.perms->mode == (mode_t) -1 ?
+                   VIR_STORAGE_DEFAULT_VOL_PERM_MODE :
+                   vol->target.perms->mode);
+
+    return virStorageBackendCreateExec(cmd, vol->target.path,
+                                       vol->target.perms->uid,
+                                       vol->target.perms->gid,
+                                       mode);
+}
+
 enum {
     QEMU_IMG_BACKING_FORMAT_NONE = 0,
     QEMU_IMG_BACKING_FORMAT_FLAG,
@@ -1153,7 +1163,7 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
     if (!cmd)
         goto cleanup;
 
-    ret = virStorageBackendCreateExecCommand(pool, vol, cmd);
+    ret = virStorageBackendCreateExecCommand(vol, cmd);
 
     virCommandFree(cmd);
  cleanup:
@@ -1167,7 +1177,7 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
  */
 static int
 virStorageBackendCreateQcowCreate(virConnectPtr conn ATTRIBUTE_UNUSED,
-                                  virStoragePoolObjPtr pool,
+                                  virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
                                   virStorageVolDefPtr vol,
                                   virStorageVolDefPtr inputvol,
                                   unsigned int flags)
@@ -1217,7 +1227,7 @@ virStorageBackendCreateQcowCreate(virConnectPtr conn ATTRIBUTE_UNUSED,
 
     cmd = virCommandNewArgList("qcow-create", size, vol->target.path, NULL);
 
-    ret = virStorageBackendCreateExecCommand(pool, vol, cmd);
+    ret = virStorageBackendCreateExecCommand(vol, cmd);
     virCommandFree(cmd);
     VIR_FREE(size);
 
-- 
1.8.3.1




More information about the libvir-list mailing list