util: Generalize virFileDirectFd

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

@@ -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,9 @@ 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, path,
+                                          VIR_FILE_WRAPPER_BYPASS_CACHE)))
         goto endjob;
 
     /* Write header to file, followed by XML */
@@ -2644,14 +2646,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 +2705,7 @@ endjob:
 
 cleanup:
     VIR_FORCE_CLOSE(fd);
-    virFileDirectFdFree(directFd);
+    virFileWrapperFdFree(wrapperFd);
     VIR_FREE(xml);
     if (ret != 0 && needUnlink)
         unlink(path);
@@ -2939,7 +2941,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 +2961,9 @@ doCoreDump(struct qemud_driver *driver,
                            NULL, NULL)) < 0)
         goto cleanup;
 
-    if (bypass_cache && (directFd = virFileDirectFdNew(&fd, path)) == NULL)
+    if (bypass_cache &&
+        !(wrapperFd = virFileWrapperFdNew(&fd, path,
+                                          VIR_FILE_WRAPPER_BYPASS_CACHE)))
         goto cleanup;
 
     if (qemuMigrationToFile(driver, vm, fd, 0, path,
@@ -2973,14 +2977,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 +3930,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 +3953,9 @@ 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, path,
+                                           VIR_FILE_WRAPPER_BYPASS_CACHE)))
         goto error;
 
     if (saferead(fd, &header, sizeof(header)) != sizeof(header)) {
@@ -4187,7 +4194,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 +4210,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 +4230,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 +4243,7 @@ qemuDomainRestoreFlags(virConnectPtr conn,
 cleanup:
     virDomainDefFree(def);
     VIR_FORCE_CLOSE(fd);
-    virFileDirectFdFree(directFd);
+    virFileWrapperFdFree(wrapperFd);
     if (vm)
         virDomainObjUnlock(vm);
     qemuDriverUnlock(driver);
@@ -4364,10 +4371,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 +4401,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;
 }
 

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,62 @@ 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
  * @name: name of fd, for diagnostics
+ * @flags: bitwise-OR of virFileWrapperFdFlags
+ *
+ * Update @fd so that it meets parameters requested by @flags.
+ *
+ * If VIR_FILE_WRAPPER_BYPASS_CACHE bit is set in @flags, @fd will be updated
+ * in a way that all I/O to that file will bypass the system cache.  The
+ * original fd must have been created with virFileDirectFdFlag() among the
+ * flags to open().
  *
- * 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
- * do anything further with the original fd.
+ * If VIR_FILE_WRAPPER_NON_BLOCKING bit is set in @flags, @fd will be updated
+ * to ensure it properly supports non-blocking I/O, i.e., it will report
+ * EAGAIN.
+ *
+ * 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, const char *name, unsigned int flags)
 {
-    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
-     * kernel support for that is decent enough.  */
+    if (!flags) {
+        virFileError(VIR_ERR_INTERNAL_ERROR, "%s",
+                     _("invalid use with no flags"));
+        return NULL;
+    }
+
+    /* XXX support posix_fadvise rather than O_DIRECT, if the kernel support
+     * for that is decent enough. In that case, we will also need to
+     * explicitly support VIR_FILE_WRAPPER_NON_BLOCKING since
+     * VIR_FILE_WRAPPER_BYPASS_CACHE alone will no longer require spawning
+     * iohelper.
+     */
 
-    if (!O_DIRECT) {
+    if ((flags & VIR_FILE_WRAPPER_BYPASS_CACHE) && !O_DIRECT) {
         virFileError(VIR_ERR_INTERNAL_ERROR, "%s",
                      _("O_DIRECT unsupported on this platform"));
         return NULL;
@@ -156,12 +176,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 +223,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,
+                    const char *name ATTRIBUTE_UNUSED,
+                    unsigned int fdflags 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);
 }
 
 

src/util/virfile.h

@@ -50,20 +50,27 @@ 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)
+enum {
+    VIR_FILE_WRAPPER_BYPASS_CACHE   = (1 << 0),
+    VIR_FILE_WRAPPER_NON_BLOCKING   = (1 << 1),
+} virFileWrapperFdFlags;
+
+virFileWrapperFdPtr virFileWrapperFdNew(int *fd,
+                                        const char *name,
+                                        unsigned int flags)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) 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);