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@xxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list