Re: [PATCH - resend] addition of loglevel for printf in Xen HV

[Steven Rostedt <srostedt@redhat.com>]

Keir Fraser wrote:
> 
> 
> On 20/10/06 20:23, "Steven Rostedt" <srostedt@redhat.com> wrote:
> 
>> This patch really does need to go in.  Whether in its current form, or
>> modified greatly.  The ability to give the developer a level of printing
>> is a great asset.  That is why Linux has this ability.
> 
> I agree. Here are some comments:

Keir, Thanks for the comments, they are very useful.

> 
>  * We don't need 8 log levels. Noone knows the difference between
> EMERG/CRIT/ALERT, or at least I really doubt that they can be used
> consistently in a large code base. I suggest:
>     XENLOG_ERR    -- Bad errors, likely fatal (to a guest or the host)
>     XENLOG_WARN   -- Weird stuff happening, recoverable (maybe)
>     XENLOG_INFO   -- Interesting info, not too noisy
>     XENLOG_DEBUG  -- Noisy as you like

Perfectly agree. Four is fine, especially if we implement a <G> option 
for guest.

> 
>  * I think your ratelimit stuff could be usefully integrated. Instead of
> having a single threshold, have two: one above which everything is printed,
> one below which nothing is printed. In between is rate limited.

I'm not quite sure I understand this fully.

So what you are saying is instead of having the print_log_level have a 
printk_upper_log_level and a printk_lower_log_level where, as you say, 
everything below the printk_lower_log_level is printed, and the 
printk_upper_log_level where everyting above is not printed, and thus 
everything in between is rate limited? (remember 0 is ERR and 3 is DEBUG)


> 
>  * I think the guest/not-guest printing is orthogonal to the log-level
> scale. Perhaps we should have another <> specifier (<G>?) that can be used
> alongside a log level specifier. This would give another two threshold
> values (two for guest, two for non-guest) which would maybe be overkill but
> would be reasonable if we gave a dom0 tool to control it. Actually this
> could be extended to other subsystems (shadow code for example). Then we
> would have a control per subsystem falling back to default thresholds if not
> specified. That probably really is overkill!
> 
>  * Likely defaults:
>       Non-guest prints ERR/WARN always, no INFO/DEBUG
>       Guest prints ERR/WARN rate-limited, no INFO/DEBUG
>       (Like Linux, we'd probably have slacker default controls until initial
> bootstrap has completed, so you get useful info out).
> 
> What do you think?
>

I have to admit I like the ideas you have thrown to me.

So let me put this is my own words/code so that you can see what my view 
of this is.

Have 4 thresholds.

printk_dom0_upper_threshold
printk_dom0_lower_threshold
printk_guest_upper_threshold
printk_guest_lower_threshold

Have 4 log levels:

XENLOG_ERR
XENLOG_WARN
XENLOG_INFO
XENLOG_DEBUG

Add a XENLOG_GUEST (defined as "<G>")

And for less typing add

XENLOG_G_ERR
XENLOG_G_WARN
XENLOG_G_INFO
XENLOG_G_DEBUG

which would just append the XENLOG_GUEST in front of the associated warning.

example;

    printk(XENLOG_G_ERR "fatal error in guest\n");

    printk(XENLOG_INFO "dom0 is running happily\n");


Then in printk itself, we would have

   upper_thresh = printk_dom0_upper_threshold;
   lower_thresh = printk_dom0_lower_threshold;
   level = some_default;

   if (strncmp("<G>",fmt,3)==0) {
	upper_thresh = printk_guest_upper_threshold;
	lower_thresh = printk_guest_lower_threshold;
	level = some_guest_default;
	fmt += 3;
   }
   if (strncmp("<[0-3]>",fmt,3)==0) {  /* using regexpr and not C to make
                                          this readable */
	level =~ s/<(0-3)>/$1/;  /* perl format :-) */
   }

   if (level > upper_thresh)
	return; /* do nothing */

   if (level >= lower_thresh)
	if (rate_limit())
		return;

   print as normal.


So anything in between the upper and lower thresholds inclusive would be 
rate limited.  Also I'm not sure of the best way dom0 can communicate 
threshold changes to the hypervisor.  As well as if there should be a 
config to set the defaults.

So what are your thoughts?

-- Steve