[libvirt] [PATCH v2 20/20] virlog: Split parsing and setting priority

[John Ferlan]

On 08/18/2016 07:47 AM, Erik Skultety wrote:
> Handling of outputs and filters has been changed in a way that splits
> parsing and defining. Do the same thing for logging priority as well, this
> however, doesn't need much of a preparation.
> ---
>  src/util/virlog.c | 21 +++++++++------------
>  tests/eventtest.c |  3 ++-
>  2 files changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/src/util/virlog.c b/src/util/virlog.c
> index 713cd0c..683cf6b 100644
> --- a/src/util/virlog.c
> +++ b/src/util/virlog.c
> @@ -220,7 +220,9 @@ int
>  virLogSetDefaultPriority(virLogPriority priority)
>  {
>      if ((priority < VIR_LOG_DEBUG) || (priority > VIR_LOG_ERROR)) {
> -        VIR_WARN("Ignoring invalid log level setting.");
> +        virReportError(VIR_ERR_INVALID_ARG,
> +                       _("Failed to set logging priority, argument '%u' is "
> +                         "invalid"), priority);

By this point I was too lazy to check, but we're not going to start
seeing strange failures from some rogue/incorrect setting that
previously essentially ignored this.

>          return -1;
>      }
>      if (virLogInitialize() < 0)
> @@ -1158,20 +1160,15 @@ virLogGetNbOutputs(void)


You'll need to update the comments to this function about treturn values...

Conditional ACK w/ adjustment and assurance about the above usage of
virReportError vs. VIR_WARN


John

>  int
>  virLogParseDefaultPriority(const char *priority)
>  {
> -    int ret = -1;
> -
>      if (STREQ(priority, "1") || STREQ(priority, "debug"))
> -        ret = virLogSetDefaultPriority(VIR_LOG_DEBUG);
> +        return VIR_LOG_DEBUG;
>      else if (STREQ(priority, "2") || STREQ(priority, "info"))
> -        ret = virLogSetDefaultPriority(VIR_LOG_INFO);
> +        return VIR_LOG_INFO;
>      else if (STREQ(priority, "3") || STREQ(priority, "warning"))
> -        ret = virLogSetDefaultPriority(VIR_LOG_WARN);
> +        return VIR_LOG_WARN;
>      else if (STREQ(priority, "4") || STREQ(priority, "error"))
> -        ret = virLogSetDefaultPriority(VIR_LOG_ERROR);
> -    else
> -        VIR_WARN("Ignoring invalid log level setting");
> -
> -    return ret;
> +        return VIR_LOG_ERROR;
> +    return -1;
>  }
>  
>  
> @@ -1191,7 +1188,7 @@ virLogSetFromEnv(void)
>  
>      debugEnv = virGetEnvAllowSUID("LIBVIRT_DEBUG");
>      if (debugEnv && *debugEnv)
> -        virLogParseDefaultPriority(debugEnv);
> +        virLogSetDefaultPriority(virLogParseDefaultPriority(debugEnv));
>      debugEnv = virGetEnvAllowSUID("LIBVIRT_LOG_FILTERS");
>      if (debugEnv && *debugEnv)
>          virLogSetFilters(debugEnv);
> diff --git a/tests/eventtest.c b/tests/eventtest.c
> index 4b62f34..011bedc 100644
> --- a/tests/eventtest.c
> +++ b/tests/eventtest.c
> @@ -311,7 +311,8 @@ mymain(void)
>      if (virThreadInitialize() < 0)
>          return EXIT_FAILURE;
>      char *debugEnv = getenv("LIBVIRT_DEBUG");
> -    if (debugEnv && *debugEnv && (virLogParseDefaultPriority(debugEnv) == -1)) {
> +    if (debugEnv && *debugEnv &&
> +        (virLogSetDefaultPriority(virLogParseDefaultPriority(debugEnv)) < 0)) {
>          fprintf(stderr, "Invalid log level setting.\n");
>          return EXIT_FAILURE;
>      }
>