6:37 p.m.
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.
---
v3: new patch
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 56b5bdf..4cc5705 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -874,8 +874,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 42d2125..a222309 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1881,11 +1881,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
@@ -1919,7 +1917,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);
@@ -1930,12 +1928,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 02f8b5b..5e8d94a 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -363,14 +363,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;
- flags = VIR_FILE_OP_FORCE_PERMS | VIR_FILE_OP_RETURN_FD;
+ flags = VIR_FILE_OPEN_FORCE_PERMS;
if (pool->def->type == VIR_STORAGE_POOL_NETFS)
- flags |= VIR_FILE_OP_AS_UID;
+ 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, flags)) < 0) {
+ if ((fd = virFileOpenAs(vol->target.path,
+ O_RDWR | O_CREAT | O_EXCL,
+ vol->target.perms.mode, uid, gid,
+ 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