Re: [PATCH] igvm: populate errp in stub functions

[Markus Armbruster <armbru@redhat.com>]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Tue, Jun 09, 2026 at 03:20:14PM +0200, Luigi Leonardi wrote:
>> Use error_setg() to report that IGVM is not available, matching
>> the pattern used by other stubs in the tree.
>> 
>> Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
>> Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
>> ---
>>  stubs/igvm.c | 3 +++
>>  1 file changed, 3 insertions(+)
>> 
>> diff --git a/stubs/igvm.c b/stubs/igvm.c
>> index 9e9f683fc9..dfb85eb548 100644
>> --- a/stubs/igvm.c
>> +++ b/stubs/igvm.c
>> @@ -17,15 +17,18 @@ int qigvm_x86_get_mem_map_entry(int index,
>>                                  ConfidentialGuestMemoryMapEntry *entry,
>>                                  Error **errp)
>>  {
>> +    error_setg(errp, "IGVM not supported on this platform");
>>      return -1;
>>  }
>>  
>>  int qigvm_x86_set_vp_context(void *data, int index, Error **errp)
>>  {
>> +    error_setg(errp, "IGVM not supported on this platform");
>>      return -1;
>>  }
>>  
>>  int qigvm_directive_madt(QIgvm *ctx, const uint8_t *header_data, Error **errp)
>>  {
>> +    error_setg(errp, "IGVM not supported on this platform");
>>      return -1;
>>  }
>
> This is not wrong per-se, so on that  basis
>
>   Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>
> but are any of these stubs actually reachable when IGVM is not
> enabled in the build ? Usually with stubs we find that one
> or two methods are the primary entrypoints which must return
> an error, at which point everything else becomes unreachable.
> The latter cases can just be g_assert_not_reached() as a sanity
> check that some unexpected codepath isn't calling in without
> checking status earlier. Those would thus would not need
> error_setg, nor a 'return' statement.

I much prefer g_assert_not_reached() to unreachable (and thus
untestable) errors.