[libvirt PATCH] virnetdevbridge: Ignore EEXIST when adding an entry to fdb

[Laine Stump]

On 4/30/21 12:22 PM, Jiri Denemark wrote:
> When updating entries in a bridge forwarding database (i.e., when
> macTableManager='libvirt' is configured for the bridge), we may end up
> in a situation when the entry we want to add is already present. Let's
> just ignore the error in such a case.
> 
> This fixes an error to resume a domain when fdb entries were not
> properly removed when the domain was paused:
> 
>      virsh # resume test
>      error: Failed to resume domain test
>      error: error adding fdb entry for vnet2: File exists
> 
> For some reason, fdb entries are only removed when libvirt explicitly
> stops CPUs, but nothing happens when we just get STOP event from QEMU.
> An alternative approach would be to make sure we always remove the
> entries regardless on why a domain was paused (e.g., during migration),

Yeah, somebody (/me stares off into distance while whistling) should 
look into that, since what you're doing here is just masking the root 
cause of your problem.

> but that would be a significantly more disruptive change with possible
> side effects.

Agreed, and I see no down side to what your patch does, even after we 
fix the root cause - after all, what if qemu exits while libvirtd is 
stopped? the FDB entry wouldn't get removed and your patch would end up 
being required anyway.

> 
> Signed-off-by: Jiri Denemark <jdenemar at redhat.com>

Reviewed-by: Laine Stump <laine at redhat.com>

It might be nice to mention https://bugzilla.redhat.com/1603155 (and 
also to note the commit ID of this patch in that BZ once it's pushed, as 
well as re-opening it, since it *is* being fixed).

> ---
>   src/util/virnetdevbridge.c | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c
> index 7b5ea4fe1d..4fe84cc162 100644
> --- a/src/util/virnetdevbridge.c
> +++ b/src/util/virnetdevbridge.c
> @@ -1063,9 +1063,13 @@ virNetDevBridgeFDBAddDel(const virMacAddr *mac, const char *ifname,
>           if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err)))
>               goto malformed_resp;
>           if (err->error) {
> -            virReportSystemError(-err->error,
> -                                 _("error adding fdb entry for %s"), ifname);
> -            return -1;
> +            if (isAdd && -err->error == EEXIST) {
> +                VIR_DEBUG("fdb entry for %s already exists", ifname);
> +            } else {
> +                virReportSystemError(-err->error,
> +                                     _("error adding fdb entry for %s"), ifname);
> +                return -1;
> +            }
>           }
>           break;
>       case NLMSG_DONE:
>