Re: [Qemu-devel] [PATCH v5] qom, qmp, hmp, qapi: create qom-type-prop-list for class properties

[Valentin Rakush]

Hi Eduardo,

thank you for your explanations.

> You don't have to, if you just do object_new() like

> qmp_device_list_properties() does. Both ObjectClass::properties

> and DeviceClas::props are translated to object instance

> properties (Object::properties).

I should foresee these. Qemu has the object oriented approach implemented in C. 

I missed this point. Thank you for explanation. 

I did the following changes in the target-i386/cpu.c

     dc->props = host_x86_cpu_properties;

     /* Reason: host_x86_cpu_initfn() dies when !kvm_enabled() */

-    dc->cannot_destroy_with_object_finalize_yet = true;

+    dc->cannot_destroy_with_object_finalize_yet = false;

and 

     /*

      * Reason: x86_cpu_initfn() calls cpu_exec_init(), which saves the

      * object in cpus -> dangling pointer after final object_unref().

      */

-    dc->cannot_destroy_with_object_finalize_yet = true;

+    dc->cannot_destroy_with_object_finalize_yet = false;

and now I can add class properties to the target-i386/cpu.c from this patch 

http://lists.nongnu.org/archive/html/qemu-devel/2015-08/msg03117.html 

and then command "qemu-system-x86_64 -device core2duo-x86_64-cpu,help"

will show me the class properties.  

However according to this patch 

http://lists.nongnu.org/archive/html/qemu-devel/2015-10/msg05013.html 

I am checking how to make cpu class not to fail in device-list-properties call

throught QMP interface.

Basically my goal is to put class properties into the target-i386/cpu.c and 

print them when necessary. And after our discussion qmp_device_list_properties 

is a already available for this but cannot_destroy_with_object_finalize_yet 

flag should be set to false.

Please let me know if this is wrong approach. 

Thank you,

Valentin

On Tue, Feb 2, 2016 at 6:55 PM, Eduardo Habkost <address@hidden> wrote:

On Sun, Jan 31, 2016 at 04:40:54PM +0300, Valentin Rakush wrote:
> Hi Eduardo,
>
> I will try to answer some of your questions at this email and will answer
> other questions later.
>
> > Can you clarify what you mean by "TYPE_DEVICE has its own
> > properties"? TYPE_DEVICE properties are registered as normal QOM
> > properties.
>
> It is possible that I do not understand object model correctly....
>
> This commit 16bf7f522a2f adds GHashTable *properties; to the ObjectClass
> struct in the include/qom/object.h
> The typedef struct DeviceClass from include/hw/qdev-core.h is inherited
> from ObjectClass. Also DeviceClass has it own properties
> Property *props.
>
> In the device_list_properties we call
>
> static DevicePropertyInfo *make_device_property_info
>
> Which tries to downcast class to DEVICE_CLASS
>
> for (prop = DEVICE_CLASS(klass)->props; prop && prop->name; prop++) {
>
> So we are using Property *props, defined in the DeviceClass, but we do not
> use GHashTable * properties, defined in the ObjectClass. Here I mean that
> DeviceClass has its own properties.

Oh, I misunderstood you. I was talking about object properties,
the ones at Object::properties. Yes, in this case we have
duplication between DeviceClass::props and
ObjectClass::properties.

>
> > I don't understand what you mean, here. GlobalProperties are not
> > machine properties, they are just property=value pairs to be
> > registered as global properties. They are unrelated to the
> > properties TYPE_MACHINE actually has.
>
> Same here. The struct MachineClass is defined in the include/hw/boards.h It
> has a member GlobalProperty *compat_props;
> But after commit 16bf7f522a2f it would be better to use ObjectClass
> properties. IMHO. I did not check how compat_props are used in the code yet.

In this case it's different: ObjectClass::compat_props are not
machine properties. They are just property=value pairs to be
registered as global properties when running the machine. They
will never appear in qom-type-prop-list because they are a
completely different thing.

>
> > Could you clarify what you mean by "process different classes
> > differently"?
>
> In the list_device_properties function we should have several conditional
> statements like
>
> if (machine = object_class_dynamic_cast(class, TYPE_MACHINE)) {
> /* process machine properties using MachineClass GlobalProperty
> *compat_props; */
> }
> else if (machine = object_class_dynamic_cast(class, TYPE_DEVICE)) {
> /* process device class properties, using DeviceClass Property *props; */
> }
> else if (machine = object_class_dynamic_cast(class, TYPE_CPU)) {
> /* process CPU, using ObjectClass GHashTable *properties; */
> }

You don't have to, if you just do object_new() like
qmp_device_list_properties() does. Both ObjectClass::properties
and DeviceClas::props are translated to object instance
properties (Object::properties).

>
> > 5) -cpu options:
> >
> > Ditto. the list will be incomplete unless all CPU subclasses are
> > converted to use only class-properties, or the new command uses
> > object_new().
>
> This is a use case that I initially tried to implement.

This use case can be implemented easily using object_new(), like
qmp_device_list_properties() already does.

--
Eduardo