Re: [Qemu-devel] [PATCH 10/10] machine: Skip global registration for non-existing classes

[Marcel Apfelbaum <marcel@redhat.com>]

On 06/19/2016 07:39 PM, Marcel Apfelbaum wrote:
> On 06/15/2016 11:32 PM, Eduardo Habkost wrote:
>> MachineClass::compat_props may point to class names that are not
>> compiled into the QEMU binary. Skip registering those as global
>> properties. This will allow the qdev global property code to
>> implement stricter checks on the global property values in the
>> future.
>>
>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>> ---
>>   hw/core/machine.c | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/hw/core/machine.c b/hw/core/machine.c
>> index d6f6be4..51697cb 100644
>> --- a/hw/core/machine.c
>> +++ b/hw/core/machine.c
>> @@ -583,6 +583,7 @@ static void machine_class_finalize(ObjectClass *klass, void *data)
>>   void machine_register_compat_props(MachineState *machine)
>>   {
>>       MachineClass *mc = MACHINE_GET_CLASS(machine);
>> +    ObjectClass *oc;
>>       int i;
>>       GlobalProperty *p;
>>
>> @@ -592,6 +593,14 @@ void machine_register_compat_props(MachineState *machine)
>>
>>       for (i = 0; i < mc->compat_props->len; i++) {
>>           p = g_array_index(mc->compat_props, GlobalProperty *, i);
>
> Hi Eduardo,
>
>> +
>> +        /* Skip registering globals for non-existing device classes */
>> +        oc = object_class_by_name(p->driver);
>
> This I understand. It ensure a corresponding class is in QEMU binary.
> But even so, if such property is not available should we silently continue?
> Maybe the user things the property is set...

Please forgive the spelling errors and disregard the above question.

>
>
>> +        oc = object_class_dynamic_cast(oc, TYPE_DEVICE);
>
> This I don't understand. Do we check here?
>

Regarding my last question, it seems you followed an existing pattern
here, however I don't quite understand, we found the class,
why do we need to ensure is a TYPE_DEVICE?

What error case do we want to prevent?

Thanks,
Marcel

> Thanks,
> Marcel
>
>> +        if (!oc) {
>> +            continue;
>> +        }
>> +
>>           /* Machine compat_props must never cause errors: */
>>           p->errp = &error_abort;
>>           qdev_prop_register_global(p);
>>
>