On 15.05.2015 17:13, Laine Stump wrote: > On 05/15/2015 05:35 AM, Michal Privoznik wrote: >> On 14.05.2015 21:38, Laine Stump wrote: >>> The kernel won't complain if you set the mac address and vlan tag for >>> an SRIOV VF via its PF, and it will even let you assign the PF to a >>> guest using PCI device assignment or macvtap passthrough. But if the >>> PF isn't online, the device won't be usable in the guest. This patch >>> makes sure that it is turned on. >>> >>> Since multiple guests/VFs could use the same PF, there is no point in >>> ever setting the PF *off*line. >>> >>> This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=893738 >>> >>> Originally filed against RHEL6, but present in every version of >>> libvirt until today. >>> --- >>> src/util/virnetdev.c | 11 +++++++++++ >>> 1 file changed, 11 insertions(+) >>> >>> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c >>> index e14b401..7022dfa 100644 >>> --- a/src/util/virnetdev.c >>> +++ b/src/util/virnetdev.c >>> @@ -2258,6 +2258,17 @@ virNetDevReplaceVfConfig(const char *pflinkdev, int vf, >>> char macstr[VIR_MAC_STRING_BUFLEN]; >>> char *fileData = NULL; >>> int ifindex = -1; >>> + bool pfIsOnline; >>> + >>> + /* Assure that PF is online prior to twiddling with the VF. It >>> + * *should* be, but if the PF isn't online the changes made to the >>> + * VF via the PF won't take effect, yet there will be no error >>> + * reported. >>> + */ >>> + if (virNetDevGetOnline(pflinkdev, &pfIsOnline) < 0) >>> + return ret; >>> + if (!pfIsOnline && virNetDevSetOnline(pflinkdev, true) < 0) >>> + return ret; >>> >>> if (virNetDevGetVfConfig(pflinkdev, vf, &oldmac, &oldvlanid) < 0) >>> goto cleanup; >>> >> ACK. Should we set the device back to its previous state if something >> goes wrong later in the function? > > I don't think so. The PF needs to be on in order for any VF to work > properly, and we can't assume that nobody else has started trying to use > it since we set it online. (this is similar to what we do with the > ip_forward setting in the network driver). > > I guess an alternative would be to *never* set the PF online in libvirt, > but instead to log an error an fail; still better than silently failing > to pass traffic. Any opinions on which is the better approach? I think the better is if libvirt sets the PF online. So my ACK to the patch still holds. Michal -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list