From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35470) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aSQh5-0000iM-EF for qemu-devel@nongnu.org; Sun, 07 Feb 2016 09:52:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aSQh3-0003es-OE for qemu-devel@nongnu.org; Sun, 07 Feb 2016 09:52:07 -0500 Received: from mail-wm0-x241.google.com ([2a00:1450:400c:c09::241]:35426) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aSQh3-0003eo-DB for qemu-devel@nongnu.org; Sun, 07 Feb 2016 09:52:05 -0500 Received: by mail-wm0-x241.google.com with SMTP id g62so11492373wme.2 for ; Sun, 07 Feb 2016 06:52:04 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <20160202155556.GD3869@thinpad.lan.raisama.net> References: <1453710287-12706-1-git-send-email-valentin.rakush@gmail.com> <20160126153538.GN3869@thinpad.lan.raisama.net> <20160126155121.GI15172@redhat.com> <20160126172635.GO3869@thinpad.lan.raisama.net> <20160126221913.GA13460@redhat.com> <20160127150937.GQ3869@thinpad.lan.raisama.net> <20160127152359.GC14935@redhat.com> <20160129152801.GZ3869@thinpad.lan.raisama.net> <20160202155556.GD3869@thinpad.lan.raisama.net> Date: Sun, 7 Feb 2016 17:52:04 +0300 Message-ID: From: Valentin Rakush Content-Type: multipart/alternative; boundary=001a114545d640d9e4052b2f3ac9 Subject: Re: [Qemu-devel] [PATCH v5] qom, qmp, hmp, qapi: create qom-type-prop-list for class properties List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: Markus Armbruster , qemu-devel@nongnu.org, lcapitulino@redhat.com, asmetanin@virtuozzo.com, "Denis V. Lunev" , =?UTF-8?Q?Andreas_F=C3=A4rber?= --001a114545d640d9e4052b2f3ac9 Content-Type: text/plain; charset=UTF-8 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 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 > --001a114545d640d9e4052b2f3ac9 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Hi Eduardo,

thank you for your explanat= ions.

> You don't have to, if you just= do object_new() like
> qmp_device_list_properties() does. Bot= h ObjectClass::properties
> and DeviceClas::props are translat= ed to object instance
> properties (Object::properties).
=

I should foresee these. Qemu has the object orien= ted approach implemented in C.=C2=A0
I missed this point. Thank y= ou for explanation.=C2=A0

I did the following chan= ges in the target-i386/cpu.c

=C2=A0 =C2=A0 = =C2=A0dc->props =3D host_x86_cpu_properties;
=C2=A0 =C2=A0 =C2= =A0/* Reason: host_x86_cpu_initfn() dies when !kvm_enabled() */
-= =C2=A0 =C2=A0dc->cannot_destroy_with_object_finalize_yet =3D true;
+ =C2=A0 =C2=A0dc->cannot_destroy_with_object_finalize_yet =3D fal= se;

and=C2=A0

<= div>=C2=A0 =C2=A0 =C2=A0/*
=C2=A0 =C2=A0 =C2=A0 * Reason: x86_cpu= _initfn() calls cpu_exec_init(), which saves the
=C2=A0 =C2=A0 = =C2=A0 * object in cpus -> dangling pointer after final object_unref().<= /div>
=C2=A0 =C2=A0 =C2=A0 */
- =C2=A0 =C2=A0dc->cannot_de= stroy_with_object_finalize_yet =3D true;
+ =C2=A0 =C2=A0dc->ca= nnot_destroy_with_object_finalize_yet =3D false;

=
and now I can add class properties to the target-i386/cpu.c from this = patch=C2=A0
and then command "qemu-sys= tem-x86_64 -device core2duo-x86_64-cpu,help"
will show me th= e class properties. =C2=A0

However according to th= is patch=C2=A0
I am checking how to make cp= u class not to fail in device-list-properties call
throught QMP i= nterface.

Basically my goal is to put class proper= ties into the target-i386/cpu.c and=C2=A0
print them when necessa= ry. And after our discussion qmp_device_list_properties=C2=A0
is = a already available for this but cannot_destroy_with_object_finalize_yet=C2= =A0
flag should be set to false.

Please = let me know if this is wrong approach.=C2=A0

T= hank you,
Valentin

On Tue, Feb 2, 2016 at 6:55 PM, Eduardo Habkost <ehabk= ost@redhat.com> wrote:
On Sun, Jan 31, 2016 at 04:40:54PM +0300, Valentin Rakush wrot= e:
> 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 ObjectCla= ss
> struct in the include/qom/object.h
> The typedef struct DeviceClass from include/hw/qdev-core.h is inherite= d
> 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 =3D 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 t= hat
> 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=3Dvalue 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 cod= e yet.

In this case it's different: ObjectClass::compat_props are not machine properties. They are just property=3Dvalue 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 classe= s
> > differently"?
>
> In the list_device_properties function we should have several conditio= nal
> statements like
>
> if (machine =3D object_class_dynamic_cast(class, TYPE_MACHINE)) {
> /* process machine properties using MachineClass GlobalProperty
> *compat_props; */
> }
> else if (machine =3D object_class_dynamic_cast(class, TYPE_DEVICE)) {<= br> > /* process device class properties, using DeviceClass Property *props;= */
> }
> else if (machine =3D 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<= br> > > 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

--001a114545d640d9e4052b2f3ac9--