Re: [PATCH v2 12/20] KVM: selftests: Add APIs to override memory region types with custom memslots

[Itaru Kitayama]

On Thu, Sep 10, 2026 at 09:24:14AM -0700, Sean Christopherson wrote:
> On Thu, Sep 10, 2026, Ritesh Harjani wrote:
[...]
> > May I suggest few changes in the naming of these APIs:
> > 
> > static inline void vm_override_mem_region(struct kvm_vm *vm,
> > 					      enum kvm_mem_region_type type,
> > 					      u32 slot)
> > {
> > 	TEST_ASSERT(vm->memslots[type] == KVM_INVALID_MEMSLOT,
> > 		    "Memory region type '%u' was already overridden with slot=%u",
> > 		    type, vm->memslots[type]);
> > 
> > 	vm->memslots[type] = slot;
> > }
> > 
> > static inline void vm_override_add_mem_region_flags(struct kvm_vm *vm,
> > 					    enum kvm_mem_region_type type,
> > 					    enum vm_mem_backing_src_type src_type,
> > 					    gpa_t gpa, u32 slot, u64 npages,
> > 					    u32 flags)
> > {
> > 	vm_override_mem_region(vm, type, slot);
> > 	vm_userspace_mem_region_add(vm, src_type, gpa, slot, npages, flags);
> > }
> > 
> > static inline void vm_override_add_mem_region(struct kvm_vm *vm,
> > 					  enum kvm_mem_region_type type,
> > 					  enum vm_mem_backing_src_type src_type,
> > 					  gpa_t gpa, u32 slot, u64 npages)
> > {
> > 	vm_override_add_mem_region_flags(vm, type, src_type, gpa, slot, npages, 0);
> > }
> > 
> > Those "_add_" and "_flags" in the function names easily gives away
> > the difference in the APIs, rather than differentiating via "__".
> 
> I am strongly against postfixes like "_flags".  We do use postfixes for the VM
> creation APIs, e.g. vm_create_barebones(), vm_create_with_vcpus(), etc., but only
> because there are so many possible combinations that differentiating through
> underscores is completely infeasible (unless we forced callers to regurgitate
> huge amounts of boilerplace and/or had an absurd number of params), and because
> the collection of APIs is tree-like, as opposed to a single linear chain of APIs.
> 
> For this, there is a much more finite set of possibilities, and the set of APIs
> is a direct liner chain (no underscores => __ => ____).
> 
> I agree that not capturing that {,__}vm_override_mem_region() adds a userspace
> memory region isn't ideal, but due to the term "memory region" already being
> somewhat overloaded, I don't want to have "add" in the name as I think that will
> make it harder to differentiate between the "enum kvm_mem_region_type" APIs and
> the "userspace memory region" APIs.
> 
> And IMO it's totally fine for a function name to express what the API does at a
> higher level, without capturing the exact operations in explicit detail.  E.g.
> __vm_create() (and even ____vm_create()) obviously does a lot more than literally
> KVM_VM_CREATE.

As I have been looking at the direct liner chain of vm_create() for some
time, I agree with Sean. To create with a non-default guest mode ID, we have to
start with those underscored functions, but I still prefer short function
names.

Thanks,
Itaru.