[libvirt] [PATCH 1/3] avoid closing fd more than once in virFDStreamOpenFileInternal()
When we assign fd and childfd to elements of fds[], we will close these fds more than once on error. So we should assign fds[] to -1 after we assign fd and childfd to elements of fds[]. We should also close childfd on error. --- src/fdstream.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/src/fdstream.c b/src/fdstream.c index fca0f41..0de3b04 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, @@ -650,6 +650,7 @@ virFDStreamOpenFileInternal(virStreamPtr st, fd = fds[1]; virCommandSetInputFD(cmd, childfd); } + fds[0] = fds[1] = -1; virCommandSetErrorFD(cmd, &errfd); if (virCommandRunAsync(cmd, NULL) < 0) @@ -668,6 +669,7 @@ error: VIR_FORCE_CLOSE(fds[0]); VIR_FORCE_CLOSE(fds[1]); VIR_FORCE_CLOSE(fd); + VIR_FORCE_CLOSE(childfd); VIR_FORCE_CLOSE(errfd); if (oflags & O_CREAT) unlink(path); -- 1.7.1
If we migrate to fd, spec->fwdType is not MIGRATION_FWD_DIRECT, and we will close spec->dest.fd.local in qemuMigrationRun(). So we should set spec->dest.fd.local to -1 in qemuMigrationRun() because the caller doTunnelMigrate() will close it too. --- src/qemu/qemu_migration.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) 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); -- 1.7.1
We should not set *outfd or *errfd if virExecWithHook() failed, because the caller may close these fds. --- src/util/command.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/util/command.c b/src/util/command.c index 5b94f1e..aef9131 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
participants (1)
-
Wen Congyang