[libvirt] [PATCH 1/2] nodedev: sysfs: Set PCI_PHYSICAL_FUNCTION flag more carefully

[Andrea Bolognani]

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