[libvirt] [PATCH v4 04/29] network: use 'bridge' as actual type instead of 'network'

[Michal Privoznik]

On 4/17/19 7:19 PM, Daniel P. Berrangé wrote:
> Ports allocated on virtual networks with type=nat|route|open all get
> given an actual type of 'network'.
> 
> Only ports in networks with type=bridge use an actual type of 'bridge'.
> 
> This distinction makes little sense since the virtualization drivers
> will treat both actual types in exactly the same way, as they're all
> just bridge devices a VM needs to be connected to.
> 
> This doesn't affect user visible XML since the "actual" device XML
> is internal only, but we need code to convert the data upgrades.
> 
> Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
> ---
>   src/conf/domain_conf.c      | 28 +++++++++++++++++++++++-----
>   src/network/bridge_driver.c | 11 +++--------
>   src/qemu/qemu_driver.c      |  2 +-
>   3 files changed, 27 insertions(+), 14 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 0df3c2ed49..1dde45a6fd 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -5081,6 +5081,19 @@ virDomainDiskDefPostParse(virDomainDiskDefPtr disk,
>           return -1;
>       }
>   
> +    /* Older libvirtd uses actualType==network, but we now
> +     * just use actualType==bridge, as nothing needs to
> +     * distinguish the two cases, and this simplifies virt
> +     * drive code */
> +    if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
> +        net->data.network.actual != NULL  &&
> +        net->data.network.actual->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
> +        char mac[VIR_MAC_STRING_BUFLEN];
> +        virMacAddrFormat(&net->mac, mac);
> +        VIR_DEBUG("Updating NIC %s actual type to bridge", mac);
> +        net->data.network.actual->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
> +    }
> +
>       return 0;
>   }
>   
> @@ -11267,11 +11280,16 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
>       }
>   
>       bandwidth_node = virXPathNode("./bandwidth", ctxt);
> -    if (bandwidth_node &&
> -        virNetDevBandwidthParse(&actual->bandwidth,
> -                                bandwidth_node,
> -                                def->type == VIR_DOMAIN_NET_TYPE_NETWORK) < 0)
> -        goto error;
> +    if (bandwidth_node) {
> +        bool allowFloor =
> +            (actual->type == VIR_DOMAIN_NET_TYPE_NETWORK) ||
> +            (actual->type == VIR_DOMAIN_NET_TYPE_BRIDGE &&
> +             actual->data.bridge.brname != NULL);
> +        if (virNetDevBandwidthParse(&actual->bandwidth,
> +                                    bandwidth_node,
> +                                    allowFloor) < 0)
> +            goto error;
> +    }
>   
>       vlanNode = virXPathNode("./vlan", ctxt);
>       if (vlanNode && virNetDevVlanParse(vlanNode, ctxt, &actual->vlan) < 0)
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index 6ed0bf1e8e..4055939ade 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -4491,11 +4491,7 @@ networkAllocateActualDevice(virNetworkPtr net,
>       case VIR_NETWORK_FORWARD_NAT:
>       case VIR_NETWORK_FORWARD_ROUTE:
>       case VIR_NETWORK_FORWARD_OPEN:
> -        /* for these forward types, the actual net type really *is*
> -         * NETWORK; we just keep the info from the portgroup in
> -         * iface->data.network.actual
> -         */
> -        iface->data.network.actual->type = VIR_DOMAIN_NET_TYPE_NETWORK;
> +        iface->data.network.actual->type = VIR_DOMAIN_NET_TYPE_BRIDGE;

Actually, this breaks a lot of stuff. Firstly, for a live XML the type 
is changed to <interface type='bridge'> .. </interface>. This is because 
virDomainNetDefFormat() decides to format actual info. Having said that, 
transitionally some APIs are broken too. For instance, I have the 
following interface for my domain (inactive XML):

     <interface type='network' trustGuestRxFilters='yes'>
       <mac address='52:54:00:a4:6f:91'/>
       <source network='default'/>
       <bandwidth>
         <inbound average='1024' peak='4096' floor='500' burst='1024'/>
         <outbound average='10240'/>
       </bandwidth>
       <model type='virtio'/>
       <mtu size='9000'/>
     </interface>

Now, 'virsh domif-setlink', 'virsh update-device', 'virsh domiftune' 
don't work, because at runtime type is changed to 'bridge' and we don't 
implement many features for that network type.

I'll post patches for the first two issues I've found. But they look 
pretty hacky and I'm not sure my testing was exhaustive enough to find 
other cases where this will break. The third one I have no idea how to 
fix because it boils down to looking a network in the bridge driver but 
we don't know the network name because we don't parse it for interface 
type bridge.

Should we revert this?

Michal