Rename virFileCreate to virFileOperation, add hook function

It turns out it is also useful to be able to perform other operations
on a file created while running as a different uid (eg, write things
to that file), and possibly to do this to a file that already
exists. This patch adds an optional hook function to the renamed (for
more accuracy of purpose) virFileOperation; the hook will be called
after the file has been opened (possibly created) and gid/mode
checked/set, before closing it.

As with the other operations on the file, if the VIR_FILE_OP_AS_UID
flag is set, this hook function will be called in the context of a
child process forked from the process that called virFileOperation.
The implication here is that, while all data in memory is available to
this hook function, any modification to that data will not be seen by
the caller - the only indication in memory of what happened in the
hook will be the return value (which the hook should set to 0 on
success, or one of the standard errno values on failure).

Another piece of making the function more flexible was to add an
"openflags" argument. This arg should contain exactly the flags to be
passed to open(2), eg O_RDWR | O_EXCL, etc.

In the process of adding the hook to virFileOperation, I also realized
that the bits to fix up file owner/group/mode settings after creation
were being done in the parent process, which could fail, so I moved
them to the child process where they should be.

* src/util/util.[ch]: rename and rework virFileCreate-->virFileOperation,
  and redo flags in virDirCreate
* storage/storage_backend.c, storage/storage_backend_fs.c: update the
  calls to virFileOperation/virDirCreate to reflect changes in the API,
  but don't yet take advantage of the hook.

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index a12ddc744515f2a083b6c0ad360486ee6e557663..07c116ae3395f340f618dbc9172abba0aaffc014 100644 (file)
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -290,10 +290,13 @@ virStorageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED,
         return -1;
     }
 
-    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) {
+    if ((createstat = virFileOperation(vol->target.path, O_RDWR | O_CREAT | O_EXCL,
+                                       vol->target.perms.mode,
+                                       vol->target.perms.uid, vol->target.perms.gid,
+                                       NULL, NULL,
+                                       VIR_FILE_OP_FORCE_PERMS |
+                                       (pool->def->type == VIR_STORAGE_POOL_NETFS
+                                        ? VIR_FILE_OP_AS_UID : 0))) < 0) {
         virReportSystemError(createstat,
                              _("cannot create path '%s'"),
                              vol->target.path);

diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index bbd5787a10333787fe8633d60c390a3d20019179..8279d781f152b7e4d77fcf04cfe390b7e2871c6e 100644 (file)
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -533,9 +533,9 @@ virStorageBackendFileSystemBuild(virConnectPtr conn ATTRIBUTE_UNUSED,
                             pool->def->target.perms.mode,
                             pool->def->target.perms.uid,
                             pool->def->target.perms.gid,
-                            VIR_FILE_CREATE_ALLOW_EXIST |
+                            VIR_DIR_CREATE_FORCE_PERMS | VIR_DIR_CREATE_ALLOW_EXIST |
                             (pool->def->type == VIR_STORAGE_POOL_NETFS
-                             ? VIR_FILE_CREATE_AS_UID : 0)) != 0)) {
+                             ? VIR_DIR_CREATE_AS_UID : 0)) != 0)) {
         virReportSystemError(err, _("cannot create path '%s'"),
                              pool->def->target.path);
         goto error;
@@ -779,8 +779,9 @@ static int createFileDir(virConnectPtr conn ATTRIBUTE_UNUSED,
 
     if ((err = virDirCreate(vol->target.path, vol->target.perms.mode,
                             vol->target.perms.uid, vol->target.perms.gid,
+                            VIR_DIR_CREATE_FORCE_PERMS |
                             (pool->def->type == VIR_STORAGE_POOL_NETFS
-                             ? VIR_FILE_CREATE_AS_UID : 0))) != 0) {
+                             ? VIR_DIR_CREATE_AS_UID : 0))) != 0) {
         virReportSystemError(err, _("cannot create path '%s'"),
                              vol->target.path);
         return -1;

diff --git a/src/util/util.c b/src/util/util.c
index 1182729a2427e99a45cccbbc733cc6a9bb39889c..624e5702455688a064f8276ee387f21cfce34031 100644 (file)
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -1212,15 +1212,15 @@ int virFileExists(const char *path)
 }
 
 
