[libvirt] [PATCH v3 10/36] network: move fixup for domain actual net def out of network driver

[Laine Stump]

On 3/19/19 8:46 AM, Daniel P. Berrangé wrote:
> The hypervisor drivers are soon going to communicate with the network
> driver via public APIs only. As such the network driver will not ever
> see the domain actual network def. Thus the backwards compatibility
> fixup logic must be moved out of the network driver.
>
> Signed-off-by: Daniel P. Berrangé <berrange at redhat.com
> ---
>   src/conf/domain_conf.c      | 25 +++++++++++++++++++++++++
>   src/network/bridge_driver.c | 20 --------------------
>   2 files changed, 25 insertions(+), 20 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 99b75e26f3..35d965d2a3 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -30210,6 +30210,7 @@ virDomainNetNotifyActualDevice(virConnectPtr conn,
>                                  virDomainDefPtr dom,
>                                  virDomainNetDefPtr iface)
>   {
> +    virDomainNetType actualType = virDomainNetGetActualType(iface);
>       virNetworkPtr net = NULL;
>   
>       if (!netNotify)
> @@ -30218,6 +30219,30 @@ virDomainNetNotifyActualDevice(virConnectPtr conn,
>       if (!(net = virNetworkLookupByName(conn, iface->data.network.name)))
>           return;
>   
> +    /* if we're restarting libvirtd after an upgrade from a version
> +     * that didn't save bridge name in actualNetDef for
> +     * actualType==network, we need to copy it in so that it will be
> +     * available in all cases
> +     */
> +    if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK &&
> +        !iface->data.network.actual->data.bridge.brname) {
> +        char *bridge = virNetworkGetBridgeName(net);
> +        if (!bridge)
> +            goto cleanup;
> +        VIR_FREE(iface->data.network.actual->data.bridge.brname);
> +        iface->data.network.actual->data.bridge.brname = bridge;
> +    }
> +


The above code was added in libvirt 1.2.11 in December 2014. It seems 
nearly impossible that someone would be upgrading from a libvirt that 
was 1.2.10 or older straight to libvirt-5.2.0 *without rebooting their 
host*. For this reason, I think the above code can/should be retired.


Reviewed-by: Laine Stump <laine at laine.org>


(either way, whether you decide to leave that code in or not.)


> +    /* 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 (actualType == VIR_DOMAIN_NET_TYPE_NETWORK) {
> +        iface->data.network.actual->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
> +        actualType = VIR_DOMAIN_NET_TYPE_BRIDGE;
> +    }
> +
> +
>       if (netNotify(net, dom, iface) < 0)
>           goto cleanup;
>   
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index 5f3371b150..a1ae90a34c 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -4816,26 +4816,6 @@ networkNotifyActualDevice(virNetworkPtr net,
>           goto error;
>       }
>   
> -    /* if we're restarting libvirtd after an upgrade from a version
> -     * that didn't save bridge name in actualNetDef for
> -     * actualType==network, we need to copy it in so that it will be
> -     * available in all cases
> -     */
> -    if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK &&
> -        !iface->data.network.actual->data.bridge.brname &&
> -        (VIR_STRDUP(iface->data.network.actual->data.bridge.brname,
> -                    netdef->bridge) < 0))
> -            goto error;
> -
> -    /* 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 (actualType == VIR_DOMAIN_NET_TYPE_NETWORK) {
> -        iface->data.network.actual->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
> -        actualType = VIR_DOMAIN_NET_TYPE_BRIDGE;
> -    }
> -
>       if (!iface->data.network.actual ||
>           (actualType != VIR_DOMAIN_NET_TYPE_DIRECT &&
>            actualType != VIR_DOMAIN_NET_TYPE_HOSTDEV)) {