[libvirt] [PATCH 2/5] Moved the code to create implicit interface pool from PF to a new function

[Laine Stump]

On 06/08/2012 11:29 AM, Shradha Shah wrote:
> This makes the code reusable

Rather than adding new code, then moving that new code around along with
existing code that needs to be moved, it would be cleaner and easier to
follow if you first moved the existing code, then introduced the new
code (directly in the place that it will eventually live).

>
> Signed-off-by: Shradha Shah <sshah at solarflare.com>
> ---
>  src/network/bridge_driver.c |  109 ++++++++++++++++++++++++++-----------------
>  1 files changed, 66 insertions(+), 43 deletions(-)
>
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index 41e3a49..8540003 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -2761,6 +2761,60 @@ int networkRegister(void) {
>   * "backend" function table.
>   */
>  
> +/* networkCreateInterfacePool:
> + * @netdef: the original NetDef from the network
> + *
> + * Creates an implicit interface pool of VF's when a PF dev is given
> + */
> +static int 
> +networkCreateInterfacePool(virNetworkDefPtr netdef) {
> +    unsigned int num_virt_fns = 0;
> +    char **vfname = NULL;
> +    int ret = -1, ii = 0;
> +    
> +    if ((virNetDevGetVirtualFunctions(netdef->forwardPfs->dev,
> +                                      &vfname, &num_virt_fns)) < 0) {
> +        networkReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Could not get Virtual functions on %s"),
> +                           netdef->forwardPfs->dev);
> +        goto finish;
> +    }
> +    
> +    if (num_virt_fns == 0) {
> +        networkReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("No Vf's present on SRIOV PF %s"),
> +                           netdef->forwardPfs->dev);
> +        goto finish;
> +    }
> +    
> +    if ((VIR_ALLOC_N(netdef->forwardIfs, num_virt_fns)) < 0) {
> +        virReportOOMError();
> +        goto finish;
> +    }
> +    
> +    netdef->nForwardIfs = num_virt_fns;
> +    
> +    for (ii = 0; ii < netdef->nForwardIfs; ii++) {
> +        netdef->forwardIfs[ii].dev = strdup(vfname[ii]);
> +        if (!netdef->forwardIfs[ii].dev) {
> +            virReportOOMError();
> +            goto finish;
> +        }
> +        netdef->forwardIfs[ii].usageCount = 0;
> +        if (wildcmp("????:??:??.?", netdef->forwardIfs[ii].dev)) 
> +            netdef->forwardIfs[ii].isPciAddr = true;
> +        else 
> +            netdef->forwardIfs[ii].isPciAddr = false;
> +    }
> +    
> +    ret = 0;
> +finish:
> +    for (ii = 0; ii < num_virt_fns; ii++)
> +        VIR_FREE(vfname[ii]);
> +    VIR_FREE(vfname);
> +    return ret;
> +}
> +
>  /* networkAllocateActualDevice:
>   * @iface: the original NetDef from the domain
>   *
> @@ -2779,8 +2833,6 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
>      virNetworkObjPtr network;
>      virNetworkDefPtr netdef;
>      virPortGroupDefPtr portgroup;
> -    unsigned int num_virt_fns = 0;
> -    char **vfname = NULL;
>      int ii;
>      int ret = -1;
>  
> @@ -2858,7 +2910,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
>                 (netdef->forwardType == VIR_NETWORK_FORWARD_VEPA) ||
>                 (netdef->forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH)) {
>          virNetDevVPortProfilePtr virtport = NULL;
> -
> +        int rc = -1;
>          /* <forward type='bridge|private|vepa|passthrough'> are all
>           * VIR_DOMAIN_NET_TYPE_DIRECT.
>           */
> @@ -2926,57 +2978,31 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
>               */
>              if (netdef->forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH) {
>                  if ((netdef->nForwardPfs > 0) && (netdef->nForwardIfs <= 0)) {
> -                    if ((virNetDevGetVirtualFunctions(netdef->forwardPfs->dev,
> -                                                      &vfname, &num_virt_fns)) < 0) {
> +                    if((rc = networkCreateInterfacePool(netdef)) < 0) {
>                          networkReportError(VIR_ERR_INTERNAL_ERROR,
> -                                           _("Could not get Virtual functions on %s"),
> -                                           netdef->forwardPfs->dev);
> +                                           _("Could not create Interface Pool from PF"));
>                          goto cleanup;
>                      }
> -
> -                    if (num_virt_fns == 0) {
> +                }
> +                
> +                /*if dev names are pci addrs in passthrough mode: error*/
> +                for (ii = 0; ii <netdef->nForwardIfs; ii++) {
> +                    if (netdef->forwardIfs[ii].isPciAddr == true) {
>                          networkReportError(VIR_ERR_INTERNAL_ERROR,
> -                                           _("No Vf's present on SRIOV PF %s"),
> -                                           netdef->forwardPfs->dev);
> +                                           _("Passthrough mode does not work with PCI addresses and needs Interface names"));
>                          goto cleanup;
>                      }
> -
> -                    if ((VIR_ALLOC_N(netdef->forwardIfs, num_virt_fns)) < 0) {
> -                        virReportOOMError();
> -                        goto cleanup;
> -                    }
> -
> -                    netdef->nForwardIfs = num_virt_fns;
> -
> -                    for (ii = 0; ii < netdef->nForwardIfs; ii++) {
> -                        netdef->forwardIfs[ii].dev = strdup(vfname[ii]);
> -                        if (!netdef->forwardIfs[ii].dev) {
> -                            virReportOOMError();
> -                            goto cleanup;
> -                        }
> -                        netdef->forwardIfs[ii].usageCount = 0;
> -                        if (wildcmp("????:??:??.?",netdef->forwardIfs[ii].dev)) 
> -                            netdef->forwardIfs[ii].isPciAddr = true;
> -                        else 
> -                            netdef->forwardIfs[ii].isPciAddr = false;
> -
> -                        if (netdef->forwardIfs[ii].isPciAddr == true) {
> -                            networkReportError(VIR_ERR_INTERNAL_ERROR,
> -                                               _("Passthrough mode does not work with PCI addresses and needs Interface names"));
> -                            goto cleanup;
> -                        }
> -                    }
>                  }
> -
> +                
>                  /* pick first dev with 0 usageCount */
>  
>                  for (ii = 0; ii < netdef->nForwardIfs; ii++) {
>                      if (netdef->forwardIfs[ii].usageCount == 0) {
>                          dev = &netdef->forwardIfs[ii];
>                          break;
> -                    }
> +                     }
>                  }
> -            } else if ((netdef->forwardType == VIR_NETWORK_FORWARD_PRIVATE) &&
> +            }else if ((netdef->forwardType == VIR_NETWORK_FORWARD_PRIVATE) &&
>                         iface->data.network.actual->data.direct.virtPortProfile &&
>                         (iface->data.network.actual->data.direct.virtPortProfile->virtPortType
>                          == VIR_NETDEV_VPORT_PROFILE_8021QBH)) {
> @@ -3017,9 +3043,6 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
>  
>      ret = 0;
>  cleanup:
> -    for (ii = 0; ii < num_virt_fns; ii++)
> -        VIR_FREE(vfname[ii]);
> -    VIR_FREE(vfname);
>      if (network)
>          virNetworkObjUnlock(network);
>      if (ret < 0) {