Re: [PATCH v3 09/36] network: move re-attach of bridge device out of network driver

[Laine Stump <laine@xxxxxxxxx>]

On 3/19/19 8:46 AM, Daniel P. Berrangé wrote:
> During initial NIC setup the hypervisor drivers are responsible for
> attaching the TAP device to the bridge device. Any fixup after libvirtd
> restarts should thus also be their responsibility.
>
> Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>


*And* this fixes a bug that someone reported on IRC awhile back (and 
does it in the correct way, rather than the stupid f'ed up way I would 
have done it if I hadn't completely forgot about it until just this 
instant :-P)


Reviewed-by: Laine Stump <laine@xxxxxxxxx>


> ---
>   src/conf/domain_conf.c      | 20 +++++++++++++++++++-
>   src/conf/domain_conf.h      |  2 +-
>   src/network/bridge_driver.c | 27 ++++++---------------------
>   3 files changed, 26 insertions(+), 23 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index bcd997a705..99b75e26f3 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -54,6 +54,7 @@
>   #include "virsecret.h"
>   #include "virstring.h"
>   #include "virnetdev.h"
> +#include "virnetdevtap.h"
>   #include "virnetdevmacvlan.h"
>   #include "virhostdev.h"
>   #include "virmdev.h"
> @@ -30217,8 +30218,25 @@ virDomainNetNotifyActualDevice(virConnectPtr conn,
>       if (!(net = virNetworkLookupByName(conn, iface->data.network.name)))
>           return;
>   
> -    netNotify(net, dom, iface);
> +    if (netNotify(net, dom, iface) < 0)
> +        goto cleanup;
> +
> +    if (virDomainNetGetActualType(iface) == VIR_DOMAIN_NET_TYPE_BRIDGE) {
> +        /*
> +         * NB: we can't notify the guest of any MTU change anyway,
> +         * so there is no point in trying to learn the actualMTU
> +         * (final arg to virNetDevTapReattachBridge())
> +         */
> +        if (virNetDevTapReattachBridge(iface->ifname,
> +                                       iface->data.network.actual->data.bridge.brname,
> +                                       &iface->mac, dom->uuid,
> +                                       virDomainNetGetActualVirtPortProfile(iface),
> +                                       virDomainNetGetActualVlan(iface),
> +                                       iface->mtu, NULL) < 0)
> +            goto cleanup;
> +    }
>   
> + cleanup:
>       virObjectUnref(net);
>   }
>   
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 3db319c23d..30aa985344 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -3515,7 +3515,7 @@ typedef int
>                                           virDomainDefPtr dom,
>                                           virDomainNetDefPtr iface);
>   
> -typedef void
> +typedef int
>   (*virDomainNetNotifyActualDeviceImpl)(virNetworkPtr net,
>                                         virDomainDefPtr dom,
>                                         virDomainNetDefPtr iface);
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index 7df0c0f67a..5f3371b150 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -4776,12 +4776,11 @@ networkAllocateActualDevice(virNetworkPtr net,
>    * Called to notify the network driver when libvirtd is restarted and
>    * finds an already running domain. If appropriate it will force an
>    * allocation of the actual->direct.linkdev to get everything back in
> - * order, or re-attach the interface's tap device to the network's
> - * bridge.
> + * order.
>    *
> - * No return value (but does log any failures)
> + * Returns 0 on success, -1 on failure.
>    */
> -static void
> +static int
>   networkNotifyActualDevice(virNetworkPtr net,
>                             virDomainDefPtr dom,
>                             virDomainNetDefPtr iface)
> @@ -4792,6 +4791,7 @@ networkNotifyActualDevice(virNetworkPtr net,
>       virNetworkDefPtr netdef;
>       virNetworkForwardIfDefPtr dev = NULL;
>       size_t i;
> +    int ret = -1;
>   
>       obj = virNetworkObjFindByName(driver->networks, net->name);
>       if (!obj) {
> @@ -4836,22 +4836,6 @@ networkNotifyActualDevice(virNetworkPtr net,
>           actualType = VIR_DOMAIN_NET_TYPE_BRIDGE;
>       }
>   
> -    /* see if we're connected to the correct bridge */
> -    if (netdef->bridge) {
> -        /*
> -         * NB: we can't notify the guest of any MTU change anyway,
> -         * so there is no point in trying to learn the actualMTU
> -         * (final arg to virNetDevTapReattachBridge())
> -         */
> -        if (virNetDevTapReattachBridge(iface->ifname, netdef->bridge,
> -                                       &iface->mac, dom->uuid,
> -                                       virDomainNetGetActualVirtPortProfile(iface),
> -                                       virDomainNetGetActualVlan(iface),
> -                                       iface->mtu, NULL) < 0) {
> -            goto error;
> -        }
> -    }
> -
>       if (!iface->data.network.actual ||
>           (actualType != VIR_DOMAIN_NET_TYPE_DIRECT &&
>            actualType != VIR_DOMAIN_NET_TYPE_HOSTDEV)) {
> @@ -4980,10 +4964,11 @@ networkNotifyActualDevice(virNetworkPtr net,
>           goto error;
>       }
>       networkLogAllocation(netdef, actualType, dev, iface, true);
> +    ret = 0;
>   
>    cleanup:
>       virNetworkObjEndAPI(&obj);
> -    return;
> +    return ret;
>   
>    error:
>       goto cleanup;


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