[PATCH v3 4/6] virFileWrapperFd: Switch to new virCommandDoAsyncIO

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

Commit 34e8f63a32f83 introduced support for catching errors from
libvirt iohelper. However, at those times there wasn't such fancy
API as virCommandDoAsyncIO(), so everything has to be implemented
on our own. But since we do have the API now, we can use it and
drop our implementation then.
---
 src/util/virfile.c | 82 +++---------------------------------------------------
 1 file changed, 4 insertions(+), 78 deletions(-)

diff --git a/src/util/virfile.c b/src/util/virfile.c
index 5cca54d..b4765fb 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -135,58 +135,11 @@ virFileDirectFdFlag(void)
  * read-write is not supported, just a single direction.  */
 struct _virFileWrapperFd {
     virCommandPtr cmd; /* Child iohelper process to do the I/O.  */
-    int err_fd; /* FD to read stderr of @cmd */
     char *err_msg; /* stderr of @cmd */
-    size_t err_msg_len; /* strlen of err_msg so we don't
-                           have to compute it every time */
-    int err_watch; /* ID of watch in the event loop */
 };
 
 #ifndef WIN32
 /**
- * virFileWrapperFdReadStdErr:
- * @watch: watch ID
- * @fd: the read end of pipe to iohelper's stderr
- * @events: an OR-ed set of events which occurred on @fd
- * @opaque: virFileWrapperFdPtr
- *
- * This is a callback to our eventloop which will read iohelper's
- * stderr, reallocate @opaque->err_msg and copy data.
- */
-static void
-virFileWrapperFdReadStdErr(int watch ATTRIBUTE_UNUSED,
-                           int fd, int events, void *opaque)
-{
-    virFileWrapperFdPtr wfd = (virFileWrapperFdPtr) opaque;
-    char ebuf[1024];
-    ssize_t nread;
-
-    if (events & VIR_EVENT_HANDLE_READABLE) {
-        while ((nread = saferead(fd, ebuf, sizeof(ebuf)))) {
-            if (nread < 0) {
-                if (errno != EAGAIN)
-                    virReportSystemError(errno, "%s",
-                                         _("unable to read iohelper's stderr"));
-                break;
-            }
-
-            if (VIR_REALLOC_N(wfd->err_msg, wfd->err_msg_len + nread + 1) < 0) {
-                virReportOOMError();
-                return;
-            }
-            memcpy(wfd->err_msg + wfd->err_msg_len, ebuf, nread);
-            wfd->err_msg_len += nread;
-            wfd->err_msg[wfd->err_msg_len] = '\0';
-        }
-    }
-
-    if (events & VIR_EVENT_HANDLE_HANGUP) {
-        virEventRemoveHandle(watch);
-        wfd->err_watch = -1;
-    }
-}
-
-/**
  * virFileWrapperFdNew:
  * @fd: pointer to fd to wrap
  * @name: name of fd, for diagnostics
@@ -245,8 +198,6 @@ virFileWrapperFdNew(int *fd, const char *name, unsigned int flags)
         return NULL;
     }
 
-    ret->err_watch = -1;
-
     mode = fcntl(*fd, F_GETFL);
 
     if (mode < 0) {
@@ -279,38 +230,16 @@ virFileWrapperFdNew(int *fd, const char *name, unsigned int flags)
         virCommandAddArg(ret->cmd, "0");
     }
 
-    /* In order to catch iohelper stderr, we must:
-     * - pass a FD to virCommand (-1 to auto-allocate one)
-     * - change iohelper's env so virLog functions print to stderr
+    /* In order to catch iohelper stderr, we must change
+     * iohelper's env so virLog functions print to stderr
      */
-    ret->err_fd = -1;
-    virCommandSetErrorFD(ret->cmd, &ret->err_fd);
     virCommandAddEnvPair(ret->cmd, "LIBVIRT_LOG_OUTPUTS", "1:stderr");
+    virCommandSetErrorBuffer(ret->cmd, &ret->err_msg);
+    virCommandDoAsyncIO(ret->cmd);
 
     if (virCommandRunAsync(ret->cmd, NULL) < 0)
         goto error;
 
-    /* deliberately don't use virCommandNonblockingFDs here as it is all or
-     * nothing. And we want iohelper's stdin and stdout to block (default).
-     * However, stderr is read within event loop and therefore it must be
-     * nonblocking.*/
-    if (virSetNonBlock(ret->err_fd) < 0) {
-        virReportSystemError(errno, "%s",
-                             _("Failed to set non-blocking "
-                               "file descriptor flag"));
-        goto error;
-    }
-
-    if ((ret->err_watch = virEventAddHandle(ret->err_fd,
-                                            VIR_EVENT_HANDLE_READABLE,
-                                            virFileWrapperFdReadStdErr,
-                                            ret, NULL)) < 0) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("unable to register iohelper's "
-                         "stderr FD in the eventloop"));
-        goto error;
-    }
-
     if (VIR_CLOSE(pipefd[!output]) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("unable to close pipe"));
         goto error;
@@ -389,9 +318,6 @@ virFileWrapperFdFree(virFileWrapperFdPtr wfd)
     if (!wfd)
         return;
 
-    VIR_FORCE_CLOSE(wfd->err_fd);
-    if (wfd->err_watch != -1)
-        virEventRemoveHandle(wfd->err_watch);
     VIR_FREE(wfd->err_msg);
 
     virCommandFree(wfd->cmd);
-- 
1.8.0.2

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list