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.
---
v5: rebase to changes in patch 4/13
src/libvirt_private.syms | 2 +-
src/qemu/qemu_driver.c | 20 +++-----
src/storage/storage_backend.c | 12 ++--
src/util/util.c | 105 +++++++++++++---------------------------
src/util/util.h | 15 ++----
5 files changed, 55 insertions(+), 99 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 93504e5..fb95858 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -898,8 +898,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 7a6e4ff..8ee2a29 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1913,11 +1913,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
@@ -1951,7 +1949,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);
@@ -1962,12 +1960,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 b0a512c..7015475 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -390,14 +390,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 a0a331d..49a8bb6 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -1391,10 +1391,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;
@@ -1417,7 +1417,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,
@@ -1425,17 +1425,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);
@@ -1482,35 +1472,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;
@@ -1523,11 +1505,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
@@ -1535,7 +1516,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_STREAM, 0, pair) < 0) {
ret = -errno;
virReportSystemError(errno,
@@ -1565,7 +1546,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 {
@@ -1601,19 +1582,13 @@ int virFileOperation(const char *path, int openflags, mode_t mode,
goto parenterror;
}
if (!WIFEXITED(status) || (ret = -WEXITSTATUS(status)) == -EACCES ||
- ((flags & VIR_FILE_OP_RETURN_FD) && fd == -1)) {
+ 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;
}
@@ -1626,6 +1601,7 @@ parenterror:
ret = -errno;
goto childerror;
}
+ VIR_FORCE_CLOSE(pair[0]);
/* set desired uid/gid, then attempt to create the file */
@@ -1665,7 +1641,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,
@@ -1673,8 +1649,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));
do {
@@ -1685,17 +1660,6 @@ parenterror:
ret = -errno;
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;
@@ -1704,6 +1668,7 @@ childerror:
* If the child exits with EACCES, then the parent tries again. */
/* XXX This makes assumptions about errno being < 255, which is
* not true on Hurd. */
+ VIR_FORCE_CLOSE(pair[1]);
ret = -ret;
if ((ret & 0xff) != ret) {
VIR_WARN("unable to pass desired return value %d", ret);
@@ -1813,17 +1778,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 b1ca871..7b7722a 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -127,16 +127,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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list