On 1/9/19 9:57 PM, Laine Stump wrote:
Since I'm going to be adding at least one more firewalld-specific function, this seems like a good time to separate the code that's unique to firewalld from the more-generic "firewall" file.
Signed-off-by: Laine Stump <laine@laine.org> --- include/libvirt/virterror.h | 1 + src/libvirt_private.syms | 3 + src/util/Makefile.inc.am | 2 + src/util/virerror.c | 1 + src/util/virfirewall.c | 86 ++---------------------- src/util/virfirewalld.c | 128 ++++++++++++++++++++++++++++++++++++ src/util/virfirewalld.h | 33 ++++++++++ src/util/virfirewallpriv.h | 2 - tests/virfirewalltest.c | 1 + 9 files changed, 173 insertions(+), 84 deletions(-) create mode 100644 src/util/virfirewalld.c create mode 100644 src/util/virfirewalld.h
[...] I was also looking as a learning opportunity, but I see Dan is reviewing as well... Still I'll point out what I have in any case. oddly enough I had similar thoughts regarding that moved hunk with the (error.level == VIR_ERR_ERROR) checking...
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c3d6306809..583868f422 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1918,6 +1918,9 @@ virFirewallSetLockOverride; virFirewallStartRollback; virFirewallStartTransaction;
+# util/virfirewalld.h +virFirewallDApplyRule; +virFirewallDStatus;
Just a syntax/spacing NIT here about 2 blank lines seems to be the norm between groupings. So need one before and one after.
# util/virfirmware.h virFirmwareFreeList;
[...] John