[sheepdog] [PATCH v2] dog: log dog operation in syslog

[Hitoshi Mitake]

At Fri, 27 Dec 2013 16:45:29 +0800,
Liu Yuan wrote:
> 
> On Fri, Dec 27, 2013 at 05:25:18PM +0900, Hitoshi Mitake wrote:
> > This patch is based on a request from our internal team. Currently,
> > sheepdog doesn't provide a way of logging admin's operation. The lack
> > of this feature is serious because we cannot find any evidence of
> > operation.
> > 
> > This patch adds the logging feature in dog command. If the environment
> > variable, SHEEPDOG_DOG_LOG, has value, dog command logs its command
> > line argument with syslog. If SHEEPDOG_DOG_LOG_PATH is specified
> > additionally, the log is written to the file of the path.
> > 
> > Below is an example of the log:
> > - with standard syslog
> >   Dec 27 10:57:42 dhcpff33 sheepdog admin operation[9809]:  dog
> >   cluster format -c 1
> > - with custom path
> >   2013 Dec 27 17:20:56 :  dog vdi list
> > 
> > Signed-off-by: Hitoshi Mitake <mitake.hitoshi at lab.ntt.co.jp>
> > ---
> > 
> > v2: support logging with a custom file
> > 
> >  dog/dog.c |   61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 61 insertions(+)
> > 
> > diff --git a/dog/dog.c b/dog/dog.c
> > index b092e62..4bb22a3 100644
> > --- a/dog/dog.c
> > +++ b/dog/dog.c
> > @@ -12,6 +12,8 @@
> >  #include <stdio.h>
> >  #include <stdlib.h>
> >  #include <signal.h>
> > +#include <sys/time.h>
> > +#include <fcntl.h>
> >  
> >  #include "sheepdog_proto.h"
> >  #include "sheep.h"
> > @@ -351,6 +353,63 @@ static size_t get_nr_nodes(void)
> >  	return sd_nodes_nr;
> >  }
> >  
> > +static void log_dog_operation(int argc, char **argv)
> > +{
> > +	int length = 0, printed = 0;
> > +	char *msg;
> > +	const char *custom_log_path;
> > +
> > +	if (!getenv("SHEEPDOG_DOG_LOG"))
> > +		/* don't log operation of dog */
> > +		return;
> > +
> > +
> > +	for (int i = 0; i < argc; i++)
> > +		length += 1 + strlen(argv[i]); /* 1 is for space */
> > +
> > +	length++; /* 1 is for '\0' */
> > +	msg = xcalloc(length, sizeof(char));
> 
> This looks trick, can we simply use a static array like msg[4096] ?

Which part is tricky? I think it is simple enough. In addition,
allocating static array isn't suitable for this case. dog can receive
long argument in a case of "vdi setattr".


> 
> > +	for (int i = 0; i < argc; i++)
> > +		printed += snprintf(msg + printed, length - printed,
> > +				    " %s", argv[i]);
> > +
> > +	custom_log_path = getenv("SHEEPDOG_DOG_LOG_PATH");
> > +	if (custom_log_path) {
> > +		struct timeval tv;
> > +		struct tm tm;
> > +		char time_str[256];
> > +		int fd;
> > +
> > +		printf("custom_log_path: %s\n", custom_log_path);
> 
> This should be removed or every command will see it popping out
> 

Sorry, this is an obsolete printf(). I forgot to remove it during
changing...

Thanks,
Hitoshi