syslog support to xentoollog

syslog support to xentoollog

[Gihan Munasinghe]

Guys

Is there any plans to add syslog support in to xentoollog.

Is this something that xentoollog would like to adopt .

Thanks
Gihan

Re: syslog support to xentoollog

[Stefano Stabellini]

On Wed, 2 Jun 2010, Gihan Munasinghe wrote:
> Guys
> 
> Is there any plans to add syslog support in to xentoollog.
> 
> Is this something that xentoollog would like to adopt .
> 

That is something we definitely want, patches are welcome :-)

Re: syslog support to xentoollog

[Gihan Munasinghe]

Stefano Stabellini wrote:
> On Wed, 2 Jun 2010, Gihan Munasinghe wrote:
>   
>> Guys
>>
>> Is there any plans to add syslog support in to xentoollog.
>>
>> Is this something that xentoollog would like to adopt .
>>
>>     
>
> That is something we definitely want, patches are welcome :-)
>
>   
Great I'll do some patching to get syslog working.

looking at the xentool.h, following definitions

#define XTL_STDIOSTREAM_SHOW_PID      01u
#define XTL_STDIOSTREAM_SHOW_DATE     02u
#define XTL_STDIOSTREAM_HIDE_PROGRESS 04u

I would suggest to be as
#define XTL_SHOW_PID      01u
#define XTL_SHOW_DATE     02u
#define XTL_HIDE_PROGRESS 04u
So it will be more generic for any loggers that will be implemented

also is everyone happy with following loglevel mappings between 
xentoollog_level and syslog

        {XTL_DEBUG,LOG_DEBUG},
        {XTL_VERBOSE,LOG_DEBUG},
        {XTL_DETAIL,LOG_DEBUG},
        {XTL_PROGRESS,LOG_DEBUG},
        {XTL_INFO,LOG_INFO},
        {XTL_NOTICE,LOG_NOTICE},
        {XTL_WARN,LOG_WARNING},
        {XTL_ERROR,LOG_ERR},
        {XTL_CRITICAL,LOG_CRIT}

Thanks
Gihan

Re: syslog support to xentoollog

[Ian Jackson]

Gihan Munasinghe writes ("Re: [Xen-devel] syslog support to xentoollog"):
> Great I'll do some patching to get syslog working.

Excellent.

> looking at the xentool.h, following definitions
> 
> #define XTL_STDIOSTREAM_SHOW_PID      01u
> #define XTL_STDIOSTREAM_SHOW_DATE     02u
> #define XTL_STDIOSTREAM_HIDE_PROGRESS 04u
> 
> I would suggest to be as
> #define XTL_SHOW_PID      01u
> #define XTL_SHOW_DATE     02u
> #define XTL_HIDE_PROGRESS 04u
> So it will be more generic for any loggers that will be implemented

Sure.

Note though that XTL_SHOW_DATE isn't going to be relevant for syslog
since in syslog the date is put in by syslogd.

> also is everyone happy with following loglevel mappings between 
> xentoollog_level and syslog
> 
>         {XTL_DEBUG,LOG_DEBUG},
>         {XTL_VERBOSE,LOG_DEBUG},
>         {XTL_DETAIL,LOG_DEBUG},
>         {XTL_PROGRESS,LOG_DEBUG},
>         {XTL_INFO,LOG_INFO},
>         {XTL_NOTICE,LOG_NOTICE},
>         {XTL_WARN,LOG_WARNING},
>         {XTL_ERROR,LOG_ERR},
>         {XTL_CRITICAL,LOG_CRIT}

That's probably right.

Ian.

[PATCH] syslog support to xentoollog

[Gihan Munasinghe]

[-- Attachment #1: Type: text/plain, Size: 1427 bytes --]

Guys

See the attach patch, this implements a xentoollog consumer that can log 
in to syslog

Following are the changes

xentoollog.h:
1) Generic, xtl_logger_set_minlevel and xtl_logger_adjust_flags 
functions to be called from outside,
2) Making logger flags to be more generic
3) Change the struct xentool_logger, each logger will implement its own 
"set_minlevel" and "adjust_flags"  which will be invoked by 
xtl_logger_set_minlevel and xtl_logger_adjust_flags generic functions.
4) New function prototype to create a xentool_logger_syslogger

