Re: [PATCH] domain: move vmtrace_alloc_buffer() invocation in vcpu_create()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.02.2026 17:29, Andrew Cooper wrote:
> On 16/02/2026 3:51 pm, Jan Beulich wrote:
>> The label used upon the function failing is wrong.
> 
> Is it?  Which label do you think it ought to be?

fail_sched, as Roger did point out in reply to the original other patch.
After all ...

>>  Instead of correcting
>> the label, move the invocation up a little, to also avoid it altogether
>> for the idle domain (where ->vmtrace_size would be zero, and hence the
>> function would bail right away anyway).
>>
>> Fixes: 217dd79ee292 ("xen/domain: Add vmtrace_size domain creation 
>> parameter")
>> Reported-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>
>> --- a/xen/common/domain.c
>> +++ b/xen/common/domain.c
>> @@ -493,14 +493,14 @@ struct vcpu *vcpu_create(struct domain *
>>          set_bit(_VPF_down, &v->pause_flags);
>>          vcpu_info_reset(v);
>>          init_waitqueue_vcpu(v);
>> +
>> +        if ( vmtrace_alloc_buffer(v) != 0 )
>> +            goto fail_wq;
>>      }
>>  
>>      if ( sched_init_vcpu(v) != 0 )
>>          goto fail_wq;

... this comes first, and ...

>> -    if ( vmtrace_alloc_buffer(v) != 0 )
>> -        goto fail_wq;
>> -
>>      if ( arch_vcpu_create(v) != 0 )
>>          goto fail_sched;

... here the correct label is used.

Jan

> The positioning was intentional.  I just didn't get to wiring up Intel
> PT for Xen context yet, and tying the buffer to the idle vCPU would be
> the obvious choice there.
> 
> The chances of getting around to that are admittedly low, so I don't
> mind moving the call in principle (noting that this is a wish that
> likely won't materialise), but the claim over the label needs resolving.
> 
> ~Andrew