-static int virFileCreateSimple(const char *path, mode_t mode, uid_t uid, gid_t gid,
-                               unsigned int flags) {
-    int open_flags = O_RDWR | O_CREAT | ((flags & VIR_FILE_CREATE_ALLOW_EXIST)
-                                          ? 0 : O_EXCL);
+static int virFileOperationNoFork(const char *path, int openflags, mode_t mode,
+                                  uid_t uid, gid_t gid,
+                                  virFileOperationHook hook, void *hookdata,
+                                  unsigned int flags) {
     int fd = -1;
     int ret = 0;
     struct stat st;
 
-    if ((fd = open(path, open_flags, mode)) < 0) {
+    if ((fd = open(path, openflags, mode)) < 0) {
         ret = errno;
         virReportSystemError(errno, _("failed to create file '%s'"),
                              path);
@@ -1238,13 +1238,17 @@ static int virFileCreateSimple(const char *path, mode_t mode, uid_t uid, gid_t g
                              path, uid, gid);
         goto error;
     }
-    if (fchmod(fd, mode) < 0) {
+    if ((flags & VIR_FILE_OP_FORCE_PERMS)
+        && (fchmod(fd, mode) < 0)) {
         ret = errno;
         virReportSystemError(errno,
                              _("cannot set mode of '%s' to %04o"),
                              path, mode);
         goto error;
     }
+    if ((hook) && ((ret = hook(fd, hookdata)) != 0)) {
+        goto error;
+    }
     if (close(fd) < 0) {
         ret = errno;
         virReportSystemError(errno, _("failed to close new file '%s'"),
@@ -1259,13 +1263,13 @@ error:
     return ret;
 }
 
-static int virDirCreateSimple(const char *path, mode_t mode, uid_t uid, gid_t gid,
+static int virDirCreateNoFork(const char *path, mode_t mode, uid_t uid, gid_t gid,
                               unsigned int flags) {
     int ret = 0;
     struct stat st;
 
     if ((mkdir(path, mode) < 0)
-        && !((errno == EEXIST) && (flags & VIR_FILE_CREATE_ALLOW_EXIST)))
+        && !((errno == EEXIST) && (flags & VIR_DIR_CREATE_ALLOW_EXIST)))
        {
         ret = errno;
         virReportSystemError(errno, _("failed to create directory '%s'"),
@@ -1285,7 +1289,8 @@ static int virDirCreateSimple(const char *path, mode_t mode, uid_t uid, gid_t gi
                              path, uid, gid);
         goto error;
     }
-    if (chmod(path, mode) < 0) {
+    if ((flags & VIR_DIR_CREATE_FORCE_PERMS)
+        && (chmod(path, mode) < 0)) {
         ret = errno;
         virReportSystemError(errno,
                              _("cannot set mode of '%s' to %04o"),
@@ -1297,18 +1302,20 @@ error:
 }
 
 #ifndef WIN32
-int virFileCreate(const char *path, mode_t mode,
-                  uid_t uid, gid_t gid, unsigned int flags) {
+int virFileOperation(const char *path, int openflags, mode_t mode,
+                     uid_t uid, gid_t gid,
+                     virFileOperationHook hook, void *hookdata,
+                     unsigned int flags) {
     struct stat st;
     pid_t pid;
     int waitret, status, ret = 0;
     int fd;
 
-    if ((!(flags & VIR_FILE_CREATE_AS_UID))
+    if ((!(flags & VIR_FILE_OP_AS_UID))
         || (getuid() != 0)
-        || ((uid == 0) && (gid == 0))
-        || ((flags & VIR_FILE_CREATE_ALLOW_EXIST) && (stat(path, &st) >= 0))) {
-        return virFileCreateSimple(path, mode, uid, gid, flags);
+        || ((uid == 0) && (gid == 0))) {
+        return virFileOperationNoFork(path, openflags, mode, uid, gid,
+                                      hook, hookdata, flags);
     }
 
     /* parent is running as root, but caller requested that the
@@ -1338,34 +1345,8 @@ int virFileCreate(const char *path, mode_t mode,
         if (!WIFEXITED(status) || (ret == EACCES)) {
             /* fall back to the simpler method, which works better in
              * some cases */
-            return virFileCreateSimple(path, mode, uid, gid, flags);
-        }
-        if (ret != 0) {
-            goto parenterror;
-        }
-
-        /* check if group was set properly by creating after
-         * setgid. If not, try doing it with chown */
-        if (stat(path, &st) == -1) {
-            ret = errno;
-            virReportSystemError(errno,
-                                 _("stat of '%s' failed"),
-                                 path);
-            goto parenterror;
-        }
-        if ((st.st_gid != gid) && (chown(path, -1, gid) < 0)) {
-            ret = errno;
-            virReportSystemError(errno,
-                                 _("cannot chown '%s' to group %u"),
-                                 path, gid);
-            goto parenterror;
-        }
-        if (chmod(path, mode) < 0) {
-            ret = errno;
-            virReportSystemError(errno,
-                                 _("cannot set mode of '%s' to %04o"),
-                                 path, mode);
-            goto parenterror;
+            return virFileOperationNoFork(path, openflags, mode, uid, gid,
+                                          hook, hookdata, flags);
         }
 parenterror:
         return ret;
@@ -1395,7 +1376,7 @@ parenterror:
                              uid, path);
         goto childerror;
     }
-    if ((fd = open(path, O_RDWR | O_CREAT | O_EXCL, mode)) < 0) {
+    if ((fd = open(path, openflags, mode)) < 0) {
         ret = errno;
         if (ret != EACCES) {
             /* in case of EACCES, the parent will retry */
@@ -1405,6 +1386,29 @@ parenterror:
         }
         goto childerror;
     }
+    if (fstat(fd, &st) == -1) {
+        ret = errno;
+        virReportSystemError(errno, _("stat of '%s' failed"), path);
+        goto childerror;
+    }
+    if ((st.st_gid != gid)
+        && (fchown(fd, -1, gid) < 0)) {
+        ret = errno;
+        virReportSystemError(errno, _("cannot chown '%s' to (%u, %u)"),
+                             path, uid, gid);
+        goto childerror;
+    }
+    if ((flags & VIR_FILE_OP_FORCE_PERMS)
+        && (fchmod(fd, mode) < 0)) {
+        ret = errno;
+        virReportSystemError(errno,
+                             _("cannot set mode of '%s' to %04o"),
+                             path, mode);
+        goto childerror;
+    }
+    if ((hook) && ((ret = hook(fd, hookdata)) != 0)) {
+        goto childerror;
+    }
     if (close(fd) < 0) {
         ret = errno;
         virReportSystemError(errno, _("child failed to close new file '%s'"),
@@ -1423,11 +1427,11 @@ int virDirCreate(const char *path, mode_t mode,
     int waitret;
     int status, ret = 0;
 
-    if ((!(flags & VIR_FILE_CREATE_AS_UID))
+    if ((!(flags & VIR_DIR_CREATE_AS_UID))
         || (getuid() != 0)
         || ((uid == 0) && (gid == 0))
-        || ((flags & VIR_FILE_CREATE_ALLOW_EXIST) && (stat(path, &st) >= 0))) {
-        return virDirCreateSimple(path, mode, uid, gid, flags);
+        || ((flags & VIR_DIR_CREATE_ALLOW_EXIST) && (stat(path, &st) >= 0))) {
+        return virDirCreateNoFork(path, mode, uid, gid, flags);
     }
 
     int forkRet = virFork(&pid);
@@ -1451,34 +1455,11 @@ int virDirCreate(const char *path, mode_t mode,
         if (!WIFEXITED(status) || (ret == EACCES)) {
             /* fall back to the simpler method, which works better in
              * some cases */
-            return virDirCreateSimple(path, mode, uid, gid, flags);
+            return virDirCreateNoFork(path, mode, uid, gid, flags);
         }
         if (ret != 0) {
             goto parenterror;
         }
-
-        /* check if group was set properly by creating after
-         * setgid. If not, try doing it with chown */
-        if (stat(path, &st) == -1) {
-            ret = errno;
-            virReportSystemError(errno,
-                                 _("stat of '%s' failed"),
-                                 path);
-            goto parenterror;
-        }
-        if ((st.st_gid != gid) && (chown(path, -1, gid) < 0)) {
-            ret = errno;
-            virReportSystemError(errno,
-                                 _("cannot chown '%s' to group %u"),
-                                 path, gid);
-            goto parenterror;
-        }
-        if (chmod(path, mode) < 0) {
-            virReportSystemError(errno,
-                                 _("cannot set mode of '%s' to %04o"),
-                                 path, mode);
-            goto parenterror;
-        }
 parenterror:
         return ret;
     }
@@ -1513,20 +1494,45 @@ parenterror:
         }
         goto childerror;
     }
+    /* check if group was set properly by creating after
+     * setgid. If not, try doing it with chown */
+    if (stat(path, &st) == -1) {
+        ret = errno;
+        virReportSystemError(errno,
+                             _("stat of '%s' failed"), path);
+        goto childerror;
+    }
+    if ((st.st_gid != gid) && (chown(path, -1, gid) < 0)) {
+        ret = errno;
+        virReportSystemError(errno,
+                             _("cannot chown '%s' to group %u"),
+                             path, gid);
+        goto childerror;
+    }
+    if ((flags & VIR_DIR_CREATE_FORCE_PERMS)
+        && chmod(path, mode) < 0) {
+        virReportSystemError(errno,
+                             _("cannot set mode of '%s' to %04o"),
+                             path, mode);
+        goto childerror;
+    }
 childerror:
     _exit(ret);
 }
 
 #else /* WIN32 */
 
-int virFileCreate(const char *path, mode_t mode,
-                  uid_t uid, gid_t gid, unsigned int flags) {
-    return virFileCreateSimple(path, mode, uid, gid, flags);
+int virFileOperation(const char *path, int openflags, mode_t mode,
+                  uid_t uid, gid_t gid,
+                  virFileOperationHook hook, void *hookdata,
+                  unsigned int flags) {
+    return virFileOperationNoFork(path, openflags, mode, uid, gid,
+                                  hook, hookdata, flags);
 }
 
 int virDirCreate(const char *path, mode_t mode,
                  uid_t uid, gid_t gid, unsigned int flags) {
-    return virDirCreateSimple(path, mode, uid, gid, flags);
+    return virDirCreateNoFork(path, mode, uid, gid, flags);
 }
 #endif
 

diff --git a/src/util/util.h b/src/util/util.h
index 13bc39c7d6715e263150a0f2ee08a4fba910c7b4..cc05abe664c6aaf0e7b0d52b2c3787e0c1ae3fac 100644 (file)
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -110,13 +110,22 @@ char *virFindFileInPath(const char *file);
 int virFileExists(const char *path);
 
 enum {
-    VIR_FILE_CREATE_NONE        = 0,
-    VIR_FILE_CREATE_AS_UID      = (1 << 0),
-    VIR_FILE_CREATE_ALLOW_EXIST = (1 << 1),
+    VIR_FILE_OP_NONE        = 0,
+    VIR_FILE_OP_AS_UID      = (1 << 0),
+    VIR_FILE_OP_FORCE_PERMS = (1 << 1),
 };
+typedef int (*virFileOperationHook)(int fd, void *data);
+int virFileOperation(const char *path, int openflags, mode_t mode,
+                     uid_t uid, gid_t gid,
+                     virFileOperationHook hook, void *hookdata,
+                     unsigned int flags) ATTRIBUTE_RETURN_CHECK;
 
-int virFileCreate(const char *path, mode_t mode, uid_t uid, gid_t gid,
-                  unsigned int flags) ATTRIBUTE_RETURN_CHECK;
+enum {
+    VIR_DIR_CREATE_NONE        = 0,
+    VIR_DIR_CREATE_AS_UID      = (1 << 0),
+    VIR_DIR_CREATE_FORCE_PERMS = (1 << 1),
+    VIR_DIR_CREATE_ALLOW_EXIST = (1 << 2),
+};
 int virDirCreate(const char *path, mode_t mode, uid_t uid, gid_t gid,
                  unsigned int flags) ATTRIBUTE_RETURN_CHECK;
 int virFileMakePath(const char *path) ATTRIBUTE_RETURN_CHECK;