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@xxxxxxxxx> > --- > 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
