Re: [RFC v2 5/5] KVM: selftests: Print the vcpu_id when KVM_CREATE_VCPU ioctl fails

[Ritesh Harjani (IBM) <ritesh.list@gmail.com>]

sashiko-bot@kernel.org writes:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The added `pr_info()` call clobbers the global `errno` variable before it can be read by the `TEST_ASSERT_VM_VCPU_IOCTL()` macro.
> --
>
> commit 9022e7c111e4121ff5a4ed4a90dcbfd2df0a62cd
> Author: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
>
> KVM: selftests: Print the vcpu_id when KVM_CREATE_VCPU ioctl fails
>
> This commit adds a debug print statement to display the vcpu_id and errno
> when the KVM_CREATE_VCPU ioctl fails.
>
>> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
>> index 0dc67c1502cfb..6a0ee68036236 100644
>> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
>> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
>> @@ -1354,6 +1354,9 @@ struct kvm_vcpu *__vm_vcpu_add(struct kvm_vm *vm, u32 vcpu_id)
>>  	vcpu->vm = vm;
>>  	vcpu->id = vcpu_id;
>>  	vcpu->fd = __vm_ioctl(vm, KVM_CREATE_VCPU, (void *)(unsigned long)vcpu_id);
>> +	if (vcpu->fd < 0)
>> +		pr_info("Failed KVM_CREATE_VCPU for vcpu_id %u with errno %d\n",
>> +			vcpu_id, errno);
>>  	TEST_ASSERT_VM_VCPU_IOCTL(vcpu->fd >= 0, KVM_CREATE_VCPU, vcpu->fd, vm);
>
> [Severity: Medium]
> Does this pr_info() call clobber errno before it can be evaluated by
> TEST_ASSERT_VM_VCPU_IOCTL()?
>
> Since pr_info() uses standard C library print functions, it can modify errno
> internally even on success. TEST_ASSERT_VM_VCPU_IOCTL() captures errno
> to detect specific failure modes and to format the assertion error message.
>
> Could this cause the test framework to print an incorrect error code or
> bypass checks like dead VM detection when KVM_CREATE_VCPU fails?
>

yup, I will fix this in v3.. Something like this -

+       if (vcpu->fd < 0) {
+               int saved_errno = errno;
+               pr_info("Failed KVM_CREATE_VCPU for vcpu_id %u with errno %d\n",
+                       vcpu_id, saved_errno);
+               errno = saved_errno;
+       }

Also I noticed that all other KVM selftests have moved from

uint64_t -> u64
uint32_t -> u32
uint8_t -> u8
int64_t -> s64
...

So, I will add separate patches on top of the current patch similar to
how it was done earlier to convert powerpc kvm selftests as well - to
keep the review simpler w/o mixing too many changes in v3.

-ritesh