Re: [RFC PATCT 2/2] TODO virNodeDeviceUpdateCaps: checks missing?

On Thu, Mar 28, 2024 at 04:00 PM +0100, Ján Tomko <jtomko(a)redhat.com&gt; wrote: > On a Wednesday in 2024, Marc Hartmayer wrote: >>I'm not familiar with the code so I cannot decide if ignoring the return values >>is a bug or not. At least, it looks awkward and should be annotated. >> > > Adding error reporting after years of real-world usage can be tricky (as > evidenced by the VPD error reporting reverts by Peter). Ohh yes, we should understand the code first. > > I think virNodeDeviceGetSCSITargetCaps erroring out if (!virFCIsCapableRport(rport)) > is incorrect - there were non-FC SCSI targets long before the FC code > was added. Ok, but then we should fix the virNodeDeviceGetSCSITargetCaps and add the proposed check, otherwise the code looks just odd. IMO, a better name for virNodeDeviceGetSCSITargetCaps and for the other functions s/Get/Update/g e.g. virNodeDeviceUpdateSCSITargetCaps …and check all other functions, e.g. virNetDevGetLinkInfo, and all? other functions called by `virNodeDeviceUpdateCaps` have no clear out functionality.