Create storage volumes directly with desired uid/gid

In order to avoid problems trying to chown files that were created by
root on a root-squashing nfs server, fork a new process that setuid's
to the desired uid before creating the file. (It's only done this way
if the pool containing the new volume is of type 'netfs', otherwise
the old method of creating the file followed by chown() is used.)

This changes the semantics of the "create_func" slightly - previously
it was assumed that this function just created the file, then the
caller would chown it to the desired uid. Now, create_func does both
operations.

There are multiple functions that can take on the role of create_func:

createFileDir - previously called mkdir(), now calls virDirCreate().
virStorageBackendCreateRaw - previously called open(),
                             now calls virFileCreate().
virStorageBackendCreateQemuImg - use virRunWithHook() to setuid/gid.
virStorageBackendCreateQcowCreate - same.
virStorageBackendCreateBlockFrom - preserve old behavior (but attempt
                                   chown when necessary even if not root)

* src/storage/storage_backend.[ch] src/storage/storage_backend_disk.c
  src/storage/storage_backend_fs.c src/storage/storage_backend_logical.c
  src/storage/storage_driver.c: change the create_func implementations,
  also propagate the pool information to be able to detect NETFS ones.

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 9dc801c4f6af74a771469f259e925ee17ea37f02..bc656f24e0b86062d7508672b8ce97dca741947b 100644 (file)
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -205,6 +205,7 @@ cleanup:
 
 static int
 virStorageBackendCreateBlockFrom(virConnectPtr conn,
+                                 virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
                                  virStorageVolDefPtr vol,
                                  virStorageVolDefPtr inputvol,
                                  unsigned int flags ATTRIBUTE_UNUSED)
