Re: [PATCH] domain: skip more stuff for idle's vCPU-s in vcpu_create()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.02.2026 16:06, Roger Pau Monné wrote:
> On Wed, Nov 26, 2025 at 11:31:46AM +0100, Jan Beulich wrote:
>> Nothing hypercall-related needs setting up there. Nor do we need to
>> check whether the idle domain is shutting down - it never will.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> ---
>> For vmtrace_alloc_buffer() adding the conditional may be questionable: The
>> function checks d->vmtrace_size first thing, bailing immediately when it's
>> zero (which it always will be for the idle domain).
> 
> Oh, I notice this now (after having written the comment below).  I
> would then add a comment rather than the extra check.  And possibly an
> ASSERT(!is_idle_domain(d)); inside of `vmtrace_alloc_buffer()` after
> the vmtrace_size check.

Hmm, we could move the call anyway, as you ...

>> @@ -450,7 +450,7 @@ struct vcpu *vcpu_create(struct domain *
>>      if ( sched_init_vcpu(v) != 0 )
>>          goto fail_wq;
>>  
>> -    if ( vmtrace_alloc_buffer(v) != 0 )
>> +    if ( !is_idle_domain(d) && vmtrace_alloc_buffer(v) != 0 )
>>          goto fail_wq;
> 
> There's an existing issue here, the usage of fail_rq is wrong here.
> It should use fail_sched instead.  However I wonder whether we want to
> move the `vmtrace_alloc_buffer()` call inside the existing `else {`
> branch of the is_idle_domain() condition, as to avoid this extra
> is_idle_domain() check here?

... suggest here, thus dealing with the wrong label use at the same time.
Since this change will want backporting, I guess I'll move that to a
separate patch, though. (I don't think I'll bother with adding an
assertion in the function.)

Jan