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 <[email protected]>
>> ---
>> 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
