libxl: fix leaking logfile fds

[libvirt] [PATCH] libxl: fix leaking logfile fds

Posted by Jim Fehlig 5 years, 10 months ago

Per-domain log files were introduced in commit a30b08b7179. The FILE
objects associated with these log files are stored in a hash table
using domid as a key. When a domain is shutdown, destroyed, or
otherwise powered-off, the FILE object is removed from the hash table,
where the free function will close the FILE.

Unfortunately the call to remove the FILE from the hash table occurs
after setting domid=-1 in the libxlDomainCleanup() function. The
object is never removed from the hash table, the free function is
never called, and the underlying fd is leaked. Fix by removing the
FILE object from the hash table before setting domid=-1.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
 src/libxl/libxl_domain.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index d4859d6707..d12b1b1b4b 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -769,6 +769,7 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver,
         VIR_WARN("Unable to release lease on %s", vm->def->name);
     VIR_DEBUG("Preserving lock state '%s'", NULLSTR(priv->lockState));
 
+    libxlLoggerCloseFile(cfg->logger, vm->def->id);
     vm->def->id = -1;
 
     if (priv->deathW) {
@@ -822,8 +823,6 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver,
         VIR_FREE(xml);
     }
 
-    libxlLoggerCloseFile(cfg->logger, vm->def->id);
-
     virDomainObjRemoveTransientDef(vm);
     virObjectUnref(cfg);
 }
-- 
2.16.3

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

Re: [libvirt] [PATCH] libxl: fix leaking logfile fds

Posted by Ján Tomko 5 years, 10 months ago

On Thu, May 31, 2018 at 03:57:20PM -0600, Jim Fehlig wrote:
>Per-domain log files were introduced in commit a30b08b7179. The FILE
>objects associated with these log files are stored in a hash table
>using domid as a key. When a domain is shutdown, destroyed, or
>otherwise powered-off, the FILE object is removed from the hash table,
>where the free function will close the FILE.
>
>Unfortunately the call to remove the FILE from the hash table occurs
>after setting domid=-1 in the libxlDomainCleanup() function. The
>object is never removed from the hash table, the free function is
>never called, and the underlying fd is leaked. Fix by removing the
>FILE object from the hash table before setting domid=-1.
>
>Signed-off-by: Jim Fehlig <jfehlig@suse.com>
>---
> src/libxl/libxl_domain.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

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