From: Eduardo Habkost <ehabkost@redhat.com>
To: Valentin Rakush <valentin.rakush@gmail.com>
Cc: "Markus Armbruster" <armbru@redhat.com>,
qemu-devel@nongnu.org, lcapitulino@redhat.com,
asmetanin@virtuozzo.com, "Denis V. Lunev" <den@openvz.org>,
"Andreas Färber" <afaerber@suse.de>
Subject: Re: [Qemu-devel] [PATCH v5] qom, qmp, hmp, qapi: create qom-type-prop-list for class properties
Date: Tue, 2 Feb 2016 13:55:56 -0200 [thread overview]
Message-ID: <20160202155556.GD3869@thinpad.lan.raisama.net> (raw)
In-Reply-To: <CAL0ArcwwzabnEc5mLAetYsoyKfpvdvWjDYd1AsSFf-f1WHzbAQ@mail.gmail.com>
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
next prev parent reply other threads:[~2016-02-02 15:56 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-25 8:24 [Qemu-devel] [PATCH v5] qom, qmp, hmp, qapi: create qom-type-prop-list for class properties Valentin Rakush
2016-01-26 15:35 ` Eduardo Habkost
2016-01-26 15:42 ` Eric Blake
2016-01-26 15:51 ` Daniel P. Berrange
2016-01-26 17:26 ` Eduardo Habkost
2016-01-26 22:19 ` Daniel P. Berrange
2016-01-27 9:53 ` Valentin Rakush
2016-01-27 15:09 ` Eduardo Habkost
2016-01-27 15:23 ` Daniel P. Berrange
2016-01-29 10:03 ` Valentin Rakush
2016-01-29 15:28 ` Eduardo Habkost
2016-01-31 13:40 ` Valentin Rakush
2016-02-02 15:55 ` Eduardo Habkost [this message]
2016-02-07 14:52 ` Valentin Rakush
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160202155556.GD3869@thinpad.lan.raisama.net \
--to=ehabkost@redhat.com \
--cc=afaerber@suse.de \
--cc=armbru@redhat.com \
--cc=asmetanin@virtuozzo.com \
--cc=den@openvz.org \
--cc=lcapitulino@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=valentin.rakush@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).