[libvirt] [PATCHv4 09/15] util: rename virFileOperation to virFileOpenAs

Eric Blake eblake at redhat.com
Thu Mar 10 01:45:49 UTC 2011


This patch intentionally doesn't change indentation, in order to
make it easier to review the real changes.

* src/util/util.h (VIR_FILE_OP_RETURN_FD, virFileOperationHook):
Delete.
(virFileOperation): Rename...
(virFileOpenAs): ...and reduce parameters.
* src/util/util.c (virFileOperationNoFork, virFileOperation):
Rename and simplify.
* src/qemu/qemu_driver.c (qemudDomainSaveFlag): Adjust caller.
* src/storage/storage_backend.c (virStorageBackendCreateRaw):
Likewise.
* src/libvirt_private.syms: Reflect rename.
---
 src/libvirt_private.syms      |    2 +-
 src/qemu/qemu_driver.c        |   20 +++-----
 src/storage/storage_backend.c |   12 ++--
 src/util/util.c               |  103 +++++++++++++----------------------------
 src/util/util.h               |   15 ++----
 5 files changed, 53 insertions(+), 99 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c0da78e..a9f7e39 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -876,8 +876,8 @@ virFileIsExecutable;
 virFileLinkPointsTo;
 virFileMakePath;
 virFileMatchesNameSuffix;
+virFileOpenAs;
 virFileOpenTty;
