mirror of
https://gitlab.com/libvirt/libvirt.git
synced 2024-08-28 19:41:16 +00:00
util: fix erroneous requirement for phys_port_id to get ifname of a VF
Commit795e9e05c3
(libvirt-7.7.0) refactored the code in virpci.c and virnetdev.c that gathered lists of the Virtual Functions (VF) of an SRIOV Physical Function (PF) to simplify the code. Unfortunately the simplification made the assumption, in the new function virPCIGetVirtualFunctionsFull(), that a VF's netdev interface name should only be retrieved if the PF had a valid phys_port_id. That is an incorrect assumption - only a small handful of (now previous-generation) Mellanox SRIOV cards actually use phys_port_id (this is for an odd design where there are multiple physical network ports on a single PCI address); all other SRIOV cards (including new Mellanox cards) have a file in sysfs called phys_port_id, but it can't be read, and so the pfPhysPortID string is NULL. The result of this logic error is that virtual networks that are a pool of VFs to be used for macvtap connections will be unable to start, giving an errror like this: VF 0 of SRIOV PF enp130s0f0 couldn't be added to the interface pool because it isn't bound to a network driver - possibly in use elsewhere This error message is misinformed - the caller of virNetDevGetVirtualFunctionsFull() only *thinks* that the VF isn't bound to a network driver because it doesn't see a netdev name for the VF in the list. But that's only because virNetDevGetVirtualFunctionsFull() didn't even try to get the names! We do need a way for virPCIGetVirtualFunctionsFull() to sometimes retrieve the netdev names and sometimes not. One way of doing that would be to send down the netdev name of the PF whenever we also want to know the netdev names of the VFs, but send a NULL when we don't. This can conveniently be done by just *replacing* pfPhysPortID in the arglist with pfNetDevName - pfPhysPortID is determined by simply calling virNetDevGetPhysPortID(pfNetDevName) so we can just make that call down in virPCIGetVirtualFunctionsFull() (when needed). This solves the regression introduced by commit795e9e05c3
, and also nicely sets us up to (in a subsequent commit) move the call to virNetDevGetPhysPortID() down one layer further to virPCIGetNetName(), where it really belongs! Resolves: https://bugzilla.redhat.com/2025432 Fixes:795e9e05c3
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This commit is contained in:
parent
c71181b666
commit
71345f91d7
@ -1223,15 +1223,11 @@ virNetDevGetVirtualFunctions(const char *pfname,
|
|||||||
virPCIVirtualFunctionList **vfs)
|
virPCIVirtualFunctionList **vfs)
|
||||||
{
|
{
|
||||||
g_autofree char *pf_sysfs_device_link = NULL;
|
g_autofree char *pf_sysfs_device_link = NULL;
|
||||||
g_autofree char *pfPhysPortID = NULL;
|
|
||||||
|
|
||||||
if (virNetDevGetPhysPortID(pfname, &pfPhysPortID) < 0)
|
|
||||||
return -1;
|
|
||||||
|
|
||||||
if (virNetDevSysfsFile(&pf_sysfs_device_link, pfname, "device") < 0)
|
if (virNetDevSysfsFile(&pf_sysfs_device_link, pfname, "device") < 0)
|
||||||
return -1;
|
return -1;
|
||||||
|
|
||||||
if (virPCIGetVirtualFunctionsFull(pf_sysfs_device_link, vfs, pfPhysPortID) < 0)
|
if (virPCIGetVirtualFunctionsFull(pf_sysfs_device_link, vfs, pfname) < 0)
|
||||||
return -1;
|
return -1;
|
||||||
|
|
||||||
return 0;
|
return 0;
|
||||||
|
@ -2340,8 +2340,8 @@ virPCIGetPhysicalFunction(const char *vf_sysfs_path,
|
|||||||
* virPCIGetVirtualFunctionsFull:
|
* virPCIGetVirtualFunctionsFull:
|
||||||
* @sysfs_path: path to physical function sysfs entry
|
* @sysfs_path: path to physical function sysfs entry
|
||||||
* @vfs: filled with the virtual function data
|
* @vfs: filled with the virtual function data
|
||||||
* @pfPhysPortID: Optional physical port id. If provided the network interface
|
* @pfNetDevName: Optional netdev name of this PF. If provided, the netdev
|
||||||
* name of the VFs is queried too.
|
* names of the VFs are queried too.
|
||||||
*
|
*
|
||||||
*
|
*
|
||||||
* Returns virtual functions of a physical function.
|
* Returns virtual functions of a physical function.
|
||||||
@ -2349,7 +2349,7 @@ virPCIGetPhysicalFunction(const char *vf_sysfs_path,
|
|||||||
int
|
int
|
||||||
virPCIGetVirtualFunctionsFull(const char *sysfs_path,
|
virPCIGetVirtualFunctionsFull(const char *sysfs_path,
|
||||||
virPCIVirtualFunctionList **vfs,
|
virPCIVirtualFunctionList **vfs,
|
||||||
const char *pfPhysPortID)
|
const char *pfNetDevName)
|
||||||
{
|
{
|
||||||
g_autofree char *totalvfs_file = NULL;
|
g_autofree char *totalvfs_file = NULL;
|
||||||
g_autofree char *totalvfs_str = NULL;
|
g_autofree char *totalvfs_str = NULL;
|
||||||
@ -2390,8 +2390,12 @@ virPCIGetVirtualFunctionsFull(const char *sysfs_path,
|
|||||||
return -1;
|
return -1;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (pfPhysPortID) {
|
if (pfNetDevName) {
|
||||||
if (virPCIGetNetName(device_link, 0, pfPhysPortID, &fnc.ifname) < 0) {
|
g_autofree char *pfPhysPortID = NULL;
|
||||||
|
|
||||||
|
if (virNetDevGetPhysPortID(pfNetDevName, &pfPhysPortID) < 0 ||
|
||||||
|
virPCIGetNetName(device_link, 0, pfPhysPortID, &fnc.ifname) < 0) {
|
||||||
|
|
||||||
g_free(fnc.addr);
|
g_free(fnc.addr);
|
||||||
return -1;
|
return -1;
|
||||||
}
|
}
|
||||||
@ -2712,7 +2716,7 @@ virPCIGetPhysicalFunction(const char *vf_sysfs_path G_GNUC_UNUSED,
|
|||||||
int
|
int
|
||||||
virPCIGetVirtualFunctionsFull(const char *sysfs_path G_GNUC_UNUSED,
|
virPCIGetVirtualFunctionsFull(const char *sysfs_path G_GNUC_UNUSED,
|
||||||
virPCIVirtualFunctionList **vfs G_GNUC_UNUSED,
|
virPCIVirtualFunctionList **vfs G_GNUC_UNUSED,
|
||||||
const char *pfPhysPortID G_GNUC_UNUSED)
|
const char *pfNetDevName G_GNUC_UNUSED)
|
||||||
{
|
{
|
||||||
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported));
|
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported));
|
||||||
return -1;
|
return -1;
|
||||||
|
@ -230,7 +230,7 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(virPCIVirtualFunctionList, virPCIVirtualFunctionLi
|
|||||||
|
|
||||||
int virPCIGetVirtualFunctionsFull(const char *sysfs_path,
|
int virPCIGetVirtualFunctionsFull(const char *sysfs_path,
|
||||||
virPCIVirtualFunctionList **vfs,
|
virPCIVirtualFunctionList **vfs,
|
||||||
const char *pfPhysPortID);
|
const char *pfNetDevName);
|
||||||
int virPCIGetVirtualFunctions(const char *sysfs_path,
|
int virPCIGetVirtualFunctions(const char *sysfs_path,
|
||||||
virPCIVirtualFunctionList **vfs);
|
virPCIVirtualFunctionList **vfs);
|
||||||
|
|
||||||
|
Loading…
Reference in New Issue
Block a user