1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH v2 0/4] Reimplement logging wildcards to use full shell glob syntax
  4. View patch

[libvirt] [PATCH v2 1/4] Revert "util: virlog: Introduce wildcard to log filters"

Daniel P. Berrangé posted 4 patches 7 years, 7 months ago
[libvirt] [PATCH v2 1/4] Revert "util: virlog: Introduce wildcard to log filters"
Posted by Daniel P. Berrangé 7 years, 7 months ago 
This reverts commit 8daa593b0741452a78daea76075254ae7d3c6ab6.

There are two undesirable aspects to the impl

  - Only a bare wildcard is permitted
  - The wildcard match is not performed in the order listed
---
 src/remote/libvirtd.conf |  4 +---
 src/util/virlog.c        | 44 +-------------------------------------------
 src/util/virlog.h        |  1 -
 3 files changed, 2 insertions(+), 47 deletions(-)

diff --git a/src/remote/libvirtd.conf b/src/remote/libvirtd.conf
index 7d823cf1ad..9c0080dc06 100644
--- a/src/remote/libvirtd.conf
+++ b/src/remote/libvirtd.conf
@@ -368,9 +368,7 @@
 # Multiple filters can be defined in a single @filters, they just need to be
 # separated by spaces. Note that libvirt performs "first" match, i.e. if
 # there are concurrent filters, the first one that matches will be applied,
-# given the order in log_filters with the exception of a wildcard filter, since
-# that's only taken into account if no other filter has matched, so
-# "4:* 1:util.pci" and "1:util.pci 4:*" are equivalent definitions.
+# given the order in log_filters.
 #
 # e.g. to only get warning or errors from the remote layer and only errors
 # from the event layer:
diff --git a/src/util/virlog.c b/src/util/virlog.c
index 81a9dc4395..5810643e1b 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -497,38 +497,6 @@ virLogHostnameString(char **rawmsg,
     return 0;
 }
 
-/* virLogFiltersFind:
- * @filters: haystack
- * @nfilters: haystack size
- * @key1: primary string 'needle'
- * @key2: secondary integer 'needle'
- *
- * Performs "first match" search on the input set of filters, using @key1
- * (string) and  @key2 (integer) as primary and secondary keys respectively.
- * Secondary key is only considered if primary key wasn't provided.
- *
- * Returns a pointer to the matched object or NULL if no match was found.
- */
-static virLogFilterPtr
-virLogFiltersFind(virLogFilterPtr *filters,
-                  size_t nfilters,
-                  const char *key1,
-                  unsigned int key2)
-{
-    size_t i;
-
-    if (!key1 && key2 == 0)
-        return NULL;
-
-    for (i = 0; i < nfilters; i++) {
-        if ((key1 && STREQ(key1, filters[i]->match)) ||
-            filters[i]->flags == key2)
-            return filters[i];
-    }
-
-    return NULL;
-}
-
 
 static void
 virLogSourceUpdate(virLogSourcePtr source)
