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

[Roopa Prabhu]

On 8/15/11 4:03 AM, "Stefan Berger" <stefanb at linux.vnet.ibm.com> wrote:

> On 08/12/2011 07:14 PM, Roopa Prabhu wrote:
>> From: Roopa Prabhu<roprabhu at 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 at cisco.com>
>> Signed-off-by: Christian Benvenuti<benve at cisco.com>
>> Signed-off-by: David Wang<dwang2 at 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?

Agree. Will implement it as pciConfigAddressEqual.


>> +
>> +/*
>> + * 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.
Inline should do I think. However, I am thinking I will just define these
functions to return -1 for non-linux (same as you have it in interface.c)

Thanks.