Going forward I would like to suggest that xentoollog.h file should only 
have generic functions, except for xtl_createlogger_* functions. Logger 
type specific function should not be included(e.g 
xtl_stdiostream_set_minlevel ) this will make sure outside code can 
switch from one logger to another (from stdiologger to syslogger vice 
versa) with out breaking the code.

xtl_core.c:
1) Implementation of xtl_logger_set_minlevel and xtl_logger_adjust_flags 
functions.

xtl_logger_syslog.c :
New logger implementation to log in to syslog.


I did some testing by changing the code in xl. This seems to work, but I 
would really like if more people can test this.
Also could someone look @ the syslogger_progress function and add any 
extra bits if needed, I did a very basic implementation of this.  

Signed-off-by : Gihan Munasinghe <GMunasinghe@flexiant.com>


Thanks
Gihan

[-- Attachment #2: xen4-xentoollog-syslog --]
[-- Type: text/plain, Size: 11271 bytes --]

diff -Nuar a/tools/libxc/Makefile b/tools/libxc/Makefile
--- a/tools/libxc/Makefile	2010-06-01 23:32:43.000000000 +0100
+++ b/tools/libxc/Makefile	2010-06-03 22:56:42.000000000 +0100
@@ -29,6 +29,7 @@
 CTRL_SRCS-y       += xc_memshr.c
 CTRL_SRCS-y       += xtl_core.c
 CTRL_SRCS-y       += xtl_logger_stdio.c
+CTRL_SRCS-y       += xtl_logger_syslog.c
 CTRL_SRCS-$(CONFIG_X86) += xc_pagetab.c
 CTRL_SRCS-$(CONFIG_Linux) += xc_linux.c
 CTRL_SRCS-$(CONFIG_SunOS) += xc_solaris.c
diff -Nuar a/tools/libxc/xentoollog.h b/tools/libxc/xentoollog.h
--- a/tools/libxc/xentoollog.h	2010-06-01 23:32:43.000000000 +0100
+++ b/tools/libxc/xentoollog.h	2010-06-04 22:49:54.000000000 +0100
@@ -45,16 +45,28 @@
           * will always be called with done==0 for each new
           * context/doing_what */;
     void (*destroy)(struct xentoollog_logger *logger);
+    void (*set_minlevel)(xentoollog_logger *logger,
+                                  xentoollog_level min_level);
+    void (*adjust_flags)(xentoollog_logger *logger,
+                                  unsigned set_flags, unsigned clear_flags);
     /* each logger can put its necessary data here */
+
 };
 
 
 /*---------- facilities for consuming log messages ----------*/
 
+//-- Should be depricated not generic enough --//
 #define XTL_STDIOSTREAM_SHOW_PID      01u
 #define XTL_STDIOSTREAM_SHOW_DATE     02u
 #define XTL_STDIOSTREAM_HIDE_PROGRESS 04u
 
+//---Plase use the following as the new flags when creating and setting loggers
+#define XTL_SHOW_PID      01u
+#define XTL_SHOW_DATE     02u
+#define XTL_HIDE_PROGRESS 04u
+
+
 typedef struct xentoollog_logger_stdiostream  xentoollog_logger_stdiostream;
 
 xentoollog_logger_stdiostream *xtl_createlogger_stdiostream
@@ -62,15 +74,47 @@
     /* may return 0 if malloc fails, in which case error was logged */
     /* destroy on this logger does not close the file */
 
+//This method need to be depricated use more generic xtl_logger_set_minlevel method insted
 void xtl_stdiostream_set_minlevel(xentoollog_logger_stdiostream*,
                                   xentoollog_level min_level);
