[PATCH 2/3] avoid closing fd more than once

[Wen Congyang <wency@xxxxxxxxxxxxxx>]

fdstream:
    If fd is fds[0] or fds[1], we should set to -1 if we meet
    some error.

    childfd is fds[0] or fds[1], so we should close it only when
    virFDStreamOpenFileInternal() successes.

qemu_migration:
    If we migrate to fd, spec->fwdType is not MIGRATION_FWD_DIRECT,
    we will close spec->dest.fd.local in qemuMigrationRun(). So we
    should set spec->dest.fd.local to -1 in qemuMigrationRun(). 

command:
    we should not set *outfd or *errfd if virExecWithHook() failed
    because the caller may close these fds.

---
 src/fdstream.c            |   15 ++++++++++-----
 src/qemu/qemu_migration.c |    4 +++-
 src/util/command.c        |    8 ++++----
 3 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/src/fdstream.c b/src/fdstream.c
index 32d386d..d0ea0ee 100644
--- a/src/fdstream.c
+++ b/src/fdstream.c
@@ -581,6 +581,7 @@ virFDStreamOpenFileInternal(virStreamPtr st,
     struct stat sb;
     virCommandPtr cmd = NULL;
     int errfd = -1;
+    int childfd = -1;
 
     VIR_DEBUG("st=%p path=%s oflags=%x offset=%llu length=%llu mode=%o",
               st, path, oflags, offset, length, mode);
@@ -619,7 +620,6 @@ virFDStreamOpenFileInternal(virStreamPtr st,
     if ((st->flags & VIR_STREAM_NONBLOCK) &&
         (!S_ISCHR(sb.st_mode) &&
          !S_ISFIFO(sb.st_mode))) {
-        int childfd;
 
         if ((oflags & O_ACCMODE) == O_RDWR) {
             streamsReportError(VIR_ERR_INTERNAL_ERROR,
@@ -652,15 +652,20 @@ virFDStreamOpenFileInternal(virStreamPtr st,
         }
         virCommandSetErrorFD(cmd, &errfd);
 
-        if (virCommandRunAsync(cmd, NULL) < 0)
+        if (virCommandRunAsync(cmd, NULL) < 0) {
+            /* donot close fd twice if we meet some error */
+            fd = -1;
             goto error;
-
-        VIR_FORCE_CLOSE(childfd);
+        }
     }
 
-    if (virFDStreamOpenInternal(st, fd, cmd, errfd, length) < 0)
+    if (virFDStreamOpenInternal(st, fd, cmd, errfd, length) < 0) {
+        /* donot close fd twice if we meet some error */
+        fd = -1;
         goto error;
+    }
 
+    VIR_FORCE_CLOSE(childfd);
     return 0;
 
 error:
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 6f42823..b58380b 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1910,8 +1910,10 @@ qemuMigrationRun(struct qemud_driver *driver,
         break;
 
     case MIGRATION_DEST_FD:
-        if (spec->fwdType != MIGRATION_FWD_DIRECT)
+        if (spec->fwdType != MIGRATION_FWD_DIRECT) {
             fd = spec->dest.fd.local;
+            spec->dest.fd.local = -1;
+        }
         ret = qemuMonitorMigrateToFd(priv->mon, migrate_flags,
                                      spec->dest.fd.qemu);
         VIR_FORCE_CLOSE(spec->dest.fd.qemu);
diff --git a/src/util/command.c b/src/util/command.c
index eaa9f16..2723fde 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -493,6 +493,10 @@ virExecWithHook(const char *const*argv,
     }
 
     if (pid) { /* parent */
+        if (forkRet < 0) {
+            goto cleanup;
+        }
+
         VIR_FORCE_CLOSE(null);
         if (outfd && *outfd == -1) {
             VIR_FORCE_CLOSE(pipeout[1]);
@@ -503,10 +507,6 @@ virExecWithHook(const char *const*argv,
             *errfd = pipeerr[0];
         }
 
-        if (forkRet < 0) {
-            goto cleanup;
-        }
-
         *retpid = pid;
 
         if (binary != argv[0])
-- 
1.7.1

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