[libvirt] [PATCH 07/14] nwfilter: introduce virNWFilterBinding to decouple from virDomainNet

[Jiri Denemark]

On Fri, Apr 27, 2018 at 16:25:06 +0100, Daniel P. Berrangé wrote:
> The virDomainNet struct contains everything related to configuring a
> guest network device. Out of all of this info, only 5 fields are
> relevant to configuring network filters. It will be more convenient for
> future changes to the nwfilter driver if the relevant fields are kept in
> a dedicated struct. Thus the virNWFilterBinding struct is created to
> track this information.
> 
> Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
> ---
>  src/conf/nwfilter_conf.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++-
>  src/conf/nwfilter_conf.h | 18 +++++++++++++++-
>  src/libvirt_private.syms |  2 ++
>  3 files changed, 71 insertions(+), 2 deletions(-)
> 
> diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
> index 5d04f2a93c..3d2ae9d0f3 100644
> --- a/src/conf/nwfilter_conf.c
> +++ b/src/conf/nwfilter_conf.c
> @@ -2,7 +2,7 @@
>   * nwfilter_conf.c: network filter XML processing
>   *                  (derived from storage_conf.c)
>   *
> - * Copyright (C) 2006-2014 Red Hat, Inc.
> + * Copyright (C) 2006-2018 Red Hat, Inc.
>   * Copyright (C) 2006-2008 Daniel P. Berrange
>   *
>   * Copyright (C) 2010-2011 IBM Corporation
> @@ -3265,3 +3265,54 @@ virNWFilterRuleIsProtocolEthernet(virNWFilterRuleDefPtr rule)
>          return true;
>      return false;
>  }
> +

Two empty lines between functions, please.

> +void virNWFilterBindingFree(virNWFilterBindingPtr binding)

The type should be on a separate line.

> +{
> +    if (!binding)
> +        return;
> +
> +    VIR_FREE(binding->ownername);
> +    VIR_FREE(binding->portdevname);
> +    VIR_FREE(binding->linkdevname);
> +    VIR_FREE(binding->filter);
> +    virHashFree(binding->filterparams);
> +
> +    VIR_FREE(binding);
> +}
> +

Two empty lines between functions, please.

> +virNWFilterBindingPtr virNWFilterBindingCopy(virNWFilterBindingPtr src)

The type should be on a separate line.

> +{
> +    virNWFilterBindingPtr ret;
> +
> +    if (VIR_ALLOC(ret) < 0)
> +        return NULL;
> +
> +    if (VIR_STRDUP(ret->ownername, src->ownername) < 0)
> +        goto error;
> +
> +    memcpy(ret->owneruuid, src->owneruuid, sizeof(ret->owneruuid));
> +
> +    if (VIR_STRDUP(ret->portdevname, src->portdevname) < 0)
> +        goto error;
> +
> +    if (src->linkdevname &&
> +        VIR_STRDUP(ret->linkdevname, src->linkdevname) < 0)

VIR_STRDUP can be safely used on NULL source, you don't need to guard it
with the extra check.

> +        goto error;
> +
> +    ret->mac = src->mac;
> +
> +    if (VIR_STRDUP(ret->filter, src->filter) < 0)
> +        goto error;
> +
> +    if (!(ret->filterparams = virNWFilterHashTableCreate(0)))
> +        goto error;
> +
> +    if (virNWFilterHashTablePutAll(src->filterparams, ret->filterparams) < 0)
> +        goto error;
> +
> +    return ret;
> +
> + error:
> +    virNWFilterBindingFree(ret);
> +    return NULL;
> +}
> diff --git a/src/conf/nwfilter_conf.h b/src/conf/nwfilter_conf.h
> index a31db6d3ff..8c5421ee62 100644
> --- a/src/conf/nwfilter_conf.h
> +++ b/src/conf/nwfilter_conf.h
> @@ -2,7 +2,7 @@
>   * nwfilter_conf.h: network filter XML processing
>   *                  (derived from storage_conf.h)
>   *
> - * Copyright (C) 2006-2010, 2012-2014 Red Hat, Inc.
> + * Copyright (C) 2006-2010, 2012-2018 Red Hat, Inc.
>   * Copyright (C) 2006-2008 Daniel P. Berrange
>   *
>   * Copyright (C) 2010 IBM Corporation
> @@ -545,6 +545,19 @@ struct _virNWFilterDef {
>      virNWFilterEntryPtr *filterEntries;
>  };
>  
> +typedef struct virNWFilterBinding virNWFilterBinding;
> +typedef virNWFilterBinding *virNWFilterBindingPtr;
> +
> +struct virNWFilterBinding {
> +    char *ownername;
> +    unsigned char owneruuid[VIR_UUID_BUFLEN];
> +    char *portdevname;
> +    char *linkdevname;
> +    virMacAddr mac;
> +    char *filter;
> +    virHashTablePtr filterparams;
> +};
> +
>  
>  typedef enum {
>      STEP_APPLY_NEW,
> @@ -650,6 +663,9 @@ virNWFilterRuleIsProtocolIPv6(virNWFilterRuleDefPtr rule);
>  bool
>  virNWFilterRuleIsProtocolEthernet(virNWFilterRuleDefPtr rule);
>  
> +void virNWFilterBindingFree(virNWFilterBindingPtr binding);
> +virNWFilterBindingPtr virNWFilterBindingCopy(virNWFilterBindingPtr src);

The types should be on separate lines for consistency with the rest of
this header file.

> +
>  VIR_ENUM_DECL(virNWFilterRuleAction);
>  VIR_ENUM_DECL(virNWFilterRuleDirection);
>  VIR_ENUM_DECL(virNWFilterRuleProtocol);
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index bf17d17777..9fc0aa470d 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -780,6 +780,8 @@ virDomainNumatuneSpecifiedMaxNode;
>  
>  
>  # conf/nwfilter_conf.h
> +virNWFilterBindingCopy;
> +virNWFilterBindingFree;
>  virNWFilterCallbackDriversLock;
>  virNWFilterCallbackDriversUnlock;
>  virNWFilterChainSuffixTypeToString;

See my replies to the following patches for possible modifications...

Reviewed-by: Jiri Denemark <jdenemar at redhat.com>