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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list