[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]