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

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(a)solarflare.com&gt; > --- > 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.)

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(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list