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

Re: [libvirt] [PATCH 2/5] util: move all firewalld-specific stuff into its own file




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


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