Re: [Qemu-devel] [PATCH v2] s390: return unavailable features via query-cpu-definitions

[David Hildenbrand <david@redhat.com>]

>> Wonder if we should declare all these prototypes at the beginning of the
>> file.
>>
> Don't know either. Looking around in QEMU, forward declarations for
> static functions seem to be used sparsely (only when needed). I could
> have reordered the functions to get around without forward decls but
> this would have obscured the change. Maybe defer and clean up in a
> general cleanup/refactoring?

Yes, fine with me!
[...]

>>
>> Hmmmm, if this function fails, we will create the same error multiple
>> times (as there is no way to stop this function from iterating). And we
>> will fail to create a cpu model list in case there is no host cpu model,
>> which is a change in behavior (as we will report an error).
>>
>> Would it be better to simply get the max model in
>> arch_query_cpu_definitions() and pass it via CpuDefinitionInfoListData,
>> instead of the errp variable?Simplifies things, I like it.
>>
>> Then you could simply skip the checks and set
>> info->has_unavailable_features = false in case there is no max model
>> (get_max_cpu_model() returned NULL / an error). (same behavior as for now)
>>
>> Errors from get_max_cpu_model() then should be ignored and not reported.
>>
> Just to be sure: you suggest that I should call error_free() after
> calling get_max_cpu_model, in order to prevent that the QMP command
> query-cpu-definitions fails, right?

That would be my suggestion simply don't provide runability information,
if we can't tell (because the max model is not available - e.g. with old
KVM versions without complete CPU model support), hiding the error.

-- 

Thanks,

David