[libvirt] [PATCH v2 06/20] virlog: Introduce virLogDefineFilters

[John Ferlan]

On 08/18/2016 07:47 AM, Erik Skultety wrote:
> Prepare a method that only defines a set of filters. It takes a list of
> filters, preferably created by virLogParseFilters. The original set of filters
> is reset and replaced by the new user-provided set of filters.
> 
> Signed-off-by: Erik Skultety <eskultet at redhat.com>
> ---
>  src/libvirt_private.syms |  1 +
>  src/util/virlog.c        | 26 ++++++++++++++++++++++++++
>  src/util/virlog.h        |  1 +
>  3 files changed, 28 insertions(+)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index fb7f277..0bceba7 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1843,6 +1843,7 @@ virLockSpaceReleaseResourcesForOwner;
>  
>  # util/virlog.h
>  virLogDefineFilter;
> +virLogDefineFilters;
>  virLogDefineOutput;
>  virLogDefineOutputs;
>  virLogFilterFree;
> diff --git a/src/util/virlog.c b/src/util/virlog.c
> index 2651f70..a74967b 100644
> --- a/src/util/virlog.c
> +++ b/src/util/virlog.c
> @@ -1707,3 +1707,29 @@ virLogDefineOutputs(virLogOutputPtr *outputs, size_t noutputs)
>  
>      return virLogNbOutputs;
>  }
> +
> +
> +/**
> + * virLogDefineFilters:
> + * @filters: new set of filters to be defined
> + * @nfilters: number of filters in @filters
> + *
> + * Resets any existing set of filters and defines a completely new one.

Similar to previous patch - can filters be NULL? in order to wipe out
filters...

> + *
> + * Returns number of filters successfully defined or -1 in case of error;

s/error;/error.

Although, like previous patch -1 or 0 can be returned...

> + */
> +int
> +virLogDefineFilters(virLogFilterPtr *filters, size_t nfilters)
> +{
> +    if (virLogInitialize() < 0)
> +        return -1;
> +
> +    virLogLock();
> +    virLogResetOutputs();

Copy/Paste error - s/b virLogResetFilters(); ?

> +    virLogFilters = filters;
> +    virLogNbOutputs = nfilters;

Similar copy/paste error s/b virLogNbFilters

> +    virLogFiltersSerial++;

And since virLogResetFilters updates this, it's not required to do it
twice right?

> +    virLogUnlock();
> +
> +    return virLogNbFilters;

Again, return 0 - you have 1 failure scenario, followed by a bunch of
void calls.

> +}
> diff --git a/src/util/virlog.h b/src/util/virlog.h
> index 8568830..e0fe008 100644
> --- a/src/util/virlog.h
> +++ b/src/util/virlog.h
> @@ -238,5 +238,6 @@ virLogFilterPtr virLogFilterNew(const char *match,
>  int virLogFindOutput(virLogOutputPtr *outputs, size_t noutputs,
>                       virLogDestination dest, const void *opaque);
>  int virLogDefineOutputs(virLogOutputPtr *outputs, size_t noutputs);
> +int virLogDefineFilters(virLogFilterPtr *filters, size_t nfilters);

Nice to see the editor wasn't going crazy this time ;-)

Add the ATTRIBUTE_NONNULL(1) if that's an unexpected input...

Otherwise, ACK with adjustments.

John


>  
>  #endif
>