Re: [libvirt] [PATCH 4/4] qemu: Do not keep PCI device config file open
On Tue, Dec 04, 2012 at 17:55:33 +0000, Daniel P. Berrange wrote:
On Tue, Dec 04, 2012 at 11:38:22AM +0100, Jiri Denemark wrote:
> We only need to access the PCI device config file when
> attaching/detaching the device to a domain. Keeping it open all the time
> the device is attached to a domain is useless.
IMHO this is really just papering over a really terrible API
design in the pciDevicePtr code. IMHO the core issue here is
that two of our APIs pciDeviceReset() and pciDeviceIsAssignable()
will open an FD todo their work and then not close it afterwards.
As a caller of pciDeviceReset or pciDeviceIsAssignable I would
really not expect that a file descriptor is left open. As such
I don't think requiring the caller to use pciDeviceClose is the
right approach. Those methods should be made leak-proof by
having them close the FD themselves. To re-inforce this, I'd
actually remove 'int fd' from the struct entirely, and have it
be method-local, passed around as needed.
Hmm, right, although having this terrible design allowed us to catch the
memory leak since fd leak is more visible :-) It really seems there's no need
to keep the fd open across several APIs. I'll rework this patch.
Jirka