On Tue, Apr 29, 2014 at 05:32:45PM +0100, Daniel P. Berrange wrote:
On Tue, Apr 29, 2014 at 05:25:12PM +0100, Daniel P. Berrange wrote:
> On Tue, Apr 29, 2014 at 06:19:56PM +0200, Martin Kletzander wrote:
> > Hi everyone,
> >
> > after upgrade to gnutls-3.3.0, I discovered (commandtest fails) that
> > any code linked with -lgnutls will have not 3, but 5 open file
> > descriptors upon the entry into main(). I asked on gnutls-help [1] if
> > they know they are leaking file descriptors. The response was, that
> > this is intended with the explanation being that these FDs (pointing
> > to /dev/urandom) are kept open for backward compatibility with
> > programs that may chroot into environment without /dev/urandom as the
> > previous version didn't require to have access to /dev/urandom when
> > calling gnutls code.
> >
> > Does that seem like our bug that we're relying on fixed number of open
> > file descriptors? Or that we're linking to gnutls when we don't need
> > it in commandhelper? Or should this be fixed somewhere else?
>
> Hmm, before considering the test suite - what is the behaviour when
> we use virCommand for real. ie if we launch QEMU, is gnutls causing
> us to leak a /dev/urandom FD to QEMU ? Or is the fact that we
> iterate over all FDs forcing them to be close saving us.
Oh actally I see what you mean. We're not leaking the FD from
commandtest -> commandhelper, the constructor runs in commandhelper
causing it to open up new FDs.
We should not in fact link commandhelper to libvirt.la at all since
it doesn't need any of that functionality.
That's right, I haven't realized that, I'll send a patch for that.
I was thinking that it might cause lots of problems when the library
leaves open FDs, for example when passing FDs like systemd does (I
still have to implement that, just remembered that), but then I
realized that the FDs that get passed are still numbered from 3 and
the FDs the library opens are starting from the first one *after* all
the passed ones, so this is OK from this POV.
Thanks,
Martin