On Wed, 2016-03-16 at 19:50 +0100, Andrea Bolognani wrote: > > Thinking in terms of current code - step4 will take the everything that > > was formerly on the active list and: > > > > 1. Steal the pcidevs entry > > > > 2. Call virHostdevReattachPCIDevice with the stolen lookaside pcidevs > > entry... While it appears to be an actual, it isn't. Of course, I think > > you address that in patch 22, but trying not to get too far ahead. > > > > 2a. If the device is not managed, then add to the inactiveList and > > return (call virPCIDeviceFree if unsuccessfully add). What really > > confuses me here is why we place onto the inactiveList if not managed; > > however, you address that in patch22. Still it could be addressed > > earlier if that is a separate bug... > > I would have to go back to patch 22 to make sure this is really > handled correctly there: I'm pretty confident it is, but I've > been wrong before! Most recently, while writing this series :( > > The incorrect behaviour you've noticed before has actually been > introduced by patch 15. Before that, 'pcidevs' in > virHostdevReAttachPCIDevices() was obtained by copying instances > off the active list, hence virHostdevReattachPCIDevice() would > really be passed (a copy of) an actual; now that's no longer the > case, and the code has become incorrect. > > Will fix as part of the respin. Turns out that fixing this in a separate patch would basically mean reverting the patch that introduced the behaviour in the first place. I don't think it would be worth the effort. I've confirmed that the fix is actually in patch 22, as suspected, so we should be good anyway. Cheers. -- Andrea Bolognani Software Engineer - Virtualization Team -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list