On Tue Apr 15, 2025 at 7:21 AM BST, Jan Beulich wrote:
> On 14.04.2025 20:07, Alejandro Vallejo wrote:
>> On Wed Apr 9, 2025 at 11:15 PM BST, Denis Mukhin wrote:
>>> On Tuesday, April 8th, 2025 at 9:07 AM, Alejandro Vallejo
>>> <agarc...@amd.com> wrote:
>>>> @@ -233,6 +264,12 @@ static int __init process_domain_node(
>>>> return -ENODATA;
>>>> }
>>>>
>>>> + if ( bd->domid == DOMID_INVALID )
>>>>
>>>> + bd->domid = get_initial_domain_id();
>>>>
>>>> + else if ( bd->domid != get_initial_domain_id() )
>>>>
>>>> + printk(XENLOG_WARNING
>>>> + "WARN: Booting without initial domid not supported.\n");
>>>
>>> Drop WARN since the log message is XENLOG_WARNING level already?
>>
>> As mentioned elsewhere, the point of those prefixes are to be readable.
>
> This, however, imo is a matter of consistency across the codebase, not just
> within hyperlaunch.
I agree. There is precedent though for certain printks to have a
collective pattern for ease of reading (e.g: spec_ctrl.c when printing
mitigations). With I'm merely justifying the 2 spaces followed by
lowercase.
I did try to remove them and it was strictly harder to know what they
referred to.
> Plus (again imo) if anything, prefixes that are part of
> the log output should contain proper words ("Warning:" or "Error:"), and
> they shouldn't needlessly "shout" (i.e. "FATAL:" is okay-ish to be all caps,
> but the others aren't).
>
> Jan
Right. I'm happy to rewrite them as "Warning: ..." and "Error: ..."
Cheers,
Alejandro
