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. IOW
I continue to think a log level, if so wanted, should be supplied by
the user of the macro.
Jan
