From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37904) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WKSQ7-0003bo-4S for qemu-devel@nongnu.org; Mon, 03 Mar 2014 07:56:36 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WKSPx-0003Sr-Em for qemu-devel@nongnu.org; Mon, 03 Mar 2014 07:56:35 -0500 Received: from mail-qc0-x233.google.com ([2607:f8b0:400d:c01::233]:52559) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WKSPx-0003Sn-Aa for qemu-devel@nongnu.org; Mon, 03 Mar 2014 07:56:25 -0500 Received: by mail-qc0-f179.google.com with SMTP id m20so3414335qcx.10 for ; Mon, 03 Mar 2014 04:56:24 -0800 (PST) Sender: Paolo Bonzini Message-ID: <53147BF2.4020108@redhat.com> Date: Mon, 03 Mar 2014 13:56:18 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1393765632-2753-1-git-send-email-marcel.a@redhat.com> <53145E5D.30704@redhat.com> <1393848446.11589.90.camel@localhost.localdomain> In-Reply-To: <1393848446.11589.90.camel@localhost.localdomain> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH RFC V2 0/9] qemu-machine as a QOM object List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Marcel Apfelbaum 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 Il 03/03/2014 13:07, Marcel Apfelbaum ha scritto: >> Another early refactoring should be to pass ¤t_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