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

[Viktor Mihajlovski]

On 04.07.2017 11:35, David Hildenbrand wrote:
>> +
>>  static void create_cpu_model_list(ObjectClass *klass, void *opaque)
>>  {
>> -    CpuDefinitionInfoList **cpu_list = opaque;
>> +    struct CpuDefinitionInfoListData *cpu_list_data = opaque;
>> +    CpuDefinitionInfoList **cpu_list = &cpu_list_data->list;
>>      CpuDefinitionInfoList *entry;
>>      CpuDefinitionInfo *info;
>>      char *name = g_strdup(object_class_get_name(klass));
>> @@ -300,7 +331,19 @@ static void create_cpu_model_list(ObjectClass *klass, 
>> void *opaque)
>>      info->migration_safe = scc->is_migration_safe;
>>      info->q_static = scc->is_static;
>>      info->q_typename = g_strdup(object_class_get_name(klass));
>> -
> 
> Wonder if we should add the following:
> 
> /* the host model can never be used under TCG */
> if (scc->kvm_required && !kvm_enabled())) {
>     info->has_unavailable_features = true;
>     list_add_feat("type", &info->unavailable_features);
> } else if (cpu_list_data->model ...
> 
>> +    /* check for unavailable features */
>> +    if (cpu_list_data->model) {
>> +        Object *obj;
>> +        S390CPU *sc;
>> +        obj = object_new(object_class_get_name(klass));
>> +        sc = S390_CPU(obj);
> 
> So we can drop the check here (if we have a host/max_model definition
> under KVM, then all models have sc->model set). But the current code
> should also work (it simply will not indicate runability information for
> the "host" model under TCG, as sc->model will be NULL).
Hmm ... that made me think that we could remove the check here and make
check_unavailable_features interpret a NULL model as not
usable/incompatible (a bit more robust in the unlikely event of a memory
allocation failure). Thoughts?
> >> +        if (sc->model) {
>> +            info->has_unavailable_features = true;
>> +            check_unavailable_features(cpu_list_data->model, sc->model,
>> +                                       &info->unavailable_features);
>> +        }
>> +        object_unref(obj);
>> +    }
>>  
> Looks good to me and produced the expected result under TCG.
> 
> Feel free to add my
> 
> Reviewed-by: David Hildenbrand <address@hidden>
> 
> with or without the modification.
> 


-- 

Mit freundlichen Grüßen/Kind Regards
   Viktor Mihajlovski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294