On Wed, Dec 14, 2011 at 10:50:30AM +0000, Shradha Shah wrote:
> If a system has 64 or more VF's, it is quite tedious to mention each VF
> in the interface pool.
> The following modification will implicitly create an interface pool from
> the SR-IOV PF.
> ---
> src/network/bridge_driver.c | 93 +++++++++++++++++++++++++++++++++---------
> 1 files changed, 73 insertions(+), 20 deletions(-)
>
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index 63338a2..9ae9c50 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -2817,15 +2817,19 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
> /* If there is only a single device, just return it (caller will detect
> * any error if exclusive use is required but could not be acquired).
> */
> - if (netdef->nForwardIfs == 0) {
> + if ((netdef->nForwardIfs <= 0) && (netdef->nForwardPfs <= 0)) {
> networkReportError(VIR_ERR_INTERNAL_ERROR,
> _("network '%s' uses a direct mode, but has no forward dev and no interface pool"),
> netdef->name);
> goto cleanup;
> - } else {
> + }
> +
> + else {
Minor style issue - can you keep the '} else {' alignment.
> int ii;
> virNetworkForwardIfDefPtr dev = NULL;
> -
> + unsigned int num_virt_fns = 0;
> + char **vfname = NULL;
> +
> /* pick an interface from the pool */
>
> /* PASSTHROUGH mode, and PRIVATE Mode + 802.1Qbh both require
> @@ -2833,20 +2837,67 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
> * 0. Other modes can share, so just search for the one with
> * the lowest usageCount.
> */
> - if ((netdef->forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH) ||
> - ((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))) {
> + if (netdef->forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH) {
> + if ((netdef->nForwardPfs > 0) && (netdef->nForwardIfs <= 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 out;
> + }
> +
> + if (num_virt_fns == 0) {
> + networkReportError(VIR_ERR_INTERNAL_ERROR,
> + _("No Vf's present on SRIOV PF %s"),
> + netdef->forwardPfs->dev);
> + goto out;
> + }
> +
> + if ((VIR_ALLOC_N(netdef->forwardIfs, num_virt_fns)) < 0) {
> + virReportOOMError();
> + goto out;
> + }
> +
> + netdef->nForwardIfs = num_virt_fns;
> +
> + for (ii = 0; ii < netdef->nForwardIfs; ii++) {
> + netdef->forwardIfs[ii].dev = strdup(vfname[ii]);
Missing check for OOM here
> + netdef->forwardIfs[ii].usageCount = 0;
> + }
> +
> + out:
> + for (ii = 0; ii < num_virt_fns; ii++) {
> + VIR_FREE(vfname[ii]);
> + }
> + VIR_FREE(vfname);
> + }
> +
> /* 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 {
> + }
> + 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)) {
Also '} else if '
> +
> +
> + /* 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 {
And '} else {'
> /* pick least used dev */
> dev = &netdef->forwardIfs[0];
> for (ii = 1; ii < netdef->nForwardIfs; ii++) {
> @@ -2858,7 +2909,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
> if (!dev) {
> networkReportError(VIR_ERR_INTERNAL_ERROR,
> _("network '%s' requires exclusive access to interfaces, but none are available"),
> - netdef->name);
> + netdef->name);
> goto cleanup;
> }
> iface->data.network.actual->data.direct.linkdev = strdup(dev->dev);
>From this point onwards, AFAICT, none of the patch hunks have
any functional change. They're all just whitespace changes, or
moving variable declarations. So I think the rest of this patch
can just be dropped.
> @@ -2901,6 +2952,7 @@ networkNotifyActualDevice(virDomainNetDefPtr iface)
> virNetworkObjPtr network;
> virNetworkDefPtr netdef;
> const char *actualDev;
> + virNetworkForwardIfDefPtr dev = NULL;
> int ret = -1;
>
> if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK)
> @@ -2935,12 +2987,12 @@ networkNotifyActualDevice(virDomainNetDefPtr iface)
> _("network '%s' uses a direct mode, but has no forward dev and no interface pool"),
> netdef->name);
> goto cleanup;
> - } else {
> + }
> + else {
Change back to '} else {'
> int ii;
> - virNetworkForwardIfDefPtr dev = NULL;
> -
> +
> /* find the matching interface in the pool and increment its usageCount */
> -
> +
> for (ii = 0; ii < netdef->nForwardIfs; ii++) {
> if (STREQ(actualDev, netdef->forwardIfs[ii].dev)) {
> dev = &netdef->forwardIfs[ii];
> @@ -2954,7 +3006,7 @@ networkNotifyActualDevice(virDomainNetDefPtr iface)
> netdef->name, actualDev);
> goto cleanup;
> }
> -
> +
> /* PASSTHROUGH mode, and PRIVATE Mode + 802.1Qbh both require
> * exclusive access to a device, so current usageCount must be
> * 0 in those cases.
> @@ -3001,6 +3053,7 @@ networkReleaseActualDevice(virDomainNetDefPtr iface)
> virNetworkObjPtr network = NULL;
> virNetworkDefPtr netdef;
> const char *actualDev;
> + virNetworkForwardIfDefPtr dev = NULL;
> int ret = -1;
>
> if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK)
> @@ -3036,10 +3089,10 @@ networkReleaseActualDevice(virDomainNetDefPtr iface)
> _("network '%s' uses a direct mode, but has no forward dev and no interface pool"),
> netdef->name);
> goto cleanup;
> - } else {
> + }
> + else {
And here
> int ii;
> - virNetworkForwardIfDefPtr dev = NULL;
> -
> +
> for (ii = 0; ii < netdef->nForwardIfs; ii++) {
> if (STREQ(actualDev, netdef->forwardIfs[ii].dev)) {
> dev = &netdef->forwardIfs[ii];
> @@ -3053,7 +3106,7 @@ networkReleaseActualDevice(virDomainNetDefPtr iface)
> netdef->name, actualDev);
> goto cleanup;
> }
> -
> +
> dev->usageCount--;
> VIR_DEBUG("Releasing physical device %s, usageCount %d",
> dev->dev, dev->usageCount);
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list