+
+//This method need to be depricated use more generic xtl_logger_adjust_flags method insted.
 void xtl_stdiostream_adjust_flags(xentoollog_logger_stdiostream*,
                                   unsigned set_flags, unsigned clear_flags);
   /* if set_flags and clear_flags overlap, set_flags takes precedence */
 
+
+typedef struct xentoollog_logger_syslogger xentoollog_logger_syslogger;
+
+/**
+Create a logger that will log to syslog 
+syslog_facility is the facilities that are in the sys/syslog.h
+edit /etc/syslog.conf file to indicate where you wansyslog to send log data
+
+example config:- 
+Add the following line in to /etc/syslog.conf
+ 
+local1.*       /var/log/xenlog
+restart syslogd
+When setting up the logger give LOG_LOCAL1 as the logging facility
+now all your logs will appear in /var/log/xenlog file 
+
+Please look more in to syslog and syslogng configs if you want to send the data in to remote logging servers 
+
+NOTE: XTL_SHOW_DATE flag will not take any effect as syslogd will put the date by default
+**/
+xentoollog_logger_syslogger *xtl_createlogger_syslogger
+        (int syslog_facility, xentoollog_level min_level, unsigned flags);
+
 void xtl_logger_destroy(struct xentoollog_logger *logger /* 0 is ok */);
 
 
+void xtl_logger_set_minlevel(struct xentoollog_logger *logger,xentoollog_level min_level);
+
+void xtl_logger_adjust_flags(struct xentoollog_logger *logger,
+                                  unsigned set_flags, unsigned clear_flags);
+
+
 /*---------- facilities for generating log messages ----------*/
 
 void xtl_logv(struct xentoollog_logger *logger,
@@ -104,6 +148,8 @@
     (buffer).vtable.vmessage = LOGGER##_vmessage;                       \
     (buffer).vtable.progress = LOGGER##_progress;                       \
     (buffer).vtable.destroy  = LOGGER##_destroy;                        \
+    (buffer).vtable.set_minlevel = LOGGER##_set_minlevel;                \
+    (buffer).vtable.adjust_flags = LOGGER##_adjust_flags;                \
                                                                         \
     new_consumer = malloc(sizeof(*new_consumer));                       \
     if (!new_consumer) {                                                \
diff -Nuar a/tools/libxc/xtl_core.c b/tools/libxc/xtl_core.c
--- a/tools/libxc/xtl_core.c	2010-06-01 23:32:43.000000000 +0100
+++ b/tools/libxc/xtl_core.c	2010-06-04 21:34:55.000000000 +0100
@@ -63,7 +63,23 @@
     logger->progress(logger, context, doing_what, percent, done, total);
 }
 
+void xtl_logger_set_minlevel(struct xentoollog_logger *logger,
+                             xentoollog_level min_level){
+    if (!logger || !logger->set_minlevel) return; 
+
+    logger->set_minlevel(logger,min_level);
+}
+    
+void xtl_logger_adjust_flags(struct xentoollog_logger *logger,
+                             unsigned set_flags, unsigned clear_flags){
+    if (!logger || !logger->adjust_flags) return; 
+
+    logger->adjust_flags(logger,set_flags,clear_flags);
+}
+
+
 void xtl_logger_destroy(struct xentoollog_logger *logger) {
-    if (!logger) return;
+    if (!logger || !logger->destroy) return;
+
     logger->destroy(logger);
 }
diff -Nuar a/tools/libxc/xtl_logger_stdio.c b/tools/libxc/xtl_logger_stdio.c
--- a/tools/libxc/xtl_logger_stdio.c	2010-06-01 23:32:43.000000000 +0100
+++ b/tools/libxc/xtl_logger_stdio.c	2010-06-04 21:44:52.000000000 +0100
@@ -124,6 +124,20 @@
     lg->flags = new_flags;
 }
 