@@ -1441,8 +1409,7 @@ virLogFilterNew(const char *match,
     virLogFilterPtr ret = NULL;
     char *mdup = NULL;
 
-    virCheckFlags(VIR_LOG_STACK_TRACE |
-                  VIR_LOG_WILDCARD, NULL);
+    virCheckFlags(VIR_LOG_STACK_TRACE, NULL);
 
     if (priority < VIR_LOG_DEBUG || priority > VIR_LOG_ERROR) {
         virReportError(VIR_ERR_INVALID_ARG, _("Invalid log priority %d"),
@@ -1561,8 +1528,6 @@ virLogDefineOutputs(virLogOutputPtr *outputs, size_t noutputs)
 int
 virLogDefineFilters(virLogFilterPtr *filters, size_t nfilters)
 {
-    virLogFilterPtr rc;
-
     if (virLogInitialize() < 0)
         return -1;
 
@@ -1570,10 +1535,6 @@ virLogDefineFilters(virLogFilterPtr *filters, size_t nfilters)
     virLogResetFilters();
     virLogFilters = filters;
     virLogNbFilters = nfilters;
-
-    /* if there's a wildcard filter, update default priority */
-    if ((rc = virLogFiltersFind(filters, nfilters, NULL, VIR_LOG_WILDCARD)))
-        virLogDefaultPriority = rc->priority;
     virLogUnlock();
 
     return 0;
@@ -1749,9 +1710,6 @@ virLogParseFilter(const char *src)
         match++;
     }
 
-    if (STREQ(match, "*"))
-        flags |= VIR_LOG_WILDCARD;
-
     /* match string cannot comprise just from a single '+' */
     if (!*match) {
         virReportError(VIR_ERR_INVALID_ARG,
diff --git a/src/util/virlog.h b/src/util/virlog.h
index 07b91f0b0a..8973f83b20 100644
--- a/src/util/virlog.h
+++ b/src/util/virlog.h
@@ -174,7 +174,6 @@ typedef void (*virLogCloseFunc) (void *data);
 
 typedef enum {
     VIR_LOG_STACK_TRACE = (1 << 0),
-    VIR_LOG_WILDCARD = (1 << 1)
 } virLogFlags;
 
 int virLogGetNbFilters(void);
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/4] Revert "util: virlog: Introduce wildcard to log filters"
Posted by John Ferlan 7 years, 7 months ago 

On 04/23/2018 08:28 AM, Daniel P. Berrangé wrote:
> This reverts commit 8daa593b0741452a78daea76075254ae7d3c6ab6.
> 
> There are two undesirable aspects to the impl
> 
>   - Only a bare wildcard is permitted
>   - The wildcard match is not performed in the order listed
> ---
>  src/remote/libvirtd.conf |  4 +---
>  src/util/virlog.c        | 44 +-------------------------------------------
>  src/util/virlog.h        |  1 -
>  3 files changed, 2 insertions(+), 47 deletions(-)
> 

So that it's not forgotten 0-) before 4.3.0...

Reviewed-by: John Ferlan <jferlan@redhat.com>

Safe too as long as you also revert:

209d4d6f42a01829c68ef54b042a0c53ef0b22d3

to remove the description from news.xml

I believe the rest can wait for 4.4.0, right?

John

> diff --git a/src/remote/libvirtd.conf b/src/remote/libvirtd.conf
> index 7d823cf1ad..9c0080dc06 100644
> --- a/src/remote/libvirtd.conf
> +++ b/src/remote/libvirtd.conf
> @@ -368,9 +368,7 @@
>  # Multiple filters can be defined in a single @filters, they just need to be
>  # separated by spaces. Note that libvirt performs "first" match, i.e. if
>  # there are concurrent filters, the first one that matches will be applied,
> -# given the order in log_filters with the exception of a wildcard filter, since
> -# that's only taken into account if no other filter has matched, so
> -# "4:* 1:util.pci" and "1:util.pci 4:*" are equivalent definitions.
> +# given the order in log_filters.
>  #
>  # e.g. to only get warning or errors from the remote layer and only errors
>  # from the event layer:
> diff --git a/src/util/virlog.c b/src/util/virlog.c
> index 81a9dc4395..5810643e1b 100644
> --- a/src/util/virlog.c
> +++ b/src/util/virlog.c
> @@ -497,38 +497,6 @@ virLogHostnameString(char **rawmsg,
>      return 0;
>  }
>  
> -/* virLogFiltersFind:
> - * @filters: haystack
> - * @nfilters: haystack size
> - * @key1: primary string 'needle'
> - * @key2: secondary integer 'needle'
> - *
> - * Performs "first match" search on the input set of filters, using @key1
> - * (string) and  @key2 (integer) as primary and secondary keys respectively.
> - * Secondary key is only considered if primary key wasn't provided.
> - *
> - * Returns a pointer to the matched object or NULL if no match was found.
> - */
> -static virLogFilterPtr
> -virLogFiltersFind(virLogFilterPtr *filters,
> -                  size_t nfilters,
> -                  const char *key1,
> -                  unsigned int key2)
> -{
> -    size_t i;
> -
> -    if (!key1 && key2 == 0)
> -        return NULL;
> -
> -    for (i = 0; i < nfilters; i++) {
> -        if ((key1 && STREQ(key1, filters[i]->match)) ||
> -            filters[i]->flags == key2)
> -            return filters[i];
> -    }
> -
> -    return NULL;
> -}
> -
>  
>  static void
>  virLogSourceUpdate(virLogSourcePtr source)
> @@ -1441,8 +1409,7 @@ virLogFilterNew(const char *match,
>      virLogFilterPtr ret = NULL;
>      char *mdup = NULL;
>  
> -    virCheckFlags(VIR_LOG_STACK_TRACE |
> -                  VIR_LOG_WILDCARD, NULL);
> +    virCheckFlags(VIR_LOG_STACK_TRACE, NULL);
>  
>      if (priority < VIR_LOG_DEBUG || priority > VIR_LOG_ERROR) {
>          virReportError(VIR_ERR_INVALID_ARG, _("Invalid log priority %d"),
> @@ -1561,8 +1528,6 @@ virLogDefineOutputs(virLogOutputPtr *outputs, size_t noutputs)
>  int
>  virLogDefineFilters(virLogFilterPtr *filters, size_t nfilters)
>  {
> -    virLogFilterPtr rc;
> -
>      if (virLogInitialize() < 0)
>          return -1;
>  
> @@ -1570,10 +1535,6 @@ virLogDefineFilters(virLogFilterPtr *filters, size_t nfilters)
>      virLogResetFilters();
>      virLogFilters = filters;
>      virLogNbFilters = nfilters;
> -
> -    /* if there's a wildcard filter, update default priority */
> -    if ((rc = virLogFiltersFind(filters, nfilters, NULL, VIR_LOG_WILDCARD)))
> -        virLogDefaultPriority = rc->priority;
>      virLogUnlock();
>  
>      return 0;
> @@ -1749,9 +1710,6 @@ virLogParseFilter(const char *src)
>          match++;
>      }
>  
> -    if (STREQ(match, "*"))
> -        flags |= VIR_LOG_WILDCARD;
> -
>      /* match string cannot comprise just from a single '+' */
>      if (!*match) {
>          virReportError(VIR_ERR_INVALID_ARG,
> diff --git a/src/util/virlog.h b/src/util/virlog.h
> index 07b91f0b0a..8973f83b20 100644
> --- a/src/util/virlog.h
> +++ b/src/util/virlog.h
> @@ -174,7 +174,6 @@ typedef void (*virLogCloseFunc) (void *data);
>  
>  typedef enum {
>      VIR_LOG_STACK_TRACE = (1 << 0),
> -    VIR_LOG_WILDCARD = (1 << 1)
>  } virLogFlags;
>  
>  int virLogGetNbFilters(void);
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/4] Revert "util: virlog: Introduce wildcard to log filters"
Posted by Daniel P. Berrangé 7 years, 7 months ago 
On Fri, Apr 27, 2018 at 09:38:06AM -0400, John Ferlan wrote:
> 
> 
> On 04/23/2018 08:28 AM, Daniel P. Berrangé wrote:
> > This reverts commit 8daa593b0741452a78daea76075254ae7d3c6ab6.
> > 
> > There are two undesirable aspects to the impl
> > 
> >   - Only a bare wildcard is permitted
> >   - The wildcard match is not performed in the order listed
> > ---
> >  src/remote/libvirtd.conf |  4 +---
> >  src/util/virlog.c        | 44 +-------------------------------------------
> >  src/util/virlog.h        |  1 -
> >  3 files changed, 2 insertions(+), 47 deletions(-)
> > 
> 
> So that it's not forgotten 0-) before 4.3.0...
> 
> Reviewed-by: John Ferlan <jferlan@redhat.com>
> 
> Safe too as long as you also revert:
> 
> 209d4d6f42a01829c68ef54b042a0c53ef0b22d3
> 
> to remove the description from news.xml
> 
> I believe the rest can wait for 4.4.0, right?

Yeah, that's fine with me.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.