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.
> The prior
> printk() simply ended up defaulting to a warning, and I would think
> that's what the new one should be doing too.
WARNING isn't exactly correct for a fatal event.
Ideally, we want a non-ratelimited G_ERR, but that doesn't exist. If it
appears in the future, we can use it.
The set of people running with loglvl=error is almost certainly empty,
so it doesn't matter.
~Andrew
