virFileDirectFd was used for accessing files opened with O_DIRECT using libvirt_iohelper. We will want to use the helper for accessing files regardless on O_DIRECT and thus virFileDirectFd was generalized and renamed to virFileWrapperFd. --- src/qemu/qemu_driver.c | 42 +++++++++++--------- src/util/virfile.c | 98 ++++++++++++++++++++++++++--------------------- src/util/virfile.h | 18 +++++---- 3 files changed, 87 insertions(+), 71 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3f940e8..01dbcd7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2521,7 +2521,7 @@ qemuDomainSaveInternal(struct qemud_driver *driver, virDomainPtr dom, unsigned long long pad; int fd = -1; int directFlag = 0; - virFileDirectFdPtr directFd = NULL; + virFileWrapperFdPtr wrapperFd = NULL; bool bypass_cache = flags & VIR_DOMAIN_SAVE_BYPASS_CACHE; if (qemuProcessAutoDestroyActive(driver, vm)) { @@ -2625,7 +2625,8 @@ qemuDomainSaveInternal(struct qemud_driver *driver, virDomainPtr dom, &needUnlink, &bypassSecurityDriver); if (fd < 0) goto endjob; - if (bypass_cache && (directFd = virFileDirectFdNew(&fd, path)) == NULL) + if (bypass_cache && + (wrapperFd = virFileWrapperFdNew(&fd, true, path)) == NULL) goto endjob; /* Write header to file, followed by XML */ @@ -2644,14 +2645,14 @@ qemuDomainSaveInternal(struct qemud_driver *driver, virDomainPtr dom, /* Touch up file header to mark image complete. */ if (bypass_cache) { /* Reopen the file to touch up the header, since we aren't set - * up to seek backwards on directFd. The reopened fd will + * up to seek backwards on wrapperFd. The reopened fd will * trigger a single page of file system cache pollution, but * that's acceptable. */ if (VIR_CLOSE(fd) < 0) { virReportSystemError(errno, _("unable to close %s"), path); goto endjob; } - if (virFileDirectFdClose(directFd) < 0) + if (virFileWrapperFdClose(wrapperFd) < 0) goto endjob; fd = qemuOpenFile(driver, path, O_WRONLY, NULL, NULL); if (fd < 0) @@ -2703,7 +2704,7 @@ endjob: cleanup: VIR_FORCE_CLOSE(fd); - virFileDirectFdFree(directFd); + virFileWrapperFdFree(wrapperFd); VIR_FREE(xml); if (ret != 0 && needUnlink) unlink(path); @@ -2939,7 +2940,7 @@ doCoreDump(struct qemud_driver *driver, { int fd = -1; int ret = -1; - virFileDirectFdPtr directFd = NULL; + virFileWrapperFdPtr wrapperFd = NULL; int directFlag = 0; /* Create an empty file with appropriate ownership. */ @@ -2959,7 +2960,8 @@ doCoreDump(struct qemud_driver *driver, NULL, NULL)) < 0) goto cleanup; - if (bypass_cache && (directFd = virFileDirectFdNew(&fd, path)) == NULL) + if (bypass_cache && + (wrapperFd = virFileWrapperFdNew(&fd, true, path)) == NULL) goto cleanup; if (qemuMigrationToFile(driver, vm, fd, 0, path, @@ -2973,14 +2975,14 @@ doCoreDump(struct qemud_driver *driver, path); goto cleanup; } - if (virFileDirectFdClose(directFd) < 0) + if (virFileWrapperFdClose(wrapperFd) < 0) goto cleanup; ret = 0; cleanup: VIR_FORCE_CLOSE(fd); - virFileDirectFdFree(directFd); + virFileWrapperFdFree(wrapperFd); if (ret != 0) unlink(path); return ret; @@ -3926,7 +3928,8 @@ qemuDomainSaveImageOpen(struct qemud_driver *driver, const char *path, virDomainDefPtr *ret_def, struct qemud_save_header *ret_header, - bool bypass_cache, virFileDirectFdPtr *directFd, + bool bypass_cache, + virFileWrapperFdPtr *wrapperFd, const char *xmlin, int state, bool edit, bool unlink_corrupt) { @@ -3948,7 +3951,8 @@ qemuDomainSaveImageOpen(struct qemud_driver *driver, if ((fd = qemuOpenFile(driver, path, oflags, NULL, NULL)) < 0) goto error; - if (bypass_cache && (*directFd = virFileDirectFdNew(&fd, path)) == NULL) + if (bypass_cache && + (*wrapperFd = virFileWrapperFdNew(&fd, true, path)) == NULL) goto error; if (saferead(fd, &header, sizeof(header)) != sizeof(header)) { @@ -4187,7 +4191,7 @@ qemuDomainRestoreFlags(virConnectPtr conn, int fd = -1; int ret = -1; struct qemud_save_header header; - virFileDirectFdPtr directFd = NULL; + virFileWrapperFdPtr wrapperFd = NULL; int state = -1; virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE | @@ -4203,7 +4207,7 @@ qemuDomainRestoreFlags(virConnectPtr conn, fd = qemuDomainSaveImageOpen(driver, path, &def, &header, (flags & VIR_DOMAIN_SAVE_BYPASS_CACHE) != 0, - &directFd, dxml, state, false, false); + &wrapperFd, dxml, state, false, false); if (fd < 0) goto cleanup; @@ -4223,7 +4227,7 @@ qemuDomainRestoreFlags(virConnectPtr conn, ret = qemuDomainSaveImageStartVM(conn, driver, vm, &fd, &header, path, false); - if (virFileDirectFdClose(directFd) < 0) + if (virFileWrapperFdClose(wrapperFd) < 0) VIR_WARN("Failed to close %s", path); if (qemuDomainObjEndJob(driver, vm) == 0) @@ -4236,7 +4240,7 @@ qemuDomainRestoreFlags(virConnectPtr conn, cleanup: virDomainDefFree(def); VIR_FORCE_CLOSE(fd); - virFileDirectFdFree(directFd); + virFileWrapperFdFree(wrapperFd); if (vm) virDomainObjUnlock(vm); qemuDriverUnlock(driver); @@ -4364,10 +4368,10 @@ qemuDomainObjRestore(virConnectPtr conn, int fd = -1; int ret = -1; struct qemud_save_header header; - virFileDirectFdPtr directFd = NULL; + virFileWrapperFdPtr wrapperFd = NULL; fd = qemuDomainSaveImageOpen(driver, path, &def, &header, - bypass_cache, &directFd, NULL, -1, false, + bypass_cache, &wrapperFd, NULL, -1, false, true); if (fd < 0) { if (fd == -3) @@ -4394,13 +4398,13 @@ qemuDomainObjRestore(virConnectPtr conn, ret = qemuDomainSaveImageStartVM(conn, driver, vm, &fd, &header, path, start_paused); - if (virFileDirectFdClose(directFd) < 0) + if (virFileWrapperFdClose(wrapperFd) < 0) VIR_WARN("Failed to close %s", path); cleanup: virDomainDefFree(def); VIR_FORCE_CLOSE(fd); - virFileDirectFdFree(directFd); + virFileWrapperFdFree(wrapperFd); return ret; } diff --git a/src/util/virfile.c b/src/util/virfile.c index e6b469c..218feab 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -94,12 +94,6 @@ FILE *virFileFdopen(int *fdptr, const char *mode) } -/* Opaque type for managing a wrapper around an O_DIRECT fd. For now, - * read-write is not supported, just a single direction. */ -struct _virFileDirectFd { - virCommandPtr cmd; /* Child iohelper process to do the I/O. */ -}; - /** * virFileDirectFdFlag: * @@ -116,36 +110,46 @@ virFileDirectFdFlag(void) return O_DIRECT ? O_DIRECT : -1; } +/* Opaque type for managing a wrapper around a fd. For now, + * read-write is not supported, just a single direction. */ +struct _virFileWrapperFd { + virCommandPtr cmd; /* Child iohelper process to do the I/O. */ +}; + +#ifndef WIN32 /** - * virFileDirectFdNew: + * virFileWrapperFdNew: * @fd: pointer to fd to wrap + * @bypassCache: bypass system cache * @name: name of fd, for diagnostics * - * Update *FD (created with virFileDirectFdFlag() among the flags to - * open()) to ensure that all I/O to that file will bypass the system - * cache. This must be called after open() and optional fchown() or - * fchmod(), but before any seek or I/O, and only on seekable fd. The - * file must be O_RDONLY (to read the entire existing file) or - * O_WRONLY (to write to an empty file). In some cases, *FD is - * changed to a non-seekable pipe; in this case, the caller must not + * Update @fd (possibly created with virFileDirectFdFlag() among the flags + * to open()) to ensure it properly supports non-blocking I/O, i.e., it will + * report EAGAIN. Iff @direct is true (in which case fd must have been + * created with virFileDirectFdFlag()), it will also be ensured that all + * I/O to that file will bypass the system cache. This must be called after + * open() and optional fchown() or fchmod(), but before any seek or I/O, and + * only on seekable fd. The file must be O_RDONLY (to read the entire + * existing file) or O_WRONLY (to write to an empty file). In some cases, + * @fd is changed to a non-seekable pipe; in this case, the caller must not * do anything further with the original fd. * * On success, the new wrapper object is returned, which must be later - * freed with virFileDirectFdFree(). On failure, *FD is unchanged, an + * freed with virFileWrapperFdFree(). On failure, *FD is unchanged, an * error message is output, and NULL is returned. */ -virFileDirectFdPtr -virFileDirectFdNew(int *fd, const char *name) +virFileWrapperFdPtr +virFileWrapperFdNew(int *fd, bool bypassCache, const char *name) { - virFileDirectFdPtr ret = NULL; + virFileWrapperFdPtr ret = NULL; bool output = false; int pipefd[2] = { -1, -1 }; int mode = -1; - /* XXX support posix_fadvise rather than spawning a child, if the + /* XXX support posix_fadvise rather than O_DIRECT, if the * kernel support for that is decent enough. */ - if (!O_DIRECT) { + if (bypassCache && !O_DIRECT) { virFileError(VIR_ERR_INTERNAL_ERROR, "%s", _("O_DIRECT unsupported on this platform")); return NULL; @@ -156,12 +160,7 @@ virFileDirectFdNew(int *fd, const char *name) return NULL; } -#ifdef F_GETFL - /* Mingw lacks F_GETFL, but it also lacks O_DIRECT so didn't get - * here in the first place. All other platforms reach this - * line. */ mode = fcntl(*fd, F_GETFL); -#endif if (mode < 0) { virFileError(VIR_ERR_INTERNAL_ERROR, _("invalid fd %d for %s"), @@ -208,48 +207,59 @@ virFileDirectFdNew(int *fd, const char *name) error: VIR_FORCE_CLOSE(pipefd[0]); VIR_FORCE_CLOSE(pipefd[1]); - virFileDirectFdFree(ret); + virFileWrapperFdFree(ret); return NULL; } +#else +virFileWrapperFdPtr +virFileWrapperFdNew(int *fd ATTRIBUTE_UNUSED, + bool bypassCache ATTRIBUTE_UNUSED, + const char *name ATTRIBUTE_UNUSED) +{ + virFileError(VIR_ERR_INTERNAL_ERROR, "%s", + _("virFileWrapperFd unsupported on this platform")); + return NULL; +} +#endif /** - * virFileDirectFdClose: - * @dfd: direct fd wrapper, or NULL + * virFileWrapperFdClose: + * @wfd: fd wrapper, or NULL * - * If DFD is valid, then ensure that I/O has completed, which may + * If @wfd is valid, then ensure that I/O has completed, which may * include reaping a child process. Return 0 if all data for the * wrapped fd is complete, or -1 on failure with an error emitted. - * This function intentionally returns 0 when DFD is NULL, so that - * callers can conditionally create a virFileDirectFd wrapper but + * This function intentionally returns 0 when @wfd is NULL, so that + * callers can conditionally create a virFileWrapperFd wrapper but * unconditionally call the cleanup code. To avoid deadlock, only - * call this after closing the fd resulting from virFileDirectFdNew(). + * call this after closing the fd resulting from virFileWrapperFdNew(). */ int -virFileDirectFdClose(virFileDirectFdPtr dfd) +virFileWrapperFdClose(virFileWrapperFdPtr wfd) { - if (!dfd) + if (!wfd) return 0; - return virCommandWait(dfd->cmd, NULL); + return virCommandWait(wfd->cmd, NULL); } /** - * virFileDirectFdFree: - * @dfd: direct fd wrapper, or NULL + * virFileWrapperFdFree: + * @wfd: fd wrapper, or NULL * - * Free all remaining resources associated with DFD. If - * virFileDirectFdClose() was not previously called, then this may + * Free all remaining resources associated with @wfd. If + * virFileWrapperFdClose() was not previously called, then this may * discard some previous I/O. To avoid deadlock, only call this after - * closing the fd resulting from virFileDirectFdNew(). + * closing the fd resulting from virFileWrapperFdNew(). */ void -virFileDirectFdFree(virFileDirectFdPtr dfd) +virFileWrapperFdFree(virFileWrapperFdPtr wfd) { - if (!dfd) + if (!wfd) return; - virCommandFree(dfd->cmd); - VIR_FREE(dfd); + virCommandFree(wfd->cmd); + VIR_FREE(wfd); } diff --git a/src/util/virfile.h b/src/util/virfile.h index 7be15b5..a412f25 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -50,20 +50,22 @@ FILE *virFileFdopen(int *fdptr, const char *mode) ATTRIBUTE_RETURN_CHECK; # define VIR_FORCE_CLOSE(FD) ignore_value(virFileClose(&(FD), true)) # define VIR_FORCE_FCLOSE(FILE) ignore_value(virFileFclose(&(FILE), true)) -/* Opaque type for managing a wrapper around an O_DIRECT fd. */ -struct _virFileDirectFd; +/* Opaque type for managing a wrapper around a fd. */ +struct _virFileWrapperFd; -typedef struct _virFileDirectFd virFileDirectFd; -typedef virFileDirectFd *virFileDirectFdPtr; +typedef struct _virFileWrapperFd virFileWrapperFd; +typedef virFileWrapperFd *virFileWrapperFdPtr; int virFileDirectFdFlag(void); -virFileDirectFdPtr virFileDirectFdNew(int *fd, const char *name) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +virFileWrapperFdPtr virFileWrapperFdNew(int *fd, + bool bypassCache, + const char *name) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK; -int virFileDirectFdClose(virFileDirectFdPtr dfd); +int virFileWrapperFdClose(virFileWrapperFdPtr dfd); -void virFileDirectFdFree(virFileDirectFdPtr dfd); +void virFileWrapperFdFree(virFileWrapperFdPtr dfd); int virFileLock(int fd, bool shared, off_t start, off_t len); int virFileUnlock(int fd, off_t start, off_t len); -- 1.7.8.4 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list