[libvirt] [PATCH] qemu: Avoid fd leak on incoming tunneled migration

While qemuProcessIncomingDefNew takes an fd argument and stores it in qemuProcessIncomingDef structure, the caller is still responsible for closing the file descriptor. Introduced by commit v1.2.21-140-ge7c6f4575. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 1 - src/qemu/qemu_process.c | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 8129dcd40..c23fffef2 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2690,7 +2690,6 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, listenAddress, port, dataFD[0]))) goto stopjob; - dataFD[0] = -1; /* the FD is now owned by incoming */ if (qemuProcessPrepareDomain(dconn, driver, vm, VIR_QEMU_PROCESS_START_AUTODESTROY) < 0) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index fa9990e5d..443ec88fe 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4169,6 +4169,9 @@ qemuProcessIncomingDefFree(qemuProcessIncomingDefPtr inc) * This function does not copy @path, the caller is responsible for keeping * the @path pointer valid during the lifetime of the allocated * qemuProcessIncomingDef structure. + * + * The caller is responsible for closing @fd, calling + * qemuProcessIncomingDefFree will NOT close it. */ qemuProcessIncomingDefPtr qemuProcessIncomingDefNew(virQEMUCapsPtr qemuCaps, -- 2.13.1

On 06/20/2017 12:48 PM, Jiri Denemark wrote:
While qemuProcessIncomingDefNew takes an fd argument and stores it in qemuProcessIncomingDef structure, the caller is still responsible for closing the file descriptor.
Introduced by commit v1.2.21-140-ge7c6f4575.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 1 - src/qemu/qemu_process.c | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-)
Suppose you could have gone the other way too - adding the close in DefFree... I'm fine with this way though. Reviewed-by: John Ferlan <jferlan@redhat.com> John

On Sun, Jun 25, 2017 at 09:57:43 -0400, John Ferlan wrote:
On 06/20/2017 12:48 PM, Jiri Denemark wrote:
While qemuProcessIncomingDefNew takes an fd argument and stores it in qemuProcessIncomingDef structure, the caller is still responsible for closing the file descriptor.
Introduced by commit v1.2.21-140-ge7c6f4575.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 1 - src/qemu/qemu_process.c | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-)
Suppose you could have gone the other way too - adding the close in DefFree... I'm fine with this way though.
I tried. It turned out to be quite ugly because there is a patch in which we don't want DefFree to close the fd and we'd need to hack around it there.
Reviewed-by: John Ferlan <jferlan@redhat.com>
Thanks, pushed. Jirka
participants (2)
-
Jiri Denemark
-
John Ferlan