Re: [libvirt] [PATCH 2/4 v2] pci: Add helper functions for sriov devices

On 08/12/2011 07:14 PM, Roopa Prabhu wrote: > From: Roopa Prabhu<roprabhu(a)cisco.com> > > This patch adds the following helper functions: > pciDeviceIsVirtualFunction: Function to check if a pci device is a sriov VF > pciGetVirtualFunctionIndex: Function to get the VF index of a sriov VF > pciDeviceNetName: Function to get the network device name of a pci device > pciConfigAddressCompare: Function to compare pci config addresses > > Signed-off-by: Roopa Prabhu<roprabhu(a)cisco.com> > Signed-off-by: Christian Benvenuti<benve(a)cisco.com> > Signed-off-by: David Wang<dwang2(a)cisco.com> > --- > src/util/pci.c | 118 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > src/util/pci.h | 15 +++++++ > 2 files changed, 133 insertions(+), 0 deletions(-) > > > diff --git a/src/util/pci.c b/src/util/pci.c > index e017db9..558c7ae 100644 > --- a/src/util/pci.c > +++ b/src/util/pci.c > @@ -1875,3 +1875,121 @@ out: > > return ret; > } > + > +/* > + * returns 0 if equal and 1 if not > + */ > +static int > +pciConfigAddressCompare(struct pci_config_address *bdf1, > + struct pci_config_address *bdf2) > +{ > + return !((bdf1->domain == bdf2->domain)& > + (bdf1->bus == bdf2->bus)& > + (bdf1->slot == bdf2->slot)& > + (bdf1->function == bdf2->function)); > +} Would it no be more intuitive to call implement a function pciConfigAddressEqual and return '1' in case the addresses are equal?

> + > +/* > + * Returns 1 if vf device is a virtual function, 0 if not, -1 on error > + */ > +int > +pciDeviceIsVirtualFunctionLinux(const char *vf_sysfs_device_link) > +{ > + char *vf_sysfs_physfn_link = NULL; > + int ret = -1; > + > + if (virAsprintf(&vf_sysfs_physfn_link, "%s/physfn", > + vf_sysfs_device_link)< 0) { > + virReportOOMError(); > + return ret; > + } > + > + ret = virFileExists(vf_sysfs_physfn_link); > + > + VIR_FREE(vf_sysfs_physfn_link); > + > + return ret; > +} > + > +/* > + * Returns the sriov virtual function index of vf given its pf > + */ > +int > +pciGetVirtualFunctionIndexLinux(const char *pf_sysfs_device_link, > + const char *vf_sysfs_device_link, > + int *vf_index) > +{ > + int ret = -1, i; > + unsigned int num_virt_fns = 0; > + struct pci_config_address *vf_bdf = NULL; > + struct pci_config_address **virt_fns = NULL; > + > + if (pciGetPciConfigAddressFromSysfsDeviceLink(vf_sysfs_device_link, > +&vf_bdf)) > + return ret; > + > + if (pciGetVirtualFunctionsLinux(pf_sysfs_device_link,&virt_fns, > +&num_virt_fns)) { > + VIR_DEBUG("Error getting physical function's '%s' > virtual_functions\n", > + pf_sysfs_device_link); > + goto out; > + } > + > + for (i = 0; i< num_virt_fns; i++) { > + if (!pciConfigAddressCompare(vf_bdf, virt_fns[i])) { > + *vf_index = i; > + ret = 0; > + break; > + } > + } > + > +out: > + > + /* free virtual functions */ > + for (i = 0; i< num_virt_fns; i++) > + VIR_FREE(virt_fns[i]); > + > + VIR_FREE(vf_bdf); > + > + return ret; > +} > + > +/* > + * Returns the network device name of a pci device > + */ > +int > +pciDeviceNetNameLinux(char *device_link_sysfs_path, char **netname) > +{ > + char *pcidev_sysfs_net_path = NULL; > + int ret = -1; > + DIR *dir = NULL; > + struct dirent *entry = NULL; > + > + if (virBuildPath(&pcidev_sysfs_net_path, device_link_sysfs_path, > + "net") == -1) { > + virReportOOMError(); > + return -1; > + } > + > + dir = opendir(pcidev_sysfs_net_path); > + if (dir == NULL) > + goto out; > + > + while ((entry = readdir(dir))) { > + if (!strcmp(entry->d_name, ".") || > + !strcmp(entry->d_name, "..")) > + continue; > + > + /* Assume a single directory entry */ > + *netname = strdup(entry->d_name); > + ret = 0; > + break; > + } > + > + closedir(dir); > + > +out: > + VIR_FREE(pcidev_sysfs_net_path); > + > + return ret; > +} > diff --git a/src/util/pci.h b/src/util/pci.h > index 367881e..0cdc6ec 100644 > --- a/src/util/pci.h > +++ b/src/util/pci.h > @@ -92,10 +92,25 @@ int pciGetVirtualFunctionsLinux(const char *sysfs_path, > struct pci_config_address > ***virtual_functions, > unsigned int *num_virtual_functions); > > +# define pciDeviceIsVirtualFunction(v) pciDeviceIsVirtualFunctionLinux(v) > +int pciDeviceIsVirtualFunctionLinux(const char *vf_sysfs_device_link); > + > +# define pciGetVirtualFunctionIndex(p,v,i) \ > + pciGetVirtualFunctionIndexLinux(p,v,i) > +int pciGetVirtualFunctionIndexLinux(const char *pf_sysfs_device_link, > + const char *vf_sysfs_device_link, > + int *vf_index); > + > +# define pciDeviceNetName(d,n) pciDeviceNetNameLinux(d,n) > +int pciDeviceNetNameLinux(char *device_link_sysfs_path, char **netname); > + > #else /* __linux__ */ > > # define pciGetPhysicalFunction(s,a) > # define pciGetVirtualFunctions(s,a,n) > +# define pciDeviceIsVirtualFunction(v) Also here is there is code like 'if (!pciDeviceIsVirtualFunction())' it will be expanded to if (!) on non-Linux platforms, which is not compilable. Maybe an inline function in this case will do? > +# define pciGetVirtualFunctionIndex(p,v,i) > +# define pciDeviceNetNameLinux(d,n) Same for these two also.