All of lore.kernel.org
 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 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.