ping On 2/23/21 10:35 PM, Laine Stump wrote: > Some SRIOV PFs don't have a netdev associated with them (the spec > apparently doesn't require it). In most cases when libvirt is dealing > with an SRIOV VF, that VF must have a PF, and the PF *must* have an > associated netdev (the only way to set the MAC address of a VF is by > sending a netlink message to the netdev of that VF's PF). But there > are times when we don't need for the PF to have a netdev; in > particular, when we're just getting the Switchdev Features for a VF, > we don't need the PF netdev - the netdev of the VF (apparently) works > just as well. > > Commit 6452e2f5 (libvirt 5.1.0) *kind of* made libvirt work around PFs > with no netdevs in this case - if virNetDevGetPhysicalFunction > returned an error when setting up to retrieve Switchdev feature info, > it would ignore the error, and then check if the PF netdev name was > NULL and, if so it would reset the error object and continue on rather > than returning early with a failure. The problem is that by the time > this special handling occured, the error message about missing netdev > had already been logged, which was harmless to proper operation, but > confused the user. > > Fortunately there are only 2 users of virNetDevGetPhysicalFunction, so > it is easy to redefine it's API to state that a missing netdev name is > *not* an error - in that case it will still return success, but the > caller must be prepared for the PF netdev name to be NULL. After > making this change, we can modify the two callers to behave properly > with the new semantics (for one of the callers it *is* still an error, > so the error message is moved there, but for the other it is okay to > continue), and our spurious error messages are a thing of the past. > > Resolves: https://bugzilla.redhat.com/1924616 > Fixes: 6452e2f5e1837bd753ee465e6607ed3c4f62b815 > Signed-off-by: Laine Stump <laine(a)redhat.com&gt; > --- >   src/util/virnetdev.c | 31 +++++++++++++++---------------- >   1 file changed, 15 insertions(+), 16 deletions(-) > > diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c > index 2b4c8b6280..7b766234ec 100644 > --- a/src/util/virnetdev.c > +++ b/src/util/virnetdev.c > @@ -1339,7 +1339,8 @@ virNetDevGetVirtualFunctionIndex(const char > *pfname, const char *vfname, >    * >    * @ifname : name of the physical function interface name >    * @pfname : Contains sriov physical function for interface ifname > - *           upon successful return > + *           upon successful return (might be NULL if the PF has no > + *           associated netdev. This is *not* an error) >    * >    * Returns 0 on success, -1 on failure >    * > @@ -1361,15 +1362,6 @@ virNetDevGetPhysicalFunction(const char > *ifname, char **pfname) >           return -1; >       } > -    if (!*pfname) { > -        /* The SRIOV standard does not require VF netdevs to have > -         * the netdev assigned to a PF. */ > -        virReportError(VIR_ERR_INTERNAL_ERROR, > -                       _("The PF device for VF %s has no network > device name"), > -                       ifname); > -        return -1; > -    } > - >       return 0; >   } > @@ -1442,6 +1434,17 @@ virNetDevGetVirtualFunctionInfo(const char > *vfname, char **pfname, >       if (virNetDevGetPhysicalFunction(vfname, pfname) < 0) >           return -1; > +    if (!*pfname) { > +        /* The SRIOV standard does not require VF netdevs to have the > +         * netdev assigned to a PF, but our method of retrieving > +         * VFINFO does. > +         */ > +        virReportError(VIR_ERR_INTERNAL_ERROR, > +                       _("The PF device for VF %s has no network > device name, cannot get virtual function info"), > +                       vfname); > +        return -1; > +    } > + >       if (virNetDevGetVirtualFunctionIndex(*pfname, vfname, vf) < 0) >           goto cleanup; > @@ -3204,12 +3207,8 @@ virNetDevSwitchdevFeature(const char *ifname, >       if ((is_vf = virNetDevIsVirtualFunction(ifname)) < 0) >           return ret; > -    if (is_vf == 1) { > -        /* Ignore error if PF does not have netdev assigned. > -         * In that case pfname == NULL. */ > -        if (virNetDevGetPhysicalFunction(ifname, &pfname) < 0) > -            virResetLastError(); > -    } > +    if (is_vf == 1 && virNetDevGetPhysicalFunction(ifname, &pfname) < 0) > +        return ret; >       pci_device_ptr = pfname ? virNetDevGetPCIDevice(pfname) : >                                 virNetDevGetPCIDevice(ifname); >