[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 3/5] util: new function virFirewallDInterfaceSetZone()

On 1/15/19 12:19 PM, John Ferlan wrote:
or more simply "util: Introduce virFirewallDInterfaceSetZone"

On 1/9/19 9:57 PM, Laine Stump wrote:
Sets the firewalld zone of the given interface. This function assumes
that you've already called virFirewallDIsActive(), and relies on
virDBusCallMethod's standard error reporting to log any errors.

Signed-off-by: Laine Stump <laine laine org>
  src/libvirt_private.syms |  1 +
  src/util/virfirewalld.c  | 23 +++++++++++++++++++++++
  src/util/virfirewalld.h  |  3 +++
  3 files changed, 27 insertions(+)

Caveat - not my area of knowledge, but from a learning PoV for me...

Any concerns over with this is a "compute intensive" type operation for
firewalld and whether it's felt it would be useful to getZoneOfInterface
first and compare vs. the passed zone before calling the

Any time we try to set a zone that doesn't exist, there should be an error message, so we can either go to the extra trouble to figure out if the zone setting will fail beforehand, or we can just try it.

The one exception is in the case of setting the zone to "libvirt", but I think I've covered that in my response to Daniel's review of 4/5

  My thoughts here are along the lines of other
(somewhat) recent upstream patches related to performance issues that
I've seen from Nikolay and Dan.

Well, getting a list of zones is something that only needs to be done once for each time firewalld or libvirtd is restarted, since zones can't be added/removed without restarting firewalld. I suppose attempting to set the zone multiple times when it doesn't exist could net a performance penalty, but on the other hand that would only be temporary until firewalld was updated.

But again, there's a proposal in my response to Daniel's review of 4/5 that would eliminate that as well.

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]