All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.