Re: [libvirt] [PATCH] qemu: Fix FD leak in qemudStartVMDaemon
2010/3/18 Daniel Veillard <veillard(a)redhat.com>:
On Thu, Mar 18, 2010 at 02:31:56PM +0100, Matthias Bolte wrote:
> 2010/3/18 Daniel Veillard <veillard(a)redhat.com>:
> > 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
Yep, pushed.
Matthias