@@ -212,6 +213,9 @@ virStorageBackendCreateBlockFrom(virConnectPtr conn,
     int fd = -1;
     int ret = -1;
     unsigned long long remain;
+    struct stat st;
+    gid_t gid;
+    uid_t uid;
 
     if ((fd = open(vol->target.path, O_RDWR)) < 0) {
         virReportSystemError(conn, errno,
@@ -229,6 +233,28 @@ virStorageBackendCreateBlockFrom(virConnectPtr conn,
             goto cleanup;
     }
 
+    if (fstat(fd, &st) == -1) {
+        ret = errno;
+        virReportSystemError(conn, errno, _("stat of '%s' failed"),
+                             vol->target.path);
+        goto cleanup;
+    }
+    uid = (vol->target.perms.uid != st.st_uid) ? vol->target.perms.uid : -1;
+    gid = (vol->target.perms.gid != st.st_gid) ? vol->target.perms.gid : -1;
+    if (((uid != -1) || (gid != -1))
+        && (fchown(fd, vol->target.perms.uid, vol->target.perms.gid) < 0)) {
+        virReportSystemError(conn, errno,
+                             _("cannot chown '%s' to (%u, %u)"),
+                             vol->target.path, vol->target.perms.uid,
+                             vol->target.perms.gid);
+        goto cleanup;
+    }
+    if (fchmod(fd, vol->target.perms.mode) < 0) {
+        virReportSystemError(conn, errno,
+                             _("cannot set mode of '%s' to %04o"),
+                             vol->target.path, vol->target.perms.mode);
+        goto cleanup;
+    }
     if (close(fd) < 0) {
         virReportSystemError(conn, errno,
                              _("cannot close file '%s'"),
@@ -247,12 +273,14 @@ cleanup:
 
 int
 virStorageBackendCreateRaw(virConnectPtr conn,
+                           virStoragePoolObjPtr pool,
                            virStorageVolDefPtr vol,
                            virStorageVolDefPtr inputvol,
                            unsigned int flags ATTRIBUTE_UNUSED)
 {
     int fd = -1;
     int ret = -1;
+    int createstat;
     unsigned long long remain;
     char *buf = NULL;
 
@@ -263,14 +291,23 @@ virStorageBackendCreateRaw(virConnectPtr conn,
         return -1;
     }
 
-    if ((fd = open(vol->target.path, O_RDWR | O_CREAT | O_EXCL,
-                   vol->target.perms.mode)) < 0) {
-        virReportSystemError(conn, errno,
+    if ((createstat = virFileCreate(vol->target.path, vol->target.perms.mode,
+                                    vol->target.perms.uid, vol->target.perms.gid,
+                                    (pool->def->type == VIR_STORAGE_POOL_NETFS
+                                     ? VIR_FILE_CREATE_AS_UID : 0))) < 0) {
+        virReportSystemError(conn, createstat,
                              _("cannot create path '%s'"),
                              vol->target.path);
         goto cleanup;
     }
 
+    if ((fd = open(vol->target.path, O_RDWR | O_EXCL)) < 0) {
+        virReportSystemError(conn, errno,
+                             _("cannot open new path '%s'"),
+                             vol->target.path);
+        goto cleanup;
+    }
+
     /* Seek to the final size, so the capacity is available upfront
      * for progress reporting */
     if (ftruncate(fd, vol->capacity) < 0) {
@@ -453,12 +490,87 @@ cleanup:
     return ret;
 }
 
+static int virStorageBuildSetUIDHook(void *data) {
+    virStorageVolDefPtr vol = data;
+
+    if ((vol->target.perms.gid != 0)
+        && (setgid(vol->target.perms.gid) != 0)) {
+        virReportSystemError(NULL, errno,
+                             _("Cannot set gid to %u before creating %s"),
+                             vol->target.perms.gid, vol->target.path);
+        return -1;
+    }
+    if ((vol->target.perms.uid != 0)
+        && (setuid(vol->target.perms.uid) != 0)) {
+        virReportSystemError(NULL, errno,
+                             _("Cannot set uid to %u before creating %s"),
+                             vol->target.perms.uid, vol->target.path);
+        return -1;
+    }
+    return 0;
+}
+
+static int virStorageBackendCreateExecCommand(virConnectPtr conn,
+                                              virStoragePoolObjPtr pool,
+                                              virStorageVolDefPtr vol,
+                                              const char **cmdargv) {
+    struct stat st;
+    gid_t gid;
+    uid_t uid;
+    int filecreated = 0;
+
+    if ((pool->def->type == VIR_STORAGE_POOL_NETFS)
+        && (getuid() == 0)
+        && ((vol->target.perms.uid != 0) || (vol->target.perms.gid != 0))) {
+        if (virRunWithHook(conn, cmdargv,
+                           virStorageBuildSetUIDHook, vol, NULL) == 0) {
+            /* command was successfully run, check if the file was created */
+            if (stat(vol->target.path, &st) >=0)
+                filecreated = 1;
+        }
+    }
+    if (!filecreated) {
+        if (virRun(conn, cmdargv, NULL) < 0) {
+            virReportSystemError(conn, errno,
+                                 _("Cannot run %s to create %s"),
+                                 cmdargv[0], vol->target.path);
+            return -1;
+        }
+        if (stat(vol->target.path, &st) < 0) {
+            virReportSystemError(conn, errno,
+                                 _("%s failed to create %s"),
+                                 cmdargv[0], vol->target.path);
+            return -1;
+        }
+    }
+
+    uid = (vol->target.perms.uid != st.st_uid) ? vol->target.perms.uid : -1;
+    gid = (vol->target.perms.gid != st.st_gid) ? vol->target.perms.gid : -1;
+    if (((uid != -1) || (gid != -1))
+        && (chown(vol->target.path, uid, gid) < 0)) {
+        virReportSystemError(conn, errno,
+                             _("cannot chown %s to (%u, %u)"),
+                             vol->target.path, vol->target.perms.uid,
+                             vol->target.perms.gid);
+        return -1;
+    }
+    if (chmod(vol->target.path, vol->target.perms.mode) < 0) {
+        virReportSystemError(conn, errno,
+                             _("cannot set mode of '%s' to %04o"),
+                             vol->target.path, vol->target.perms.mode);
+        return -1;
+    }
+    return 0;
+}
+
 static int
 virStorageBackendCreateQemuImg(virConnectPtr conn,
+                               virStoragePoolObjPtr pool,
                                virStorageVolDefPtr vol,
                                virStorageVolDefPtr inputvol,
                                unsigned int flags ATTRIBUTE_UNUSED)
 {
+    int ret;
     char size[100];
     char *create_tool;
     short use_kvmimg;
@@ -616,14 +728,10 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
     /* Size in KB */
     snprintf(size, sizeof(size), "%lluK", vol->capacity/1024);
 
-    if (virRun(conn, imgargv, NULL) < 0) {
-        VIR_FREE(imgargv[0]);
-        return -1;
-    }
-
+    ret = virStorageBackendCreateExecCommand(conn, pool, vol, imgargv);
     VIR_FREE(imgargv[0]);
 
-    return 0;
+    return ret;
 }
 
 /*
@@ -632,10 +740,12 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
  */
 static int
 virStorageBackendCreateQcowCreate(virConnectPtr conn,
+                                  virStoragePoolObjPtr pool,
                                   virStorageVolDefPtr vol,
                                   virStorageVolDefPtr inputvol,
                                   unsigned int flags ATTRIBUTE_UNUSED)
 {
+    int ret;
     char size[100];
     const char *imgargv[4];
 
@@ -672,14 +782,10 @@ virStorageBackendCreateQcowCreate(virConnectPtr conn,
     imgargv[2] = vol->target.path;
     imgargv[3] = NULL;
 
-    if (virRun(conn, imgargv, NULL) < 0) {
-        VIR_FREE(imgargv[0]);
-        return -1;
-    }
-
+    ret = virStorageBackendCreateExecCommand(conn, pool, vol, imgargv);
     VIR_FREE(imgargv[0]);
 
-    return 0;
+    return ret;
 }
 
 virStorageBackendBuildVolFrom

diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 1c38eeb7731218292392c397363c79b7d2c23f4f..988539cd788163b3e38244375889d36a7fd6dbae 100644 (file)
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -35,14 +35,18 @@ typedef int (*virStorageBackendRefreshPool)(virConnectPtr conn, virStoragePoolOb
 typedef int (*virStorageBackendStopPool)(virConnectPtr conn, virStoragePoolObjPtr pool);
 typedef int (*virStorageBackendDeletePool)(virConnectPtr conn, virStoragePoolObjPtr pool, unsigned int flags);
 
-typedef int (*virStorageBackendBuildVol)(virConnectPtr conn, virStorageVolDefPtr vol);
+typedef int (*virStorageBackendBuildVol)(virConnectPtr conn,
+                                         virStoragePoolObjPtr pool, virStorageVolDefPtr vol);
 typedef int (*virStorageBackendCreateVol)(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol);
 typedef int (*virStorageBackendRefreshVol)(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol);
 typedef int (*virStorageBackendDeleteVol)(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int flags);
-typedef int (*virStorageBackendBuildVolFrom)(virConnectPtr conn, virStorageVolDefPtr origvol, virStorageVolDefPtr newvol, unsigned int flags);
+typedef int (*virStorageBackendBuildVolFrom)(virConnectPtr conn, virStoragePoolObjPtr pool,
+                                             virStorageVolDefPtr origvol, virStorageVolDefPtr newvol,
+                                             unsigned int flags);
 
 /* File creation/cloning functions used for cloning between backends */
 int virStorageBackendCreateRaw(virConnectPtr conn,
+                               virStoragePoolObjPtr pool,
                                virStorageVolDefPtr vol,
                                virStorageVolDefPtr inputvol,
                                unsigned int flags);

diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c
index 72ccd8167a04e9a84b370fbc7aa498033089bc31..5ecf2105a291bd230149acf5443a5212e0492925 100644 (file)
--- a/src/storage/storage_backend_disk.c
+++ b/src/storage/storage_backend_disk.c
@@ -599,6 +599,7 @@ virStorageBackendDiskCreateVol(virConnectPtr conn,
 
 static int
 virStorageBackendDiskBuildVolFrom(virConnectPtr conn,
+                                  virStoragePoolObjPtr pool,
                                   virStorageVolDefPtr vol,
                                   virStorageVolDefPtr inputvol,
                                   unsigned int flags)
@@ -609,7 +610,7 @@ virStorageBackendDiskBuildVolFrom(virConnectPtr conn,
     if (!build_func)
         return -1;
 
-    return build_func(conn, vol, inputvol, flags);
+    return build_func(conn, pool, vol, inputvol, flags);
 }
 
 static int

diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index b03e4e92a4af00199e3bcf141e6e2f020dd54a35..cc26f2f487b129f60229d8e8d4aa4e4c033c6c0f 100644 (file)
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -737,9 +737,12 @@ virStorageBackendFileSystemVolCreate(virConnectPtr conn,
 }
 
 static int createFileDir(virConnectPtr conn,
+                         virStoragePoolObjPtr pool,
                          virStorageVolDefPtr vol,
                          virStorageVolDefPtr inputvol,
                          unsigned int flags ATTRIBUTE_UNUSED) {
+    int err;
+
     if (inputvol) {
         virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
                               "%s",
@@ -747,9 +750,11 @@ static int createFileDir(virConnectPtr conn,
         return -1;
     }
 
-    if (mkdir(vol->target.path, vol->target.perms.mode) < 0) {
-        virReportSystemError(conn, errno,
-                             _("cannot create path '%s'"),
+    if ((err = virDirCreate(vol->target.path, vol->target.perms.mode,
+                            vol->target.perms.uid, vol->target.perms.gid,
+                            (pool->def->type == VIR_STORAGE_POOL_NETFS
+                             ? VIR_FILE_CREATE_AS_UID : 0))) != 0) {
+        virReportSystemError(conn, err, _("cannot create path '%s'"),
                              vol->target.path);
         return -1;
     }
@@ -759,10 +764,10 @@ static int createFileDir(virConnectPtr conn,
 
 static int
 _virStorageBackendFileSystemVolBuild(virConnectPtr conn,
+                                     virStoragePoolObjPtr pool,
                                      virStorageVolDefPtr vol,
                                      virStorageVolDefPtr inputvol)
 {
-    int fd;
     virStorageBackendBuildVolFrom create_func;
     int tool_type;
 
@@ -794,41 +799,8 @@ _virStorageBackendFileSystemVolBuild(virConnectPtr conn,
         return -1;
     }
 
-    if (create_func(conn, vol, inputvol, 0) < 0)
-        return -1;
-
-    if ((fd = open(vol->target.path, O_RDONLY)) < 0) {
-        virReportSystemError(conn, errno,
-                             _("cannot read path '%s'"),
-                             vol->target.path);
+    if (create_func(conn, pool, vol, inputvol, 0) < 0)
         return -1;
-    }
-
-    /* We can only chown/grp if root */
-    if (getuid() == 0) {
-        if (fchown(fd, vol->target.perms.uid, vol->target.perms.gid) < 0) {
-            virReportSystemError(conn, errno,
-                                 _("cannot set file owner '%s'"),
-                                 vol->target.path);
-            close(fd);
-            return -1;
-        }
-    }
-    if (fchmod(fd, vol->target.perms.mode) < 0) {
-        virReportSystemError(conn, errno,
-                             _("cannot set file mode '%s'"),
-                             vol->target.path);
-        close(fd);
-        return -1;
-    }
-
-    if (close(fd) < 0) {
-        virReportSystemError(conn, errno,
-                             _("cannot close file '%s'"),
-                             vol->target.path);
-        return -1;
-    }
-
     return 0;
 }
 
@@ -839,8 +811,9 @@ _virStorageBackendFileSystemVolBuild(virConnectPtr conn,
  */
 static int
 virStorageBackendFileSystemVolBuild(virConnectPtr conn,
+                                    virStoragePoolObjPtr pool,
                                     virStorageVolDefPtr vol) {
-    return _virStorageBackendFileSystemVolBuild(conn, vol, NULL);
+    return _virStorageBackendFileSystemVolBuild(conn, pool, vol, NULL);
 }
 
 /*
@@ -848,10 +821,11 @@ virStorageBackendFileSystemVolBuild(virConnectPtr conn,
  */
 static int
 virStorageBackendFileSystemVolBuildFrom(virConnectPtr conn,
+                                        virStoragePoolObjPtr pool,
                                         virStorageVolDefPtr vol,
                                         virStorageVolDefPtr inputvol,
                                         unsigned int flags ATTRIBUTE_UNUSED) {
-    return _virStorageBackendFileSystemVolBuild(conn, vol, inputvol);
+    return _virStorageBackendFileSystemVolBuild(conn, pool, vol, inputvol);
 }
 
 /**

diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c
index 5eebfac92d2d3750248f5c8aadf6efec2741da5e..c2e74a5927e651f0d7665a22990632a7b76d1869 100644 (file)
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -660,6 +660,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
 
 static int
 virStorageBackendLogicalBuildVolFrom(virConnectPtr conn,
+                                     virStoragePoolObjPtr pool,
                                      virStorageVolDefPtr vol,
                                      virStorageVolDefPtr inputvol,
                                      unsigned int flags)
@@ -670,7 +671,7 @@ virStorageBackendLogicalBuildVolFrom(virConnectPtr conn,
     if (!build_func)
         return -1;
 
-    return build_func(conn, vol, inputvol, flags);
+    return build_func(conn, pool, vol, inputvol, flags);
 }
 
 static int

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index c2f7850aa261ff1d0c9a21d61d28be73525905ef..50fcbe2ee4ba5f1364ca07fc14ac1976f551708b 100644 (file)
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1331,7 +1331,7 @@ storageVolumeCreateXML(virStoragePoolPtr obj,
         voldef->building = 1;
         virStoragePoolObjUnlock(pool);
 
-        buildret = backend->buildVol(obj->conn, buildvoldef);
+        buildret = backend->buildVol(obj->conn, pool, buildvoldef);
 
         storageDriverLock(driver);
         virStoragePoolObjLock(pool);
@@ -1484,7 +1484,7 @@ storageVolumeCreateXMLFrom(virStoragePoolPtr obj,
         virStoragePoolObjUnlock(origpool);
     }
 
-    buildret = backend->buildVolFrom(obj->conn, newvol, origvol, flags);
+    buildret = backend->buildVolFrom(obj->conn, pool, newvol, origvol, flags);
 
     storageDriverLock(driver);
     virStoragePoolObjLock(pool);