target/i386: hyperv: add stub for hyperv_syndbg_query_options

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

Posted by Paolo Bonzini 1 year, 2 months ago

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>
---
 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;
+}
-- 
2.47.0

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

Posted by Michael Tokarev 1 year, 2 months ago

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?

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

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

Posted by Paolo Bonzini 1 year, 2 months ago

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
> 
>

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

Posted by Paolo Bonzini 1 year, 2 months ago

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 (#ef
> 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
> 
>