[libvirt] [PATCH] qemu: Fix FD leak in qemudStartVMDaemon

[Matthias Bolte]

2010/3/18 Daniel Veillard <veillard at redhat.com>:
> On Thu, Mar 18, 2010 at 02:31:56PM +0100, Matthias Bolte wrote:
>> 2010/3/18 Daniel Veillard <veillard at 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