+static void stdiostream_set_minlevel(struct xentoollog_logger *logger_in,
+                                  xentoollog_level min_level){
+
+    xentoollog_logger_stdiostream *lg = (void*)logger_in;
+    xtl_stdiostream_set_minlevel(lg,min_level);
+}
+
+static void stdiostream_adjust_flags(struct xentoollog_logger *logger_in,
+                                  unsigned set_flags, unsigned clear_flags) {
+
+    xentoollog_logger_stdiostream *lg = (void*)logger_in;
+    xtl_stdiostream_adjust_flags(lg,set_flags,clear_flags); 
+}
+
 xentoollog_logger_stdiostream *xtl_createlogger_stdiostream
         (FILE *f, xentoollog_level min_level, unsigned flags) {
     xentoollog_logger_stdiostream newlogger;
diff -Nuar a/tools/libxc/xtl_logger_syslog.c b/tools/libxc/xtl_logger_syslog.c
--- a/tools/libxc/xtl_logger_syslog.c	1970-01-01 01:00:00.000000000 +0100
+++ b/tools/libxc/xtl_logger_syslog.c	2010-06-05 00:52:09.000000000 +0100
@@ -0,0 +1,152 @@
+/**
+*xtl_logger_syslog.c
+*
+*xentool log comsumer implementation, that will log into syslog
+*
+*Copyright (c) 2010 Flexiant Ltd. 
+**/
+#include "xentoollog.h"
+
+#include <sys/syslog.h>
+#include <stdarg.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <unistd.h>
+#include <pthread.h>
+
+typedef struct _log_map{
+    xentoollog_level xen_log;
+    int syslog_level;
+}LOG_MAP;
+
+LOG_MAP log_mappings[] = 
+    {
+        {XTL_DEBUG,LOG_DEBUG}, 
+        {XTL_VERBOSE,LOG_DEBUG},
+        {XTL_DETAIL,LOG_DEBUG},
+        {XTL_PROGRESS,LOG_DEBUG},
+        {XTL_INFO,LOG_INFO},
+        {XTL_NOTICE,LOG_NOTICE},
+        {XTL_WARN,LOG_WARNING},
+        {XTL_ERROR,LOG_ERR},
+        {XTL_CRITICAL,LOG_CRIT}
+    };
+
+int maptable_len = sizeof(log_mappings)/sizeof(LOG_MAP);
+
+
+struct xentoollog_logger_syslogger{ 
+    xentoollog_logger vtable;
+    pthread_mutex_t mutex; //syslog in not thread safe at all therefore we need a lock before issue a syslog call 
+    unsigned flags;
+};
+
+/**
+Returns mapping syslog level for a given xentoollog_level
+**/
+static int get_syslog_level(xentoollog_level level){
+    int i; 
+    for (i=0; i<maptable_len; i++){
+        LOG_MAP map = log_mappings[i]; 
+        if (map.xen_log==level){
+           return map.syslog_level;
+        }
+    }
+    // If a mapping is not found use LOG_INFO as default
+    return LOG_INFO;
+}
+
+
+static void syslogger_vmessage(xentoollog_logger *logger_in,
+                                 xentoollog_level level,
+                                 int errnoval,
+                                 const char *context,
+                                 const char *format,
+                                 va_list al) {
+    char *buffer;
+    xentoollog_logger_syslogger *lg = (void*)logger_in;
+    
+    buffer = (char *)calloc(1024,sizeof(char));
+    if (context && (lg->flags & XTL_SHOW_PID)){
+       char *new_format = (char *) calloc(13+strlen(context)+10,sizeof(char));
+       sprintf(new_format,"[%s] [%lu] : ",context,(unsigned long)getpid());
+       strcat(buffer,new_format);
+       free(new_format);
+       
+    }else if(context){
+       char *new_format = (char *) calloc(7+strlen(context),sizeof(char));
+       sprintf(new_format,"[%s] : ",context);
+       strcat(buffer,new_format);
+       free(new_format);
+
+    }else if(lg->flags & XTL_SHOW_PID){
+       char *new_format = (char *) calloc(18,sizeof(char));
+       sprintf(new_format,"[%lu] : ",(unsigned long)getpid());
+       strcat(buffer,new_format);
+       free(new_format);
+
+    }
+  
+    if (errnoval >= 0){
+       char *new_format = (char *) calloc(100,sizeof(char));
+       sprintf(new_format,"[%s] : ",strerror(errnoval));
+       strcat(buffer,new_format);
+       free(new_format);
+    }
+ 
+    strcat(buffer,format);
+
+    pthread_mutex_lock(&lg->mutex);
+    vsyslog(get_syslog_level(level),buffer,al);
+    pthread_mutex_unlock(&lg->mutex);
+
+    free(buffer);
+}
+
+static void syslogger_progress(struct xentoollog_logger *logger_in,
+                                 const char *context,
+                                 const char *doing_what, int percent,
+                                 unsigned long done, unsigned long total){
+    xentoollog_logger_syslogger *lg = (void *)logger_in;
+    
+    pthread_mutex_lock(&lg->mutex);
+    syslog(get_syslog_level(XTL_PROGRESS),"%s%s" "%s: %lu/%lu  %3d%%",
+                          context?context:"", context?": ":"",
+                          doing_what, done, total, percent);
+    pthread_mutex_unlock(&lg->mutex);
+}
+
+static void syslogger_destroy(struct xentoollog_logger *logger_in) {
+    xentoollog_logger_syslogger *lg = (void*)logger_in;
+    closelog();
+    pthread_mutex_destroy(&lg->mutex);
+    free(lg);
+}
+
+static void syslogger_set_minlevel(struct xentoollog_logger *logger_in,
+                                  xentoollog_level minlevel){
+    setlogmask(LOG_UPTO(get_syslog_level(minlevel)));
+}
+
+static void syslogger_adjust_flags(struct xentoollog_logger *logger_in,
+                                  unsigned set_flags, unsigned clear_flags){
+    xentoollog_logger_syslogger *lg = (void *)logger_in;
+    unsigned new_flags = (lg->flags & ~clear_flags) | set_flags;
+    lg->flags = new_flags;
+}
+
+xentoollog_logger_syslogger *xtl_createlogger_syslogger
+        (int syslog_facility, xentoollog_level min_level, unsigned flags) {
+
+    xentoollog_logger_syslogger newlogger;
+    newlogger.flags = flags;
+    pthread_mutex_init(&newlogger.mutex,NULL);;
+
+    openlog("",LOG_CONS | LOG_NDELAY,syslog_facility);
+    setlogmask(LOG_UPTO(get_syslog_level(min_level)));
+
+    return XTL_NEW_LOGGER(syslogger, newlogger);
+}
+

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] syslog support to xentoollog

