On 04.07.2017 13:22, Viktor Mihajlovski wrote:
> 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?
In fact, all of this may be moot anyway: libvirt blacklists the host
model as I found out, after wondering why there was no "host" CPU with
unknown usability in the virsh domcapabilities output.
I'll take your r-b then, and abstain from posting my v4 :-).
>>>> + 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 <da...@redhat.com>
>>
>> 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
