[PATCH 0/2] network: avoid logging unnecessary and misleading errors when failing to unset a zone

Laine Stump posted 2 patches 6 months, 3 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20241022023243.235120-1-laine@redhat.com
src/network/bridge_driver.c          |  8 +++----
src/network/bridge_driver_linux.c    | 10 +++++----
src/network/bridge_driver_nop.c      |  4 +++-
src/network/bridge_driver_platform.h |  3 ++-
src/util/virfirewalld.c              | 33 ++++++++++++++++++----------
src/util/virfirewalld.h              |  2 +-
6 files changed, 38 insertions(+), 22 deletions(-)
[PATCH 0/2] network: avoid logging unnecessary and misleading errors when failing to unset a zone
Posted by Laine Stump 6 months, 3 weeks ago
While testing a recent patch that unsets the zone of bridge interfaces
when a virtual network is stopped, a side effect was noticed: when
firewalld reloaded its rules, this would result in an error log from
libvirt complaining about attempting to unset the zone of an interface
that wasn't in any zone. The two patches here fix that from different
angles:

* The first modifies the call to unsetZone so that it puts any error
  message returned from firewalld to libvirt into a virError object
  rather than logging it; this virError object is then silently
  discarded.

* The second avoids even calling firewalld to unset the zone if it's
  just going to immediately be set again. This avoids an error message
  that would be logged directly by firewalld even if libvirt didn't
  log the message it received from firewalld.

The combination of these two patches eliminate all misleading log
messages about failed attempts to unset a zone.

Laine Stump (2):
  network: ignore/don't log errors when unsetting firewalld zone
  network: don't unset the firewalld zone if it's going to be
    immediately re-set

 src/network/bridge_driver.c          |  8 +++----
 src/network/bridge_driver_linux.c    | 10 +++++----
 src/network/bridge_driver_nop.c      |  4 +++-
 src/network/bridge_driver_platform.h |  3 ++-
 src/util/virfirewalld.c              | 33 ++++++++++++++++++----------
 src/util/virfirewalld.h              |  2 +-
 6 files changed, 38 insertions(+), 22 deletions(-)

-- 
2.47.0
Re: [PATCH 0/2] network: avoid logging unnecessary and misleading errors when failing to unset a zone
Posted by Ján Tomko 6 months, 2 weeks ago
On a Monday in 2024, Laine Stump wrote:
>While testing a recent patch that unsets the zone of bridge interfaces
>when a virtual network is stopped, a side effect was noticed: when
>firewalld reloaded its rules, this would result in an error log from
>libvirt complaining about attempting to unset the zone of an interface
>that wasn't in any zone. The two patches here fix that from different
>angles:
>
>* The first modifies the call to unsetZone so that it puts any error
>  message returned from firewalld to libvirt into a virError object
>  rather than logging it; this virError object is then silently
>  discarded.
>
>* The second avoids even calling firewalld to unset the zone if it's
>  just going to immediately be set again. This avoids an error message
>  that would be logged directly by firewalld even if libvirt didn't
>  log the message it received from firewalld.
>
>The combination of these two patches eliminate all misleading log
>messages about failed attempts to unset a zone.
>
>Laine Stump (2):
>  network: ignore/don't log errors when unsetting firewalld zone
>  network: don't unset the firewalld zone if it's going to be
>    immediately re-set
>
> src/network/bridge_driver.c          |  8 +++----
> src/network/bridge_driver_linux.c    | 10 +++++----
> src/network/bridge_driver_nop.c      |  4 +++-
> src/network/bridge_driver_platform.h |  3 ++-
> src/util/virfirewalld.c              | 33 ++++++++++++++++++----------
> src/util/virfirewalld.h              |  2 +-
> 6 files changed, 38 insertions(+), 22 deletions(-)
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano