qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Valentin Rakush <valentin.rakush@gmail.com>
To: Eduardo Habkost <ehabkost@redhat.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: Sun, 7 Feb 2016 17:52:04 +0300	[thread overview]
Message-ID: <CAL0ArczHaN8Ojv9NeWXf=o15zV3aAHKtqDY0iY8GDsoD-H4NcQ@mail.gmail.com> (raw)
In-Reply-To: <20160202155556.GD3869@thinpad.lan.raisama.net>

[-- Attachment #1: Type: text/plain, Size: 5254 bytes --]

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 <ehabkost@redhat.com> 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
>

[-- Attachment #2: Type: text/html, Size: 7084 bytes --]

      reply	other threads:[~2016-02-07 14:52 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
2016-02-07 14:52                     ` Valentin Rakush [this message]

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='CAL0ArczHaN8Ojv9NeWXf=o15zV3aAHKtqDY0iY8GDsoD-H4NcQ@mail.gmail.com' \
    --to=valentin.rakush@gmail.com \
    --cc=afaerber@suse.de \
    --cc=armbru@redhat.com \
    --cc=asmetanin@virtuozzo.com \
    --cc=den@openvz.org \
    --cc=ehabkost@redhat.com \
    --cc=lcapitulino@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /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).