qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Marcel Apfelbaum <marcel.a@redhat.com>
Cc: peter.maydell@linaro.org, peter.crosthwaite@petalogix.com,
	ehabkost@redhat.com, mst@redhat.com, qemu-devel@nongnu.org,
	mtosatti@redhat.com, mdroth@linux.vnet.ibm.com,
	armbru@redhat.com, blauwirbel@gmail.com, quintela@redhat.com,
	agraf@suse.de, aliguori@amazon.com, edgar.iglesias@gmail.com,
	scottwood@freescale.com, imammedo@redhat.com,
	lcapitulino@redhat.com, afaerber@suse.de, rth@twiddle.net
Subject: Re: [Qemu-devel] [PATCH RFC V2 0/9] qemu-machine as a QOM object
Date: Mon, 03 Mar 2014 13:56:18 +0100	[thread overview]
Message-ID: <53147BF2.4020108@redhat.com> (raw)
In-Reply-To: <1393848446.11589.90.camel@localhost.localdomain>

Il 03/03/2014 13:07, Marcel Apfelbaum ha scritto:
>> Another early refactoring should be to pass &current_machine->init_args
>> to machine->init, not the "args".
> Problem is that this is a "private field", should I add a getter for it?

vl.c is already accessing it one line before, isn't it?

So vl.c is already looking at QemuMachineState internals.  I expected 
this to be temporary.

>> Now here's a possible plan to get rid of QEMUMachineInitArgs:
>>
>> 1) Add three wrappers to QemuMachine for reading kernel_filename,
>> kernel_cmdline, initrd_filename.  Unlike object_property_get_str, they
>> can skip the strdup of the value.  This way you don't have to add the
>> matching free to all uses of the fields.
>
> I have nothing against it, but this will break QEMU's unified way to
> handle object properties, right?
> Meaning I will have a field  of the object state(kernel_filename)
> and I will add a method like machine_get_field(QemuMachineState) going
> "around" QOM,,,

True.  On the other hand, I believe there is a purpose in using getters 
and setters: using object_property_get/set all the time is more verbose 
and especially less type-safe.  I prefer adding getters to naming 
properties with #define.  Especially if you need getters but not setters.

GObject uses a similar scheme where you have both regular 
getters/setters and properties.  Static languages use the getters and 
setters, while bindings for dynamic languages will always go through 
properties.

>> 2) Similarly, add get/set functions (not properties, since these are not
>> accessible via -machine) for ram_size, boot_order, cpu_model.
>
> I am sorry, here you lost me, what do you mean "accessible via -machine"?
> Maybe that cannot be queried by QOM tree?

Not yet, at least.  I prefer to keep these fields out of QOM because 
they cannot be set with -machine.  cpu_model is already accessible since 
it's related to the class of the CPU devices.

> Those getter/setters are not the same wrappers as above, going around "QOM"?

Actually they are the same thing.  But I think above you only need 
getters.  Here you also need setters.

>> 3) Now you can have something like patch 8 in this series.
> I also need patch 5 that deals with getting a string property with NULL value,

Ok.

> I see no reason why not, the main problem I see is the use of those wrappers
> or setters/getters, I suspect that the usage will be:
> 1. global QOM query to get the machine
> 2. Use this wrappers(getters/setters) to do query/alter the machine.

I think setters should appear only in vl.c.

> Doesn't QOM have another way to do this? Or I am missing something, of course.

I don't know really.  My main requirement is not to change the -machine 
interface.

Paolo

  reply	other threads:[~2014-03-03 12:56 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-02 13:07 [Qemu-devel] [PATCH RFC V2 0/9] qemu-machine as a QOM object Marcel Apfelbaum
2014-03-02 13:07 ` [Qemu-devel] [PATCH RFC V2 1/9] hw/core: introduced qemu machine as " Marcel Apfelbaum
2014-03-03 12:56   ` Michael S. Tsirkin
2014-03-03 17:49   ` Andreas Färber
2014-03-03 19:06     ` Marcel Apfelbaum
2014-03-02 13:07 ` [Qemu-devel] [PATCH RFC V2 2/9] vl: use qemu machine QOM class instead of global machines list Marcel Apfelbaum
2014-03-03 12:58   ` Michael S. Tsirkin
2014-03-03 12:57     ` Paolo Bonzini
2014-03-03 13:03       ` Marcel Apfelbaum
2014-03-03 14:52         ` Andreas Färber
2014-03-03 15:05           ` Marcel Apfelbaum
2014-03-03 18:12   ` Andreas Färber
2014-03-03 19:54     ` Marcel Apfelbaum
2014-03-02 13:07 ` [Qemu-devel] [PATCH RFC V2 3/9] hw/boards: converted current_machine to be an instance of QemuMachineCLass Marcel Apfelbaum
2014-03-03 10:49   ` Paolo Bonzini
2014-03-03 12:07     ` Marcel Apfelbaum
2014-03-03 12:46       ` Paolo Bonzini
2014-03-03 12:11     ` Marcel Apfelbaum
2014-03-02 13:07 ` [Qemu-devel] [PATCH RFC V2 4/9] hw/machine: add qemu machine opts as properties to QemuMachineState Marcel Apfelbaum
2014-03-02 13:07 ` [Qemu-devel] [PATCH RFC V2 5/9] qapi: output visitor crashes qemu if it encounters a NULL value Marcel Apfelbaum
2014-03-02 13:07 ` [Qemu-devel] [PATCH RFC V2 6/9] vl.c: do not set 'type' property in obj_set_property Marcel Apfelbaum
2014-03-03 10:11   ` Paolo Bonzini
2014-03-03 12:09     ` Marcel Apfelbaum
2014-03-03 12:47       ` Paolo Bonzini
2014-03-02 13:07 ` [Qemu-devel] [PATCH RFC V2 7/9] qom: add object_property_is_set Marcel Apfelbaum
2014-03-03 10:13   ` Paolo Bonzini
2014-03-03 12:09     ` Marcel Apfelbaum
2014-03-02 13:07 ` [Qemu-devel] [PATCH RFC V2 8/9] machine-opts: replace qemu_opt_get by QOM QemuMachine queries Marcel Apfelbaum
2014-03-03 10:11   ` Paolo Bonzini
2014-03-03 12:10     ` Marcel Apfelbaum
2014-03-02 13:07 ` [Qemu-devel] [PATCH RFC V2 9/9] hw/core: mapped QemuOpts into QEMUMachineInitArgs fields to remove duplication Marcel Apfelbaum
2014-03-03 10:13   ` Paolo Bonzini
2014-03-03 12:10     ` Marcel Apfelbaum
2014-03-03 10:50 ` [Qemu-devel] [PATCH RFC V2 0/9] qemu-machine as a QOM object Paolo Bonzini
2014-03-03 12:07   ` Marcel Apfelbaum
2014-03-03 12:56     ` Paolo Bonzini [this message]
2014-03-03 13:17       ` Marcel Apfelbaum
2014-03-03 14:10         ` Andreas Färber

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=53147BF2.4020108@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=afaerber@suse.de \
    --cc=agraf@suse.de \
    --cc=aliguori@amazon.com \
    --cc=armbru@redhat.com \
    --cc=blauwirbel@gmail.com \
    --cc=edgar.iglesias@gmail.com \
    --cc=ehabkost@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=lcapitulino@redhat.com \
    --cc=marcel.a@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=mst@redhat.com \
    --cc=mtosatti@redhat.com \
    --cc=peter.crosthwaite@petalogix.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    --cc=rth@twiddle.net \
    --cc=scottwood@freescale.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).