[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 04/15] Code to return interface name or pci_addr of the VF in actualDevice



On 08/14/2012 06:36 AM, Laine Stump wrote:
> On 08/10/2012 12:23 PM, Shradha Shah wrote:
>> The network pool should be able to keep track of both, network device
>> names nad PCI addresses, and return the appropriate one in the actualDevice
>> when networkAllocateActualDevice is called.
>>
>> Signed-off-by: Shradha Shah <sshah solarflare com>
>> ---
>>  src/network/bridge_driver.c |   33 +++++++++++++++++++++++++++------
>>  src/util/virnetdev.c        |   25 ++++++++++++-------------
>>  src/util/virnetdev.h        |    4 +++-
>>  3 files changed, 42 insertions(+), 20 deletions(-)
>>
>> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
>> index df3cc25..602e17d 100644
>> --- a/src/network/bridge_driver.c
>> +++ b/src/network/bridge_driver.c
>> @@ -59,6 +59,7 @@
>>  #include "dnsmasq.h"
>>  #include "configmake.h"
>>  #include "virnetdev.h"
>> +#include "pci.h"
>>  #include "virnetdevbridge.h"
>>  #include "virnetdevtap.h"
>>  
>> @@ -2737,10 +2738,11 @@ static int
>>  networkCreateInterfacePool(virNetworkDefPtr netdef) {
>>      unsigned int num_virt_fns = 0;
>>      char **vfname = NULL;
>> +    struct pci_config_address **virt_fns;
>>      int ret = -1, ii = 0;
>>      
>>      if ((virNetDevGetVirtualFunctions(netdef->forwardPfs->dev,
>> -                                      &vfname, &num_virt_fns)) < 0) {
>> +                                      &vfname, &virt_fns, &num_virt_fns)) < 0) { 
>>          virReportError(VIR_ERR_INTERNAL_ERROR,
>>                         _("Could not get Virtual functions on %s"),
>>                         netdef->forwardPfs->dev);
>> @@ -2762,19 +2764,38 @@ networkCreateInterfacePool(virNetworkDefPtr netdef) {
>>      netdef->nForwardIfs = num_virt_fns;
>>      
>>      for (ii = 0; ii < netdef->nForwardIfs; ii++) {
>> -        netdef->forwardIfs[ii].device.dev = strdup(vfname[ii]);
>> -        if (!netdef->forwardIfs[ii].device.dev) {
>> -            virReportOOMError();
>> -            goto finish;
> 
> To be pure in the separation of patches, the following if else should be
> removed from this patch, with just the contents of the "if" clause here.
> Then the if else + body of the else should be added in the next patch.
> 
> (And at any rate, the if() condition is incorrect here - really that
> part should happen for all forwardTypes except HOSTDEV (BRIDGE, PRIVATE,
> and VEPA also require netdev names.)

I did not include the BRIDGE, PRIVATE and VEPA cases here because the networkCreateInterfacePool
function is not called in those cases.

Should I still include the conditions for BRIDGE, PRIVATE and VEPA?

> 
> Aside from that movement of code to the next patch, ACK.
> 
>> +        if (netdef->forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH) {
>> +            if(vfname[ii]) {
>> +                netdef->forwardIfs[ii].device.dev = strdup(vfname[ii]);
>> +                if (!netdef->forwardIfs[ii].device.dev) {
>> +                    virReportOOMError();
>> +                    goto finish;
>> +                }
>> +            }
>> +            else {
>> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                               _("Passthrough mode requires interface names"));
>> +                goto finish;
>> +            }
>> +        }
>> +        else if (netdef->forwardType == VIR_NETWORK_FORWARD_HOSTDEV) {
>> +            netdef->forwardIfs[ii].type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_PCI; /*Assuming PCI as VF's are PCI devices */
>> +            netdef->forwardIfs[ii].device.pci.domain = virt_fns[ii]->domain;
>> +            netdef->forwardIfs[ii].device.pci.bus = virt_fns[ii]->bus;
>> +            netdef->forwardIfs[ii].device.pci.slot = virt_fns[ii]->slot;
>> +            netdef->forwardIfs[ii].device.pci.function = virt_fns[ii]->function;
>>          }
>>          netdef->forwardIfs[ii].usageCount = 0;
>>      }
>>      
>>      ret = 0;
>>  finish:
>> -    for (ii = 0; ii < num_virt_fns; ii++)
>> +    for (ii = 0; ii < num_virt_fns; ii++) {
>>          VIR_FREE(vfname[ii]);
>> +        VIR_FREE(virt_fns[ii]);
>> +    }
>>      VIR_FREE(vfname);
>> +    VIR_FREE(virt_fns);
>>      return ret;
>>  }
>>  
>> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
>> index f1ee0a4..8103aff 100644
>> --- a/src/util/virnetdev.c
>> +++ b/src/util/virnetdev.c
>> @@ -29,6 +29,7 @@
>>  #include "command.h"
>>  #include "memory.h"
>>  #include "pci.h"
>> +#include "logging.h"
>>  
>>  #include <sys/ioctl.h>
>>  #ifdef HAVE_NET_IF_H
>> @@ -981,18 +982,18 @@ virNetDevSysfsDeviceFile(char **pf_sysfs_device_link, const char *ifname,
>>  int
>>  virNetDevGetVirtualFunctions(const char *pfname,
>>                               char ***vfname,
>> +                             struct pci_config_address ***virt_fns,
>>                               unsigned int *n_vfname)
>>  {
>>      int ret = -1, i;
>>      char *pf_sysfs_device_link = NULL;
>>      char *pci_sysfs_device_link = NULL;
>> -    struct pci_config_address **virt_fns;
>>      char *pciConfigAddr;
>>  
>>      if (virNetDevSysfsFile(&pf_sysfs_device_link, pfname, "device") < 0)
>>          return ret;
>>  
>> -    if (pciGetVirtualFunctions(pf_sysfs_device_link, &virt_fns,
>> +    if (pciGetVirtualFunctions(pf_sysfs_device_link, virt_fns,
>>                                 n_vfname) < 0)
>>          goto cleanup;
>>  
>> @@ -1003,10 +1004,10 @@ virNetDevGetVirtualFunctions(const char *pfname,
>>  
>>      for (i = 0; i < *n_vfname; i++)
>>      {
>> -        if (pciGetDeviceAddrString(virt_fns[i]->domain,
>> -                                   virt_fns[i]->bus,
>> -                                   virt_fns[i]->slot,
>> -                                   virt_fns[i]->function,
>> +        if (pciGetDeviceAddrString((*virt_fns)[i]->domain,
>> +                                   (*virt_fns)[i]->bus,
>> +                                   (*virt_fns)[i]->slot,
>> +                                   (*virt_fns)[i]->function,
>>                                     &pciConfigAddr) < 0) {
>>              virReportSystemError(ENOSYS, "%s",
>>                                   _("Failed to get PCI Config Address String"));
>> @@ -1019,20 +1020,17 @@ virNetDevGetVirtualFunctions(const char *pfname,
>>          }
>>  
>>          if (pciDeviceNetName(pci_sysfs_device_link, &((*vfname)[i])) < 0) {
>> -            virReportSystemError(ENOSYS, "%s",
>> -                                 _("Failed to get interface name of the VF"));
>> -            goto cleanup;
>> +            VIR_INFO("VF does not have an interface name");
>>          }
>>      }
>>  
>>      ret = 0;
>>  
>>  cleanup:
>> -    if (ret < 0)
>> +    if (ret < 0) {
>>          VIR_FREE(*vfname);
>> -    for (i = 0; i < *n_vfname; i++)
>> -        VIR_FREE(virt_fns[i]);
>> -    VIR_FREE(virt_fns);
>> +        VIR_FREE(*virt_fns);
>> +    }
>>      VIR_FREE(pf_sysfs_device_link);
>>      VIR_FREE(pci_sysfs_device_link);
>>      VIR_FREE(pciConfigAddr);
>> @@ -1169,6 +1167,7 @@ cleanup:
>>  int
>>  virNetDevGetVirtualFunctions(const char *pfname ATTRIBUTE_UNUSED,
>>                               char ***vfname ATTRIBUTE_UNUSED,
>> +                             struct pci_config_address ***virt_fns ATTRIBUTE_UNUSED,
>>                               unsigned int *n_vfname ATTRIBUTE_UNUSED)
>>  {
>>      virReportSystemError(ENOSYS, "%s",
>> diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h
>> index c663e49..705ad9c 100644
>> --- a/src/util/virnetdev.h
>> +++ b/src/util/virnetdev.h
>> @@ -26,6 +26,7 @@
>>  # include "virsocketaddr.h"
>>  # include "virnetlink.h"
>>  # include "virmacaddr.h"
>> +# include "pci.h"
>>  
>>  int virNetDevExists(const char *brname)
>>      ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
>> @@ -103,9 +104,10 @@ int virNetDevGetPhysicalFunction(const char *ifname, char **pfname)
>>  
>>  int virNetDevGetVirtualFunctions(const char *pfname,
>>                                   char ***vfname,
>> +                                 struct pci_config_address ***virt_fns,
>>                                   unsigned int *n_vfname)
>>      ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
>> -    ATTRIBUTE_RETURN_CHECK;
>> +    ATTRIBUTE_NONNULL(4) ATTRIBUTE_RETURN_CHECK;
>>  
>>  int virNetDevLinkDump(const char *ifname, int ifindex,
>>                        struct nlattr **tb,
> 
> --
> libvir-list mailing list
> libvir-list redhat com
> https://www.redhat.com/mailman/listinfo/libvir-list


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]