[libvirt] [PATCHv3] replace 'if' type conditions with 'switch' for VIR_NETWORK_FORWARD_

Erik Skultety eskultet at redhat.com
Wed Jul 25 12:37:24 UTC 2018


On Tue, Jul 24, 2018 at 11:49:48AM +0800, Shi Lei wrote:
> Signed-off-by: Shi Lei <shilei.massclouds at gmx.com>
> ---
>
> v2 here: https://www.redhat.com/archives/libvir-list/2018-July/msg01423.html
> since v2:
>  - typecast def->forward.type to virNetworkForwardType explicitly
>   in all the switches rather than change its type to enum in
>   the struct definition
>
> v1 here: https://www.redhat.com/archives/libvir-list/2018-July/msg01314.html
> since v1:
>  - Change the type declaration of _virNetworkForwardDef.type
>   from int to virNetworkForwardType
>  - use the default case to report out of range error with
>   virReportEnumRangeError
>
...

> @@ -2128,20 +2150,37 @@ networkReloadFirewallRulesHelper(virNetworkObjPtr obj,
>
>      virObjectLock(obj);
>      def = virNetworkObjGetDef(obj);
> -    if (virNetworkObjIsActive(obj) &&
> -        ((def->forward.type == VIR_NETWORK_FORWARD_NONE) ||
> -         (def->forward.type == VIR_NETWORK_FORWARD_NAT) ||
> -         (def->forward.type == VIR_NETWORK_FORWARD_ROUTE))) {
> -        /* Only three of the L3 network types that are configured by
> -         * libvirt need to have iptables rules reloaded. The 4th L3
> -         * network type, forward='open', doesn't need this because it
> -         * has no iptables rules.
> -         */
> -        networkRemoveFirewallRules(def);
> -        if (networkAddFirewallRules(def) < 0) {
> -            /* failed to add but already logged */
> +    if (virNetworkObjIsActive(obj)) {
> +        switch ((virNetworkForwardType) def->forward.type) {
> +        case VIR_NETWORK_FORWARD_NONE:
> +        case VIR_NETWORK_FORWARD_NAT:
> +        case VIR_NETWORK_FORWARD_ROUTE:
> +            /* Only three of the L3 network types that are configured by
> +             * libvirt need to have iptables rules reloaded. The 4th L3
> +             * network type, forward='open', doesn't need this because it
> +             * has no iptables rules.
> +             */
> +            networkRemoveFirewallRules(def);
> +            /* No need to check return value since already logged internally */

I dropped ^this commentary, adjusted the commit message and pushed the patch.

Congratulations on your first libvirt patch.
Erik




More information about the libvir-list mailing list