Re: [PATCH] target/i386: hyperv: add stub for hyperv_syndbg_query_options

[Paolo Bonzini <pbonzini@redhat.com>]

On 11/14/24 13:41, Michael Tokarev wrote:
> 14.11.2024 15:15, Paolo Bonzini wrote:
>> Building without CONFIG_HYPERV is currently broken due to a missing
>> symbol 'hyperv_syndbg_query_options'.  Add it to the stubs
>> that exist for that very reasons.
>>
>> Reported-by: Michael Tokarev <mjt@tls.msk.ru>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> Rewviewed-by: Michael Tokarev <mjt@tls.msk.ru>
> 
> I'm a bit confused though, - why a stub is "better" than an #ifdef,
> especially in such simple cases?

To be honest the #ifdef was the first thing I did (#ifdef 
CONFIG_HYPERV).  I switched to the stub just to avoid doing the same 
thing in two different ways.

In general I prefer stubs because they put all the code in one place. 
For example there is a benefit, which doesn't apply here, when you have 
stubs with Error** arguments.  Then it's easier to make the error text 
consistent.

Paolo


> Restoring the #ifdef around this place fixes the build.
> I understand if the function in question were used in lots of
> places around the code, but here it's not the case.
> 
> Another option would be, instead of stubs, to use:
> 
> #ifndef CONFIG_SYNDBY
> #define hyperv_syndbg_query_options() 0
> #endif
> 
> which will make stubs unnecessary entirely.
> 
>> ---
>>   target/i386/kvm/hyperv-stub.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/target/i386/kvm/hyperv-stub.c b/target/i386/kvm/hyperv- 
>> stub.c
>> index 3263dcf05d3..5836f53c23b 100644
>> --- a/target/i386/kvm/hyperv-stub.c
>> +++ b/target/i386/kvm/hyperv-stub.c
>> @@ -56,3 +56,8 @@ void hyperv_x86_synic_update(X86CPU *cpu)
>>   void hyperv_x86_set_vmbus_recommended_features_enabled(void)
>>   {
>>   }
>> +
>> +uint64_t hyperv_syndbg_query_options(void)
>> +{
>> +    return 0;
>> +}
> 
> Thanks,
> 
> /mjt
> 
>