On Wed, Mar 17, 2010 at 10:35:51PM +0100, Matthias Bolte wrote: > The logfile FD is dup2'ed in __virExec in the child. The FD needs to > be closed in the parent, otherwise it leaks. > --- >  src/qemu/qemu_driver.c |    3 +++ >  1 files changed, 3 insertions(+), 0 deletions(-) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index c8f3a15..fbb1275 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -2963,6 +2963,9 @@ static int qemudStartVMDaemon(virConnectPtr conn, >      if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) >          goto abort; > > +    if (logfile != -1) > +        close(logfile); > + >      return 0; > >  cleanup:  ACK, but we test  if ((logfile = ...) < 0)       goto cleanup; so the logical counterpart would be  if (logfile >= 0)      close(logfile); Daniel

On Thu, Mar 18, 2010 at 02:31:56PM +0100, Matthias Bolte wrote: > 2010/3/18 Daniel Veillard <veillard(a)redhat.com&gt;: > > On Wed, Mar 17, 2010 at 10:35:51PM +0100, Matthias Bolte wrote: > >> The logfile FD is dup2'ed in __virExec in the child. The FD needs to > >> be closed in the parent, otherwise it leaks. > >> --- > >>  src/qemu/qemu_driver.c |    3 +++ > >>  1 files changed, 3 insertions(+), 0 deletions(-) > >> > >> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > >> index c8f3a15..fbb1275 100644 > >> --- a/src/qemu/qemu_driver.c > >> +++ b/src/qemu/qemu_driver.c > >> @@ -2963,6 +2963,9 @@ static int qemudStartVMDaemon(virConnectPtr conn, > >>      if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) > >>          goto abort; > >> > >> +    if (logfile != -1) > >> +        close(logfile); > >> + > >>      return 0; > >> > >>  cleanup: > > > >  ACK, but we test > > > >  if ((logfile = ...) < 0) > >       goto cleanup; > > > > so the logical counterpart would be > > > >  if (logfile >= 0) > >      close(logfile); > > > > Daniel > > > > True. I just copied the the close call from the cleanup block. Both > blocks (cleanup and abort) check for != 1, so one could argue to > change them to >= 0 too.  Either way, let's plug the leak :-)    thanks ! Daniel