Re: [PATCH v4 03/11] KVM: Export kvm_uevent_notify_vm_create()

[email protected] writes:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] The patch claims to export `kvm_uevent_notify_vm_create()` but fails to include the `EXPORT_SYMBOL_GPL()` or `EXPORT_SYMBOL_FOR_KVM_INTERNAL()` macro.
> --
>
> --- Patch [3]: [PATCH v4 03/11] KVM: Export kvm_uevent_notify_vm_create() ---
> commit 5e642ea16f0d267e622a352de60bfe397ecf79df
> Author: Tarun Sahu <[email protected]>
>
> KVM: Export kvm_uevent_notify_vm_create()
>
> This commit exposes the kvm_uevent_notify_vm_create function to allow
> triggering KVM event notifications during VM creation.
>
> [ ... ]
>
>> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
>> --- a/virt/kvm/kvm_main.c
>> +++ b/virt/kvm/kvm_main.c
>> @@ -6348,6 +6348,11 @@ static void kvm_uevent_notify_change(unsigned int type, struct kvm *kvm)
>>  	kfree(env);
>>  }
>>
>> +void kvm_uevent_notify_vm_create(struct kvm *kvm)
>> +{
>> +	kvm_uevent_notify_change(KVM_EVENT_CREATE_VM, kvm);
>> +}
>
> [Severity: Low]
> The commit message mentions exporting this function, but is it missing an
> EXPORT_SYMBOL_GPL or EXPORT_SYMBOL_FOR_KVM_INTERNAL declaration?
>
> Without the macro, won't dynamically loaded architecture-specific modules
> like kvm-intel.ko hit undefined symbol linking errors when they try to
> call it?
>

I'm not 100% sure about the use of EXPORT_SYMBOL... here but usually I
compile kvm as a module to try and catch these types of linking
issues. Maybe Sean will have a better method/explanation for this.

This brings about another question I have though. How does luo work if
the new kernel's deserialization of the persisted data structures
depends on a module? Like in this case, KVM is a module, if KVM isn't
loaded in the new kernel, do the data structures stick around until KVM
is loaded? Or is KVM required to be loaded as a module in the new
kernel?

> --
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3