[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] xen: Modify domain_crash() to take a print string
On 03/02/2022 15:06, Jan Beulich wrote: > On 03.02.2022 15:41, Andrew Cooper wrote: >> On 03/02/2022 14:19, Jan Beulich wrote: >>> On 03.02.2022 15:11, Andrew Cooper wrote: >>>> On 03/02/2022 13:48, Julien Grall wrote: >>>>> On 03/02/2022 13:38, Andrew Cooper wrote: >>>>>> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h >>>>>> index 37f78cc4c4c9..38b390d20371 100644 >>>>>> --- a/xen/include/xen/sched.h >>>>>> +++ b/xen/include/xen/sched.h >>>>>> @@ -736,10 +736,15 @@ void vcpu_end_shutdown_deferral(struct vcpu *v); >>>>>> * from any processor. >>>>>> */ >>>>>> void __domain_crash(struct domain *d); >>>>>> -#define domain_crash(d) do >>>>>> { \ >>>>>> - printk("domain_crash called from %s:%d\n", __FILE__, >>>>>> __LINE__); \ >>>>>> - >>>>>> __domain_crash(d); \ >>>>>> -} while (0) >>>>>> +#define domain_crash(d, ...) \ >>>>>> + do { \ >>>>>> + if ( count_args(__VA_ARGS__) == 0 ) \ >>>>>> + printk("domain_crash called from %s:%d\n", \ >>>>>> + __FILE__, __LINE__); \ >>>>> I find a bit odd that here you are using a normal printk >>>> That's unmodified from before. Only reformatted. >>>> >>>>> but... >>>>> >>>>> >>>>>> + else \ >>>>>> + printk(XENLOG_G_ERR __VA_ARGS__); \ >>>>> here it is XENLOG_G_ERR. In fact, isn't it ratelimited? If so, >>>>> wouldn't it be better to only use XENLOG_ERR so they can always be >>>>> seen? (A domain shouldn't be able to abuse it). >>>> Perhaps. I suppose it is more important information than pretty much >>>> anything else about the guest. >>> Indeed, but then - is this really an error in all cases? >> Yes. It is always a fatal event for the VM. > Which may or may not be Xen's fault. If the guest put itself in a bad > state, I don't see why we shouldn't consider such just a warning. Log level is the severity of the action, not who's potentially to blame for causing the situation. Furthermore, almost all callers who do emit appropriate diagnostics before domain_crash() already use ERR. And, as already pointed out, it doesn't matter. The line is going out on the console however you want to try and bikeshed the internals. > IOW > I continue to think a log level, if so wanted, should be supplied by > the user of the macro. That undermines the whole purpose of preventing callers from being able to omit diagnostics. ~Andrew
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |