[libvirt] [PATCH 09/12] Add a JSON properties parameter to virLog{, V}Message

[Daniel P. Berrange]

On Thu, Sep 20, 2012 at 08:24:08PM +0200, Miloslav Trmač wrote:
> ... and update all users.  No change in functionality, the parameter
> will be used in later patches.
> 
> Signed-off-by: Miloslav Trmač <mitr at redhat.com>
> ---
>  src/uml/uml_conf.c   |  2 +-
>  src/util/logging.c   | 13 +++++++++----
>  src/util/logging.h   | 17 +++++++++--------
>  src/util/viraudit.c  |  4 ++--
>  src/util/virterror.c |  2 +-
>  5 files changed, 22 insertions(+), 16 deletions(-)
> 
> diff --git a/src/util/logging.h b/src/util/logging.h
> index a66f5dc..14b7b7c 100644
> --- a/src/util/logging.h
> +++ b/src/util/logging.h
> @@ -141,13 +142,13 @@ extern int virLogParseFilters(const char *filters);
>  extern int virLogParseOutputs(const char *output);
>  extern void virLogMessage(const char *category, int priority,
>                            const char *funcname, long long linenr,
> -                          unsigned int flags,
> -                          const char *fmt, ...) ATTRIBUTE_FMT_PRINTF(6, 7);
> +                          virJSONObjectPtr properties, unsigned int flags,
> +                          const char *fmt, ...) ATTRIBUTE_FMT_PRINTF(7, 8);
>  extern void virLogVMessage(const char *category, int priority,
>                             const char *funcname, long long linenr,
> -                           unsigned int flags,
> +                           virJSONObjectPtr properties, unsigned int flags,
>                             const char *fmt,
> -                           va_list vargs) ATTRIBUTE_FMT_PRINTF(6, 0);
> +                           va_list vargs) ATTRIBUTE_FMT_PRINTF(7, 0);
>  extern int virLogSetBufferSize(int size);
>  extern void virLogEmergencyDumpAll(int signum);
>  #endif

Definite NACK to this change, since it is exposing the impl details
of the Lumberjack log output function to all users of the libvirt
logging API, not to mention the general unpleasant usability aspects
of having to build up JSON objects simply to pass a few extra metadata
fields.

I'm all for allowing more metadata properties to be passed into the
logging functions, but we need a simpler API along the lines of the
systemd journal  sd_journal_send() style which allows for a set of
key=value pairs to be passed in. I'd not try to shoe-horn it into
the existing virLogMessage() APIs. In the same way that there is
sd_journal_print() for simple string messages, vis sd_journal_send()
for arbitrary key=value pair log messages, I'd create a new API
for this called virLogMetadata and virLogVMetadata. eg to allow
sending a message with an error no

   virLogMetadata(__FILE__, VIR_LOG_WARN,
                  __FUNC__, __LINE__,
                  0,
                  "MESSAGE=Unable to open file %s: %s", "/some/file", strerror(errno),
                  "ERRNO=%d", errno,
                  NULL);

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|