-virFileOperation;
 virFilePid;
 virFileReadAll;
 virFileReadLimFD;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a73c5b9..06bc969 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1880,11 +1880,9 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom,
             goto endjob;
         }
     } else {
-        if ((fd = virFileOperation(path, O_CREAT|O_TRUNC|O_WRONLY,
-                                   S_IRUSR|S_IWUSR,
-                                   getuid(), getgid(),
-                                   NULL, NULL,
-                                   VIR_FILE_OP_RETURN_FD)) < 0) {
+        if ((fd = virFileOpenAs(path, O_CREAT|O_TRUNC|O_WRONLY,
+                                S_IRUSR|S_IWUSR,
+                                getuid(), getgid(), 0)) < 0) {
             /* If we failed as root, and the error was permission-denied
                (EACCES or EPERM), assume it's on a network-connected share
                where root access is restricted (eg, root-squashed NFS). If the
@@ -1918,7 +1916,7 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom,

                 case 0:
                 default:
-                   /* local file - log the error returned by virFileOperation */
+                   /* local file - log the error returned by virFileOpenAs */
                    virReportSystemError(-rc,
                                     _("Failed to create domain save file '%s'"),
                                         path);
@@ -1929,12 +1927,10 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom,

             /* Retry creating the file as driver->user */

-            if ((fd = virFileOperation(path, O_CREAT|O_TRUNC|O_WRONLY,
-                                       S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP,
-                                       driver->user, driver->group,
-                                       NULL, NULL,
-                                       (VIR_FILE_OP_AS_UID |
-                                        VIR_FILE_OP_RETURN_FD))) < 0) {
+            if ((fd = virFileOpenAs(path, O_CREAT|O_TRUNC|O_WRONLY,
+                                    S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP,
+                                    driver->user, driver->group,
+                                    VIR_FILE_OPEN_AS_UID)) < 0) {
                 virReportSystemError(-fd,
                                    _("Error from child process creating '%s'"),
                                      path);
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index c7c5ccd..7a3a2b8 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -364,14 +364,14 @@ virStorageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED,

     uid = (vol->target.perms.uid == -1) ? getuid() : vol->target.perms.uid;
     gid = (vol->target.perms.gid == -1) ? getgid() : vol->target.perms.gid;
-    operation_flags = VIR_FILE_OP_FORCE_PERMS | VIR_FILE_OP_RETURN_FD;
+    operation_flags = VIR_FILE_OPEN_FORCE_PERMS;
     if (pool->def->type == VIR_STORAGE_POOL_NETFS)
-        operation_flags |= VIR_FILE_OP_AS_UID;
+        operation_flags |= VIR_FILE_OPEN_AS_UID;

-    if ((fd = virFileOperation(vol->target.path,
-                               O_RDWR | O_CREAT | O_EXCL,
-                               vol->target.perms.mode, uid, gid,
-                               NULL, NULL, operation_flags)) < 0) {
+    if ((fd = virFileOpenAs(vol->target.path,
+                            O_RDWR | O_CREAT | O_EXCL,
+                            vol->target.perms.mode, uid, gid,
+                            operation_flags)) < 0) {
         virReportSystemError(-fd,
                              _("cannot create path '%s'"),
                              vol->target.path);
diff --git a/src/util/util.c b/src/util/util.c
index 6bf3219..137cdb9 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -1355,10 +1355,10 @@ virFileIsExecutable(const char *file)

 #ifndef WIN32
 /* return -errno on failure, or 0 on success */
-static int virFileOperationNoFork(const char *path, int openflags, mode_t mode,
-                                  uid_t uid, gid_t gid,
-                                  virFileOperationHook hook, void *hookdata,
-                                  unsigned int flags) {
+static int
+virFileOpenAsNoFork(const char *path, int openflags, mode_t mode,
+                    uid_t uid, gid_t gid, unsigned int flags)
+{
     int fd = -1;
     int ret = 0;
     struct stat st;
@@ -1381,7 +1381,7 @@ static int virFileOperationNoFork(const char *path, int openflags, mode_t mode,
                              path, (unsigned int) uid, (unsigned int) gid);
         goto error;
     }
-    if ((flags & VIR_FILE_OP_FORCE_PERMS)
+    if ((flags & VIR_FILE_OPEN_FORCE_PERMS)
         && (fchmod(fd, mode) < 0)) {
         ret = -errno;
         virReportSystemError(errno,
@@ -1389,17 +1389,7 @@ static int virFileOperationNoFork(const char *path, int openflags, mode_t mode,
                              path, mode);
         goto error;
     }
-    if ((hook) && ((ret = hook(fd, hookdata)) != 0)) {
-        goto error;
-    }
-    if (flags & VIR_FILE_OP_RETURN_FD)
-        return fd;
-    if (VIR_CLOSE(fd) < 0) {
-        ret = -errno;
-        virReportSystemError(errno, _("failed to close new file '%s'"),
-                             path);
-        goto error;
-    }
+    return fd;

 error:
     VIR_FORCE_CLOSE(fd);
@@ -1446,35 +1436,27 @@ error:
 }

 /**
- * virFileOperation:
+ * virFileOpenAs:
  * @path: file to open or create
  * @openflags: flags to pass to open
  * @mode: mode to use on creation or when forcing permissions
  * @uid: uid that should own file on creation
  * @gid: gid that should own file
- * @hook: callback to run once file is open; see below for restrictions
- * @hookdata: opaque data for @hook
- * @flags: bit-wise or of VIR_FILE_OP_* flags
+ * @flags: bit-wise or of VIR_FILE_OPEN_* flags
  *
  * Open @path, and execute an optional callback @hook on the open file
  * description.  @hook must return 0 on success, or -errno on failure.
- * If @flags includes VIR_FILE_OP_AS_UID, then open the file while the
+ * If @flags includes VIR_FILE_OPEN_AS_UID, then open the file while the
  * effective user id is @uid (by using a child process); this allows
  * one to bypass root-squashing NFS issues.  If @flags includes
- * VIR_FILE_OP_FORCE_PERMS, then ensure that @path has those
+ * VIR_FILE_OPEN_FORCE_PERMS, then ensure that @path has those
  * permissions before the callback, even if the file already existed
- * with different permissions.  If @flags includes
- * VIR_FILE_OP_RETURN_FD, then use SCM_RIGHTS to guarantee that any
- * @hook is run in the parent, and the return value (if non-negative)
- * is the file descriptor, left open.  Otherwise, @hook might be run
- * in a child process, so it must be async-signal-safe (ie. no
- * malloc() or anything else that depends on modifying locks held in
- * the parent), no file descriptor remains open, and 0 is returned on
- * success.  Return -errno on failure.  */
-int virFileOperation(const char *path, int openflags, mode_t mode,
-                     uid_t uid, gid_t gid,
-                     virFileOperationHook hook, void *hookdata,
-                     unsigned int flags) {
+ * with different permissions.  The return value (if non-negative)
+ * is the file descriptor, left open.  Return -errno on failure.  */
+int
+virFileOpenAs(const char *path, int openflags, mode_t mode,
+              uid_t uid, gid_t gid, unsigned int flags)
+{
     struct stat st;
     pid_t pid;
     int waitret, status, ret = 0;
@@ -1487,11 +1469,10 @@ int virFileOperation(const char *path, int openflags, mode_t mode,
     struct iovec iov;
     int forkRet;

-    if ((!(flags & VIR_FILE_OP_AS_UID))
+    if ((!(flags & VIR_FILE_OPEN_AS_UID))
         || (getuid() != 0)
         || ((uid == 0) && (gid == 0))) {
-        return virFileOperationNoFork(path, openflags, mode, uid, gid,
-                                      hook, hookdata, flags);
+        return virFileOpenAsNoFork(path, openflags, mode, uid, gid, flags);
     }

     /* parent is running as root, but caller requested that the
@@ -1499,7 +1480,7 @@ int virFileOperation(const char *path, int openflags, mode_t mode,
      * following dance avoids problems caused by root-squashing
      * NFS servers. */

-    if (flags & VIR_FILE_OP_RETURN_FD) {
+    {
         if (socketpair(AF_UNIX, SOCK_DGRAM, 0, pair) < 0) {
             ret = -errno;
             virReportSystemError(errno,
@@ -1529,7 +1510,7 @@ int virFileOperation(const char *path, int openflags, mode_t mode,
     }

     if (pid) { /* parent */
-        if (flags & VIR_FILE_OP_RETURN_FD) {
+        {
             VIR_FORCE_CLOSE(pair[1]);

             do {
@@ -1565,20 +1546,13 @@ int virFileOperation(const char *path, int openflags, mode_t mode,
             goto parenterror;
         }
         ret = -WEXITSTATUS(status);
-        if (!WIFEXITED(status) || (ret == -EACCES) ||
-            ((flags & VIR_FILE_OP_RETURN_FD) && fd == -1)) {
+        if (!WIFEXITED(status) || (ret == -EACCES) || fd == -1) {
             /* fall back to the simpler method, which works better in
              * some cases */
-            return virFileOperationNoFork(path, openflags, mode, uid, gid,
-                                          hook, hookdata, flags);
+            return virFileOpenAsNoFork(path, openflags, mode, uid, gid, flags);
         }
-        if (!ret && (flags & VIR_FILE_OP_RETURN_FD)) {
-            if ((hook) && ((ret = hook(fd, hookdata)) != 0)) {
-                VIR_FORCE_CLOSE(fd);
-                goto parenterror;
-            }
+        if (!ret)
             ret = fd;
-        }
 parenterror:
         return ret;
     }
@@ -1630,7 +1604,7 @@ parenterror:
                              path, (unsigned int) uid, (unsigned int) gid);
         goto childerror;
     }
-    if ((flags & VIR_FILE_OP_FORCE_PERMS)
+    if ((flags & VIR_FILE_OPEN_FORCE_PERMS)
         && (fchmod(fd, mode) < 0)) {
         ret = -errno;
         virReportSystemError(errno,
@@ -1638,7 +1612,7 @@ parenterror:
                              path, mode);
         goto childerror;
     }
-    if (flags & VIR_FILE_OP_RETURN_FD) {
+    {
         VIR_FORCE_CLOSE(pair[0]);
         memcpy(CMSG_DATA(cmsg), &fd, sizeof(fd));

@@ -1651,17 +1625,6 @@ parenterror:
             VIR_FORCE_CLOSE(pair[1]);
             goto childerror;
         }
-        ret = 0;
-    } else {
-        if ((hook) && ((ret = hook(fd, hookdata)) != 0))
-            goto childerror;
-        if (VIR_CLOSE(fd) < 0) {
-            ret = -errno;
-            virReportSystemError(errno,
-                                 _("child failed to close new file '%s'"),
-                                 path);
-            goto childerror;
-        }
     }

     ret = 0;
@@ -1783,17 +1746,15 @@ childerror:
 #else /* WIN32 */

 /* return -errno on failure, or 0 on success */
-int virFileOperation(const char *path ATTRIBUTE_UNUSED,
-                     int openflags ATTRIBUTE_UNUSED,
-                     mode_t mode ATTRIBUTE_UNUSED,
-                     uid_t uid ATTRIBUTE_UNUSED,
-                     gid_t gid ATTRIBUTE_UNUSED,
-                     virFileOperationHook hook ATTRIBUTE_UNUSED,
-                     void *hookdata ATTRIBUTE_UNUSED,
-                     unsigned int flags ATTRIBUTE_UNUSED)
+int virFileOpenAs(const char *path ATTRIBUTE_UNUSED,
+                  int openflags ATTRIBUTE_UNUSED,
+                  mode_t mode ATTRIBUTE_UNUSED,
+                  uid_t uid ATTRIBUTE_UNUSED,
+                  gid_t gid ATTRIBUTE_UNUSED,
+                  unsigned int flags ATTRIBUTE_UNUSED)
 {
     virUtilError(VIR_ERR_INTERNAL_ERROR,
-                 "%s", _("virFileOperation is not implemented for WIN32"));
+                 "%s", _("virFileOpenAs is not implemented for WIN32"));

     return -ENOSYS;
 }
diff --git a/src/util/util.h b/src/util/util.h
index 3970d58..cecd348 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -125,16 +125,13 @@ bool virFileIsExecutable(const char *file) ATTRIBUTE_NONNULL(1);
 char *virFileSanitizePath(const char *path);

 enum {
-    VIR_FILE_OP_NONE        = 0,
-    VIR_FILE_OP_AS_UID      = (1 << 0),
-    VIR_FILE_OP_FORCE_PERMS = (1 << 1),
-    VIR_FILE_OP_RETURN_FD   = (1 << 2),
+    VIR_FILE_OPEN_NONE        = 0,
+    VIR_FILE_OPEN_AS_UID      = (1 << 0),
+    VIR_FILE_OPEN_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)
+int virFileOpenAs(const char *path, int openflags, mode_t mode,
+                  uid_t uid, gid_t gid,
+                  unsigned int flags)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;

 enum {
-- 
1.7.4




More information about the libvir-list mailing list