Re: [PATCH v3 12/16] x86/hyperlaunch: add domain id parsing to domain config

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.04.2025 13:37, Alejandro Vallejo wrote:
> 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 
>>>> <agarciav@xxxxxxx> 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).
> 
> Right. I'm happy to rewrite them as "Warning: ..." and "Error: ..."

FTAOD - in the common case I'd prefer such prefixes to be omitted. Which
still means that there may be special cases where having them is warranted.

Jan