[Keir Fraser]

On 05/06/2010 01:51, "Gihan Munasinghe" <GMunasinghe@flexiant.com> wrote:

> Guys
> 
> See the attach patch, this implements a xentoollog consumer that can log
> in to syslog

I'll wait for Ian to review this one before applying it.

 -- Keir

> Following are the changes
> 
> xentoollog.h:
> 1) Generic, xtl_logger_set_minlevel and xtl_logger_adjust_flags
> functions to be called from outside,
> 2) Making logger flags to be more generic
> 3) Change the struct xentool_logger, each logger will implement its own
> "set_minlevel" and "adjust_flags"  which will be invoked by
> xtl_logger_set_minlevel and xtl_logger_adjust_flags generic functions.
> 4) New function prototype to create a xentool_logger_syslogger
> 
> Going forward I would like to suggest that xentoollog.h file should only
> have generic functions, except for xtl_createlogger_* functions. Logger
> type specific function should not be included(e.g
> xtl_stdiostream_set_minlevel ) this will make sure outside code can
> switch from one logger to another (from stdiologger to syslogger vice
> versa) with out breaking the code.
> 
> xtl_core.c:
> 1) Implementation of xtl_logger_set_minlevel and xtl_logger_adjust_flags
> functions.
> 
> xtl_logger_syslog.c :
> New logger implementation to log in to syslog.
> 
> 
> I did some testing by changing the code in xl. This seems to work, but I
> would really like if more people can test this.
> Also could someone look @ the syslogger_progress function and add any
> extra bits if needed, I did a very basic implementation of this.
> 
> Signed-off-by : Gihan Munasinghe <GMunasinghe@flexiant.com>
> 
> 
> Thanks
> Gihan

