On Thu, Mar 28, 2024 at 04:53 PM +0100, "Marc Hartmayer" <mhartmay@linux.ibm.com> wrote:
On Thu, Mar 28, 2024 at 04:00 PM +0100, Ján Tomko <jtomko@redhat.com> 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.
@Jan, what do you think? For the time being I’ll send a simple memory leak fix without changing the functionality. […snip] -- Kind regards / Beste Grüße Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Wolfgang Wendt Geschäftsführung: David Faller Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294