Re: [PATCH v4 3/3] xen: introduce CONFIG_HAS_DOMAIN_TYPE

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.05.2026 09:44, Oleksii Kurochko wrote:
> On 5/4/26 2:21 PM, Jan Beulich wrote:
>> On 27.04.2026 17:34, Oleksii Kurochko wrote:
>>> @@ -69,6 +70,14 @@ static inline uint32_t alloc_phandle(struct kernel_info 
>>> *kinfo)
>>>       return kinfo->next_phandle >= GUEST_PHANDLE_GIC ? 0 : 
>>> kinfo->next_phandle++;
>>>   }
>>>   
>>> +static inline void set_domain_type(struct domain *d, struct kernel_info 
>>> *kinfo)
>>
>> Pointer-to-const for the 2nd parameter?
> 
> I will apply this comment and comment above.
> 
>>
>>> +{
>>> +#ifdef CONFIG_HAS_DOMAIN_TYPE
>>> +    /* Type must be set before allocate memory */
>>
>> This comment would be more prominent if it lived outside of the #ifdef,
>> perhaps (read on) ahead of the function. I wonder though why it's only
>> a comment, and not e.g. an assertion. If an assertion was possible to
>> add, the comment would want to live next to it. Without an assertion
>> putting it ahead of the function may be better.
>>
>> Depending on how far to go, changes could be made while committing, or a
>> proper v5 may want submitting.
> 
> I think that instead of comment or just after comment the following 
> could be added:
>    ASSERT(!domain_tot_pages(d));
> 
> Jan, Michal, do you see any concern with that ASSERT() or I could add it 
> and keep your Ack-by and R-by.

I'm okay with it being added, as long as you have made sure that it is
legitimate to have. IOW (as pointed out numerous times before) you may
not assert on state that's user/admin controlled, and that isn't covered
by another, earlier check. In such a case an error would need returning
instead.

Jan