Re: [PATCH] syslog support to xentoollog

[Ian Jackson]

Gihan Munasinghe writes ("[PATCH] syslog support to xentoollog"):
> Going forward I would like to suggest that xentoollog.h file should only 
> have generic functions, except for xtl_createlogger_* functions. Logger 
> type specific function should not be included(e.g 
> xtl_stdiostream_set_minlevel ) this will make sure outside code can 
> switch from one logger to another (from stdiologger to syslogger vice 
> versa) with out breaking the code.

In general this is a good idea but it's not sensible to make it a hard
and fast rule.  Eg, if you had a function xtl_syslog_change_facility()
it wouldn't make any sense for it to be implemented by stdiostream.

Ian.

Re: [PATCH] syslog support to xentoollog

[Ian Jackson]

Gihan Munasinghe writes ("[PATCH] syslog support to xentoollog"):
> Also could someone look @ the syslogger_progress function and add any 
> extra bits if needed, I did a very basic implementation of this.  

I think it should at the very least limit the messages until the
percentage changes by at least (say) 5%.  The progress function may be
called very very often.

Ian.

Re: [PATCH] syslog support to xentoollog

[Ian Jackson]

Keir Fraser writes ("Re: [Xen-devel] [PATCH] syslog support to xentoollog"):
> I'll wait for Ian to review this one before applying it.

Great, thanks.  I think there are a few things that need doing.

Ian.

Re: [PATCH] syslog support to xentoollog

[Gihan Munasinghe]

Ian Jackson wrote:
> Gihan Munasinghe writes ("[PATCH] syslog support to xentoollog"):
>   
>> Going forward I would like to suggest that xentoollog.h file should only 
>> have generic functions, except for xtl_createlogger_* functions. Logger 
>> type specific function should not be included(e.g 
>> xtl_stdiostream_set_minlevel ) this will make sure outside code can 
>> switch from one logger to another (from stdiologger to syslogger vice 
>> versa) with out breaking the code.
>>     
>
> In general this is a good idea but it's not sensible to make it a hard
> and fast rule.  Eg, if you had a function xtl_syslog_change_facility()
> it wouldn't make any sense for it to be implemented by stdiostream.
>
>   
Should a call like xtl_syslog_change_facility() given out directly to 
the library users. this call can be wrapped with in a more generic call 
like
xtl_cahnge_log_place(struct xentool_logger , void *new_place);

In stdiostream implementation this can be caste to a stream and in 
syslogger implementation this can be cast as a  facility.
If some logger doesn't want to implement that we can have empty 
implementation

Well may be it should not be a hard and fast rule, but more of a best 
practice scenario then.


Thanks
Gihan

Re: [PATCH] syslog support to xentoollog

[Gihan Munasinghe]

Ian Jackson wrote:
> Gihan Munasinghe writes ("[PATCH] syslog support to xentoollog"):
>   
>> Also could someone look @ the syslogger_progress function and add any 
>> extra bits if needed, I did a very basic implementation of this.  
>>     
>
> I think it should at the very least limit the messages until the
> percentage changes by at least (say) 5%.  The progress function may be
> called very very often.
>
>   
Is this the only change that need to happen, are you happy with the rest 
of the patch.

Tanks
Gihan

Re: [PATCH] syslog support to xentoollog

[Ian Jackson]

Gihan Munasinghe writes ("[PATCH] syslog support to xentoollog"):
> See the attach patch, this implements a xentoollog consumer that can log 
> in to syslog

Thanks.  Here are my comments after reading the code in detail:


None of your code seems to implement min_level.  At the very least you
should be able to suppress logging of progress messages to syslog.

> +//-- Should be depricated not generic enough --//

Could you please to use the same comment style as the rest of this
file ?  Switching styles in a single file makes the it particularly
hard to read.

