On Mon, 2016-05-23 at 15:21 -0400, Laine Stump wrote: > On 05/23/2016 07:53 AM, Andrea Bolognani wrote: > > > > The flag was set if virPCIGetPhysicalFunction() did not return > > an error; unfortunately, the 'physfn' sysfs file not being > > present is not considered an error, which means that the flag > > was set pretty much unconditionally. > > This behavior is the result of commit c8b1a83605: > > https://www.redhat.com/archives/libvir-list/2016-May/msg01348.html > > Prior to this, virPCIGetPhysicalFunction had returned the return value > from virPCIGetDeviceAddressFromSysfsLink(), which would be -1 if the > physfn file didn't exist. That patch discards the return value and > hardcodes a "return 0". > > I think that virPCIGetPhysicalFunction should be restored to its > original glory (No, I didn't write it). Either that, or the other caller > to virPCIGetPhysicalFunction (virPCIGetVirtualFunctionInfo) should also > be fixed so that it doesn't crash due to the changed semantics of > virPCIGetPhysicalFunction. > > (A more correct fix may be to change the semantics of > virPCIGetPhysicalFunction() to return -1 only in the case of a bonafide > error (OOM, failure to parse the contents of an existing physfn file), > but that would also require modifying > virPCIGetDeviceAddressFromSysfsLink() to return 0/-1 rather than a > virPCIDeviceAddresPtr so that we can tell the difference between "no > physfn" and "there was a physfn but I encountered some other error". In > other words - 1) revert commit c1faf309 (part of the same "coverity > fixes" series as x8b1a83605), then 2) adjust the function to set *bdf = > 0 and return 0 when the physfn file doesn't exist.) Thanks for the analysis. Fixing the remaining caller looked like the simpler fix, so I've gone that route. Patch 2/2 is unchanged. https://www.redhat.com/archives/libvir-list/2016-May/msg01783.html -- Andrea Bolognani Software Engineer - Virtualization Team -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list