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

Jiri Denemark posted 1 patch 6 years, 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/c40e470e3e6715ba3e12016e1a1fa456e538746f.1497977320.git.jdenemar@redhat.com
src/qemu/qemu_migration.c | 1 -
src/qemu/qemu_process.c   | 3 +++
2 files changed, 3 insertions(+), 1 deletion(-)
[libvirt] [PATCH] qemu: Avoid fd leak on incoming tunneled migration
Posted by Jiri Denemark 6 years, 10 months ago
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Avoid fd leak on incoming tunneled migration
Posted by John Ferlan 6 years, 9 months ago

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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Avoid fd leak on incoming tunneled migration
Posted by Jiri Denemark 6 years, 9 months ago
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

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