> +/**
> +Create a logger that will log to syslog 
> +syslog_facility is the facilities that are in the sys/syslog.h
> +edit /etc/syslog.conf file to indicate where you wansyslog to send log data

Block comments should be
  /*
   * Like this
   */
so that they can be easily distinguished from code.

> +//This method need to be depricated use more generic xtl_logger_set_minlevel method insted
>  void xtl_stdiostream_set_minlevel(xentoollog_logger_stdiostream*,
>                                    xentoollog_level min_level);

If we are going to make these methods of every logger, you can just
delete these two functions.  This logging stuff is new enough that we
don't care about backward API compatibility.  Also "deprecated" is
spelt thus.

> +void xtl_logger_set_minlevel(struct xentoollog_logger *logger,
> +                             xentoollog_level min_level){
> +    if (!logger || !logger->set_minlevel) return; 

I think the test for !logger is wrong: I think if you call this
function and pass it logger=NULL you should get a crash.  But it's
fine to skip the call for loggers that don't support it (ie have 0 in
the vtable).

>  void xtl_logger_destroy(struct xentoollog_logger *logger) {
> -    if (!logger) return;
> +    if (!logger || !logger->destroy) return;

This is wrong because every logger must definitely have a ->destroy
method, since it at least has to free up the logger structure.  Any
which didn't must either be broken, or be some kind of static logger
which should never be destroyed.

> +typedef struct _log_map{
> +    xentoollog_level xen_log;
> +    int syslog_level;
> +}LOG_MAP;

Do not use identifiers which start with "_"; they are reserved for the
C implementation.  (In this case it's OK I think but it's a very bad
habit.)  There seems to be no need for this struct to have a struct
tag as well as a typedef.

Also, as a matter of style, I think the use of all caps for type names
is a bad idea.  Capslock is normally used for macros.

> +LOG_MAP log_mappings[] = 
> +    {

You forgot "static const".

> +int maptable_len = sizeof(log_mappings)/sizeof(LOG_MAP);

This should be a #define, surely.  If not, you forgot "static const".
But it's only used in one place.  Perhaps you should put it there.

> +    buffer = (char *)calloc(1024,sizeof(char));

sizeof(char) is 1 by definition.

You should not cast the result from calloc.

> +    if (context && (lg->flags & XTL_SHOW_PID)){

syslog already knows how to add the pid to messages.

> +       sprintf(new_format,"[%s] : ",context);
...
> +       sprintf(new_format,"[%s] : ",strerror(errnoval));

This formatting is different to that produced by stdiostream.  Surely
the messages should try to be as similar as possible.

> +       char *new_format = (char *) calloc(7+strlen(context),sizeof(char));
> +       sprintf(new_format,"[%s] : ",context);
> +       strcat(buffer,new_format);

Use asprintf.  In fact you could profitably replace much of the meat
of this function with a single asprintf call.

> +    strcat(buffer,format);

This is wrong.  What if the context or the errno value contain "%s" or
something ?  You can prepend a single "%s" to the format if you want
to prepend a fixed string.

> +    syslog(get_syslog_level(XTL_PROGRESS),"%s%s" "%s: %lu/%lu  %3d%%",
> +                          context?context:"", context?": ":"",
> +                          doing_what, done, total, percent);

You should arrange that the syslog level lookup - a linear search - is
not done on every call to ->progress, which may be called very very
requently.

Ian.

Re: [PATCH] syslog support to xentoollog

[Ian Jackson]

Gihan Munasinghe writes ("Re: [PATCH] syslog support to xentoollog"):
> Should a call like xtl_syslog_change_facility() given out directly to 
> the library users. this call can be wrapped with in a more generic call 
> like
> xtl_cahnge_log_place(struct xentool_logger , void *new_place);

This is a bad idea because the type and semantics of new_place are not
defined.

> In stdiostream implementation this can be caste to a stream and in 
> syslogger implementation this can be cast as a  facility.

Yuck.

Ian.