[libvirt PATCH] virnetdevbridge: Ignore EEXIST when adding an entry to fdb
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), but that would be a significantly more disruptive change with possible side effects. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- 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: -- 2.31.1
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@redhat.com>
Reviewed-by: Laine Stump <laine@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:
participants (2)
-
Jiri Denemark -
Laine Stump