Re: [PATCH v3 11/36] network: unconditionally merge port profiles

[Laine Stump <laine@xxxxxxxxx>]

On 3/19/19 8:46 AM, Daniel P. Berrangé wrote:
> All but one of the network types supports port profiles. Rather than
> duplicating the code to merge profiles 3 times, do it once and then
> later report an error if used from the wrong place.
>
> Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>


Reviewed-by: Laine Stump <laine@xxxxxxxxx>


> ---
>   src/network/bridge_driver.c | 54 +++++++++++++++----------------------
>   1 file changed, 21 insertions(+), 33 deletions(-)
>
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index a1ae90a34c..45a45b95d7 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -4449,6 +4449,18 @@ networkAllocateActualDevice(virNetworkPtr net,
>          iface->data.network.actual->trustGuestRxFilters
>             = netdef->trustGuestRxFilters;
>   
> +    /* merge virtualports from interface, network, and portgroup to
> +     * arrive at actual virtualport to use
> +     */
> +    if (virNetDevVPortProfileMerge3(&iface->data.network.actual->virtPortProfile,
> +                                    iface->virtPortProfile,
> +                                    netdef->virtPortProfile,
> +                                    portgroup
> +                                    ? portgroup->virtPortProfile : NULL) < 0) {
> +        goto error;
> +    }
> +    virtport = iface->data.network.actual->virtPortProfile;
> +
>       switch ((virNetworkForwardType) netdef->forward.type) {
>       case VIR_NETWORK_FORWARD_NONE:
>       case VIR_NETWORK_FORWARD_NAT:
> @@ -4467,6 +4479,15 @@ networkAllocateActualDevice(virNetworkPtr net,
>           iface->data.network.actual->data.bridge.macTableManager
>              = netdef->macTableManager;
>   
> +        if (virtport) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("<virtualport type='%s'> not supported for network "
> +                             "'%s' which uses IP forwarding"),
> +                           virNetDevVPortTypeToString(virtport->virtPortType),
> +                           netdef->name);
> +            goto error;
> +        }
> +
>           if (networkPlugBandwidth(obj, iface) < 0)
>               goto error;
>           break;
> @@ -4519,17 +4540,6 @@ networkAllocateActualDevice(virNetworkPtr net,
>           iface->data.network.actual->data.hostdev.def.source.subsys.u.pci.backend
>               = backend;
>   
> -        /* merge virtualports from interface, network, and portgroup to
> -         * arrive at actual virtualport to use
> -         */
> -        if (virNetDevVPortProfileMerge3(&iface->data.network.actual->virtPortProfile,
> -                                        iface->virtPortProfile,
> -                                        netdef->virtPortProfile,
> -                                        portgroup
> -                                        ? portgroup->virtPortProfile : NULL) < 0) {
> -            goto error;
> -        }
> -        virtport = iface->data.network.actual->virtPortProfile;
>           if (virtport) {
>               /* make sure type is supported for hostdev connections */
>               if (virtport->virtPortType != VIR_NETDEV_VPORT_PROFILE_8021QBG &&
> @@ -4559,17 +4569,6 @@ networkAllocateActualDevice(virNetworkPtr net,
>               iface->data.network.actual->data.bridge.macTableManager
>                  = netdef->macTableManager;
>   
> -            /* merge virtualports from interface, network, and portgroup to
> -             * arrive at actual virtualport to use
> -             */
> -            if (virNetDevVPortProfileMerge3(&iface->data.network.actual->virtPortProfile,
> -                                            iface->virtPortProfile,
> -                                            netdef->virtPortProfile,
> -                                            portgroup
> -                                            ? portgroup->virtPortProfile : NULL) < 0) {
> -                goto error;
> -            }
> -            virtport = iface->data.network.actual->virtPortProfile;
>               if (virtport) {
>                   /* only type='openvswitch' is allowed for bridges */
>                   if (virtport->virtPortType != VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) {
> @@ -4608,17 +4607,6 @@ networkAllocateActualDevice(virNetworkPtr net,
>           iface->data.network.actual->data.direct.mode =
>               virNetDevMacVLanModeTypeFromString(virNetworkForwardTypeToString(netdef->forward.type));
>   
> -        /* merge virtualports from interface, network, and portgroup to
> -         * arrive at actual virtualport to use
> -         */
> -        if (virNetDevVPortProfileMerge3(&iface->data.network.actual->virtPortProfile,
> -                                        iface->virtPortProfile,
> -                                        netdef->virtPortProfile,
> -                                        portgroup
> -                                        ? portgroup->virtPortProfile : NULL) < 0) {
> -            goto error;
> -        }
> -        virtport = iface->data.network.actual->virtPortProfile;
>           if (virtport) {
>               /* make sure type is supported for macvtap connections */
>               if (virtport->virtPortType != VIR_NETDEV_VPORT_PROFILE_8021QBG &&


--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list