[CRIU] [PATCH 4/4] log: Introduce log levels

[Cyrill Gorcunov]

On Fri, Feb 17, 2012 at 02:24:38PM +0400, Kir Kolyshkin wrote:
> >+		case 'v':
> >+			set_loglevel((unsigned int)atoi(optarg));
> >+			break;
> 
> Can we please make -v argument optional? Like if there's just -v, we
> increase the log level by one
> (by two in case of -vv etc), or if you specify an argument it is
> explicit log level.

Yeah, good idea Kir. I somehow forgot about this convention.

> >  	printk("\nAdditional common parameters:\n");
> >  	printk("  -D dir         save checkpoint files in specified directory\n");
> >+	printk("  -v num         settup logging level\n");
> s/settup/set/
> >+	printk("                 0 - silent\n");
> This is not actually silent, right?
> 
> s/silent/silent (only error messages)/
> 

Yeah, error messages should remain. Thanks.

> >+#define LOG_SILENT	(1)
> I would rename LOG_SILENT to LOG_ERROR
> 

OK

> >+#define LOG_INFO	(2)
> >+#define LOG_DEBUG	(3)
> >+
> >+extern void set_loglevel(unsigned int level);
> >+extern void printk_level(unsigned int level, const char *format, ...) __attribute__ ((__format__ (__printf__, 2, 3)));
> 
> I am not sure what our coding style is, and I tend to agree that
> maybe 80 chars wide is way too restrictive, but let's not make very
> long lines, it's even hard to review.
> 
> What about 100 characters per line limit?
> 

ouch, yeah, better to split it and stick with 90-100 as max.
This one is definitely overlong, no doubts.

> >+
> >+#define printk(fmt, ...)	printk_level(LOG_SILENT, fmt, ##__VA_ARGS__)
> 
> Why printk has a LOG_SILENT level? Shouldn't it be LOG_INFO?

Yes, agreed, thanks Kir, I'll update!

	Cyrill