Rather than specialcasing handling of the '*' character, use fnmatch()
to get normal shell wildcard syntax, as described in 'man glob(7)'.

To get an indication of the performance impact of using globs instead
of plain string matches, a test program was written. The list of all
260 log categories was extracted from the source. Then a typical log
filters setup was picked by creating an array of the strings "qemu",
"security", "util", "cgroup", "event", "object". Every filter string
was matched against every log category. Timing information showed that
using strstr() this took 8 microseconds, while fnmatch() took 114
microseconds.

IOW, fnmatch is 14 times slower than our existing strstr check. These
numbers show a worst case scenario that wil never be hit, because it
is rare that every log category would have data output. The log category
matches are cached, so each category is only checked once no matter how
many log statements are emitted. IOW despite being slower, this will
be lost in the noise and have no consequence on real world logging
performance.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/util/virlog.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/util/virlog.c b/src/util/virlog.c
index 5262d613f6..be9fc0cf78 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -40,6 +40,7 @@
 #if HAVE_SYS_UN_H
 # include <sys/un.h>
 #endif
+#include <fnmatch.h>
 
 #include "virerror.h"
 #include "virlog.h"
@@ -508,7 +509,7 @@ virLogSourceUpdate(virLogSourcePtr source)
         size_t i;
 
         for (i = 0; i < virLogNbFilters; i++) {
-            if (strstr(source->name, virLogFilters[i]->match)) {
+            if (fnmatch(virLogFilters[i]->match, source->name, 0) == 0) {
                 priority = virLogFilters[i]->priority;
                 flags = virLogFilters[i]->flags;
                 break;
-- 
2.14.3

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

On 04/23/2018 08:28 AM, Daniel P. Berrangé wrote:
> Rather than specialcasing handling of the '*' character, use fnmatch()
> to get normal shell wildcard syntax, as described in 'man glob(7)'.
> 
> To get an indication of the performance impact of using globs instead
> of plain string matches, a test program was written. The list of all
> 260 log categories was extracted from the source. Then a typical log
> filters setup was picked by creating an array of the strings "qemu",
> "security", "util", "cgroup", "event", "object". Every filter string
> was matched against every log category. Timing information showed that
> using strstr() this took 8 microseconds, while fnmatch() took 114
> microseconds.
> 
> IOW, fnmatch is 14 times slower than our existing strstr check. These
> numbers show a worst case scenario that wil never be hit, because it

s/wil/will

> is rare that every log category would have data output. The log category
> matches are cached, so each category is only checked once no matter how
> many log statements are emitted. IOW despite being slower, this will
> be lost in the noise and have no consequence on real world logging
> performance.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  src/util/virlog.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 

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

John

BTW: So whether this is available "everywhere" is a bit of an unknown to
me - as in strstr would seemingly work on every arch, but fnmatch has
this linux-ism wildcard thing going on which leaves a slight bit of
doubt in my mind...

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

On Mon, Apr 30, 2018 at 11:05:48AM -0400, John Ferlan wrote:
> 
> 
> On 04/23/2018 08:28 AM, Daniel P. Berrangé wrote:
> > Rather than specialcasing handling of the '*' character, use fnmatch()
> > to get normal shell wildcard syntax, as described in 'man glob(7)'.
> > 
> > To get an indication of the performance impact of using globs instead
> > of plain string matches, a test program was written. The list of all
> > 260 log categories was extracted from the source. Then a typical log
> > filters setup was picked by creating an array of the strings "qemu",
> > "security", "util", "cgroup", "event", "object". Every filter string
> > was matched against every log category. Timing information showed that
> > using strstr() this took 8 microseconds, while fnmatch() took 114
> > microseconds.
> > 
> > IOW, fnmatch is 14 times slower than our existing strstr check. These
> > numbers show a worst case scenario that wil never be hit, because it
> 
> s/wil/will
> 
> > is rare that every log category would have data output. The log category
> > matches are cached, so each category is only checked once no matter how
> > many log statements are emitted. IOW despite being slower, this will
> > be lost in the noise and have no consequence on real world logging
> > performance.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  src/util/virlog.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> 
> Reviewed-by: John Ferlan <jferlan@redhat.com>
> 
> John
> 
> BTW: So whether this is available "everywhere" is a bit of an unknown to
> me - as in strstr would seemingly work on every arch, but fnmatch has
> this linux-ism wildcard thing going on which leaves a slight bit of
> doubt in my mind...

NB, we rely on gnulib to provide us fnmatch on all platforms

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