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

Re: [libvirt] [PATCHv2 4/5] network: change cleanup: to error: in network*() functions



Looks good.

Acked-by: Kyle Mestery <kmestery cisco com>

On Aug 14, 2012, at 2:10 AM, Laine Stump wrote:

> A later patch will be adding a counter that will be
> incremented/decremented each time an guest interface starts/stops
> using a particular network. For this to work, all types of networks
> need to go through a common return sequence rather than returning
> early. To setup for this, the existing cleanup: label is renamed to
> error:, a new cleanup: label is added (when necessary), and early
> returns are changed to goto cleanup (except the case where the
> interface type != NETWORK).
> ---
> src/network/bridge_driver.c | 106 ++++++++++++++++++++++----------------------
> 1 file changed, 53 insertions(+), 53 deletions(-)
> 
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index 77b38d2..680b3f3 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -2767,9 +2767,8 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
>         virReportError(VIR_ERR_NO_NETWORK,
>                        _("no network with matching name '%s'"),
>                        iface->data.network.name);
> -        goto cleanup;
> +        goto error;
>     }
> -
>     netdef = network->def;
> 
>     /* portgroup can be present for any type of network, in particular
> @@ -2786,12 +2785,12 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
>         if (!iface->data.network.actual
>             && (VIR_ALLOC(iface->data.network.actual) < 0)) {
>             virReportOOMError();
> -            goto cleanup;
> +            goto error;
>         }
> 
>         if (virNetDevBandwidthCopy(&iface->data.network.actual->bandwidth,
>                                    portgroup->bandwidth) < 0)
> -            goto cleanup;
> +            goto error;
>     }
> 
>     if ((netdef->forwardType == VIR_NETWORK_FORWARD_NONE) ||
> @@ -2813,14 +2812,14 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
>         if (!iface->data.network.actual
>             && (VIR_ALLOC(iface->data.network.actual) < 0)) {
>             virReportOOMError();
> -            goto cleanup;
> +            goto error;
>         }
> 
>         iface->data.network.actual->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
>         iface->data.network.actual->data.bridge.brname = strdup(netdef->bridge);
>         if (!iface->data.network.actual->data.bridge.brname) {
>             virReportOOMError();
> -            goto cleanup;
> +            goto error;
>         }
> 
>         /* merge virtualports from interface, network, and portgroup to
> @@ -2831,7 +2830,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
>                                         netdef->virtPortProfile,
>                                         portgroup
>                                         ? portgroup->virtPortProfile : NULL) < 0) {
> -            goto cleanup;
> +            goto error;
>         }
>         virtport = iface->data.network.actual->virtPortProfile;
>         if (virtport) {
> @@ -2842,7 +2841,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
>                                  "'%s' which uses a bridge device"),
>                                virNetDevVPortTypeToString(virtport->virtPortType),
>                                netdef->name);
> -                goto cleanup;
> +                goto error;
>             }
>         }
> 
> @@ -2858,7 +2857,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
>         if (!iface->data.network.actual
>             && (VIR_ALLOC(iface->data.network.actual) < 0)) {
>             virReportOOMError();
> -            goto cleanup;
> +            goto error;
>         }
> 
>         /* Set type=direct and appropriate <source mode='xxx'/> */
> @@ -2886,7 +2885,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
>                                         netdef->virtPortProfile,
>                                         portgroup
>                                         ? portgroup->virtPortProfile : NULL) < 0) {
> -            goto cleanup;
> +            goto error;
>         }
>         virtport = iface->data.network.actual->virtPortProfile;
>         if (virtport) {
> @@ -2898,7 +2897,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
>                                  "'%s' which uses a macvtap device"),
>                                virNetDevVPortTypeToString(virtport->virtPortType),
>                                netdef->name);
> -                goto cleanup;
> +                goto error;
>             }
>         }
> 
> @@ -2910,7 +2909,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
>                            _("network '%s' uses a direct mode, but "
>                              "has no forward dev and no interface pool"),
>                            netdef->name);
> -            goto cleanup;
> +            goto error;
>         } else {
>             /* pick an interface from the pool */
> 
> @@ -2927,19 +2926,19 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
>                         virReportError(VIR_ERR_INTERNAL_ERROR,
>                                        _("Could not get Virtual functions on %s"),
>                                        netdef->forwardPfs->dev);
> -                        goto cleanup;
> +                        goto error;
>                     }
> 
>                     if (num_virt_fns == 0) {
>                         virReportError(VIR_ERR_INTERNAL_ERROR,
>                                        _("No Vf's present on SRIOV PF %s"),
>                                        netdef->forwardPfs->dev);
> -                        goto cleanup;
> +                        goto error;
>                     }
> 
>                     if ((VIR_ALLOC_N(netdef->forwardIfs, num_virt_fns)) < 0) {
>                         virReportOOMError();
> -                        goto cleanup;
> +                        goto error;
>                     }
> 
>                     netdef->nForwardIfs = num_virt_fns;
> @@ -2948,7 +2947,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
>                         netdef->forwardIfs[ii].dev = strdup(vfname[ii]);
>                         if (!netdef->forwardIfs[ii].dev) {
>                             virReportOOMError();
> -                            goto cleanup;
> +                            goto error;
>                         }
>                     }
>                 }
> @@ -2987,18 +2986,18 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
>                                _("network '%s' requires exclusive access "
>                                  "to interfaces, but none are available"),
>                                netdef->name);
> -                goto cleanup;
> +                goto error;
>             }
>             iface->data.network.actual->data.direct.linkdev = strdup(dev->dev);
>             if (!iface->data.network.actual->data.direct.linkdev) {
>                 virReportOOMError();
> -                goto cleanup;
> +                goto error;
>             }
>         }
>     }
> 
>     if (virNetDevVPortProfileCheckComplete(virtport, true) < 0)
> -        goto cleanup;
> +        goto error;
> 
>     if (dev) {
>         /* we are now assured of success, so mark the allocation */
> @@ -3007,7 +3006,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
>                   dev->dev, dev->connections);
>     }
>     ret = 0;
> -cleanup:
> +error:
>     for (ii = 0; ii < num_virt_fns; ii++)
>         VIR_FREE(vfname[ii]);
>     VIR_FREE(vfname);
> @@ -3042,12 +3041,6 @@ networkNotifyActualDevice(virDomainNetDefPtr iface)
>     if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK)
>        return 0;
> 
> -    if (!iface->data.network.actual ||
> -        (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_DIRECT)) {
> -        VIR_DEBUG("Nothing to claim from network %s", iface->data.network.name);
> -        return 0;
> -    }
> -
>     networkDriverLock(driver);
>     network = virNetworkFindByName(&driver->networks, iface->data.network.name);
>     networkDriverUnlock(driver);
> @@ -3055,6 +3048,13 @@ networkNotifyActualDevice(virDomainNetDefPtr iface)
>         virReportError(VIR_ERR_NO_NETWORK,
>                        _("no network with matching name '%s'"),
>                        iface->data.network.name);
> +        goto error;
> +    }
> +    netdef = network->def;
> +
> +    if (!iface->data.network.actual ||
> +        (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_DIRECT)) {
> +        VIR_DEBUG("Nothing to claim from network %s", iface->data.network.name);
>         goto cleanup;
>     }
> 
> @@ -3063,16 +3063,15 @@ networkNotifyActualDevice(virDomainNetDefPtr iface)
>         virReportError(VIR_ERR_INTERNAL_ERROR,
>                        "%s", _("the interface uses a direct "
>                                "mode, but has no source dev"));
> -        goto cleanup;
> +        goto error;
>     }
> 
> -    netdef = network->def;
>     if (netdef->nForwardIfs == 0) {
>         virReportError(VIR_ERR_INTERNAL_ERROR,
>                        _("network '%s' uses a direct mode, but "
>                          "has no forward dev and no interface pool"),
>                        netdef->name);
> -        goto cleanup;
> +        goto error;
>     } else {
>         int ii;
>         virNetworkForwardIfDefPtr dev = NULL;
> @@ -3090,7 +3089,7 @@ networkNotifyActualDevice(virDomainNetDefPtr iface)
>             virReportError(VIR_ERR_INTERNAL_ERROR,
>                            _("network '%s' doesn't have dev='%s' in use by domain"),
>                            netdef->name, actualDev);
> -            goto cleanup;
> +            goto error;
>         }
> 
>         /* PASSTHROUGH mode, and PRIVATE Mode + 802.1Qbh both require
> @@ -3106,7 +3105,7 @@ networkNotifyActualDevice(virDomainNetDefPtr iface)
>             virReportError(VIR_ERR_INTERNAL_ERROR,
>                            _("network '%s' claims dev='%s' is already in use by a different domain"),
>                            netdef->name, actualDev);
> -            goto cleanup;
> +            goto error;
>         }
>         /* we are now assured of success, so mark the allocation */
>         dev->connections++;
> @@ -3114,8 +3113,9 @@ networkNotifyActualDevice(virDomainNetDefPtr iface)
>                   dev->dev, dev->connections);
>     }
> 
> -    ret = 0;
> cleanup:
> +    ret = 0;
> +error:
>     if (network)
>         virNetworkObjUnlock(network);
>     return ret;
> @@ -3136,7 +3136,7 @@ int
> networkReleaseActualDevice(virDomainNetDefPtr iface)
> {
>     struct network_driver *driver = driverState;
> -    virNetworkObjPtr network = NULL;
> +    virNetworkObjPtr network;
>     virNetworkDefPtr netdef;
>     const char *actualDev;
>     int ret = -1;
> @@ -3144,13 +3144,6 @@ networkReleaseActualDevice(virDomainNetDefPtr iface)
>     if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK)
>        return 0;
> 
> -    if (!iface->data.network.actual ||
> -        (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_DIRECT)) {
> -        VIR_DEBUG("Nothing to release to network %s", iface->data.network.name);
> -        ret = 0;
> -        goto cleanup;
> -    }
> -
>     networkDriverLock(driver);
>     network = virNetworkFindByName(&driver->networks, iface->data.network.name);
>     networkDriverUnlock(driver);
> @@ -3158,6 +3151,13 @@ networkReleaseActualDevice(virDomainNetDefPtr iface)
>         virReportError(VIR_ERR_NO_NETWORK,
>                        _("no network with matching name '%s'"),
>                        iface->data.network.name);
> +        goto error;
> +    }
> +    netdef = network->def;
> +
> +    if (!iface->data.network.actual ||
> +        (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_DIRECT)) {
> +        VIR_DEBUG("Nothing to release to network %s", iface->data.network.name);
>         goto cleanup;
>     }
> 
> @@ -3166,16 +3166,15 @@ networkReleaseActualDevice(virDomainNetDefPtr iface)
>         virReportError(VIR_ERR_INTERNAL_ERROR,
>                        "%s", _("the interface uses a direct "
>                                "mode, but has no source dev"));
> -        goto cleanup;
> +        goto error;
>     }
> 
> -    netdef = network->def;
>     if (netdef->nForwardIfs == 0) {
>         virReportError(VIR_ERR_INTERNAL_ERROR,
>                        _("network '%s' uses a direct mode, but "
>                          "has no forward dev and no interface pool"),
>                        netdef->name);
> -        goto cleanup;
> +        goto error;
>     } else {
>         int ii;
>         virNetworkForwardIfDefPtr dev = NULL;
> @@ -3191,7 +3190,7 @@ networkReleaseActualDevice(virDomainNetDefPtr iface)
>             virReportError(VIR_ERR_INTERNAL_ERROR,
>                            _("network '%s' doesn't have dev='%s' in use by domain"),
>                            netdef->name, actualDev);
> -            goto cleanup;
> +            goto error;
>         }
> 
>         dev->connections--;
> @@ -3199,8 +3198,9 @@ networkReleaseActualDevice(virDomainNetDefPtr iface)
>                   dev->dev, dev->connections);
>     }
> 
> -    ret = 0;
> cleanup:
> +    ret = 0;
> +error:
>     if (network)
>         virNetworkObjUnlock(network);
>     virDomainActualNetDefFree(iface->data.network.actual);
> @@ -3232,7 +3232,7 @@ networkGetNetworkAddress(const char *netname, char **netaddr)
> {
>     int ret = -1;
>     struct network_driver *driver = driverState;
> -    virNetworkObjPtr network = NULL;
> +    virNetworkObjPtr network;
>     virNetworkDefPtr netdef;
>     virNetworkIpDefPtr ipdef;
>     virSocketAddr addr;
> @@ -3247,7 +3247,7 @@ networkGetNetworkAddress(const char *netname, char **netaddr)
>         virReportError(VIR_ERR_NO_NETWORK,
>                        _("no network with matching name '%s'"),
>                        netname);
> -        goto cleanup;
> +        goto error;
>     }
>     netdef = network->def;
> 
> @@ -3289,17 +3289,17 @@ networkGetNetworkAddress(const char *netname, char **netaddr)
> 
>     if (dev_name) {
>         if (virNetDevGetIPv4Address(dev_name, &addr) < 0)
> -            goto cleanup;
> -
> +            goto error;
>         addrptr = &addr;
>     }
> 
> -    if (addrptr &&
> -        (*netaddr = virSocketAddrFormat(addrptr))) {
> -        ret = 0;
> +    if (!(addrptr &&
> +          (*netaddr = virSocketAddrFormat(addrptr)))) {
> +        goto error;
>     }
> 
> -cleanup:
> +    ret = 0;
> +error:
>     if (network)
>         virNetworkObjUnlock(network);
>     return ret;
> -- 
> 1.7.11.2
> 
> --
> 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]