From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39539) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y0XL4-0006rx-VM for qemu-devel@nongnu.org; Mon, 15 Dec 2014 10:13:42 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y0XKx-0003hw-FC for qemu-devel@nongnu.org; Mon, 15 Dec 2014 10:13:34 -0500 Received: from cantor2.suse.de ([195.135.220.15]:55436 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y0XKx-0003hJ-5r for qemu-devel@nongnu.org; Mon, 15 Dec 2014 10:13:27 -0500 Message-ID: <548EFA95.3080407@suse.de> Date: Mon, 15 Dec 2014 16:13:25 +0100 From: Alexander Graf MIME-Version: 1.0 References: <1418217570-15517-1-git-send-email-marcel.a@redhat.com> <1418217570-15517-2-git-send-email-marcel.a@redhat.com> <1418274309.15503.47.camel@localhost.localdomain> In-Reply-To: <1418274309.15503.47.camel@localhost.localdomain> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 1/3] machine: remove qemu_machine_opts global list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Marcel Apfelbaum , Greg Bellows Cc: Peter Maydell , QEMU Developers , mst@redhat.com On 11.12.14 06:05, Marcel Apfelbaum wrote: > On Wed, 2014-12-10 at 16:59 -0600, Greg Bellows wrote: >> >> >> On 10 December 2014 at 07:19, Marcel Apfelbaum >> wrote: >> QEMU has support for options per machine, keeping >> a global list of options is no longer necessary. >> =20 >> Signed-off-by: Marcel Apfelbaum >> --- >> hw/core/machine.c | 45 +++++++++++++++++++++++++++++ >> hw/i386/pc.c | 7 +++++ >> hw/ppc/spapr.c | 3 ++ >> vl.c | 84 >> ++++--------------------------------------------------- >> 4 files changed, 61 insertions(+), 78 deletions(-) >> =20 >> diff --git a/hw/core/machine.c b/hw/core/machine.c >> index 19d3e3a..a0ae5f9 100644 >> --- a/hw/core/machine.c >> +++ b/hw/core/machine.c >> @@ -291,48 +291,93 @@ static void machine_initfn(Object *obj) >> =20 >> object_property_add_str(obj, "accel", >> machine_get_accel, >> machine_set_accel, NULL); >> + object_property_set_description(obj, "accel", >> + "Accelerator list", >> + NULL); >> > Hi Greg, > Thank you for your review! >=20 >> >> If this is a common combination of calls would it make sense to create >> a wrapper routine that just does both of these steps together for >> readability and convenience? Maybe something like: >> >> >> object_property_add_str_with_desc(obj, "accel", "Accelerator >> list" >> machine_get_accel, >> machine_set_accel, >> NULL); > I really thought about it, but one wrapper would not be enough, > we need also one for object_property_add_bool, object_property_add and = so on. > The code needed would be twice as this one and for the moment this > series is the only reason for that, maybe is not enough. >=20 > If you or anyone else things we should still go for it, I'll be glad to= . Maybe something like this? #define MACHINE_OPT(name, namestr, desc, type, ...) \ glue(object_property_add_, type)(obj, namestr, \ glue(machine_get, name), glue(machine_set, name), __VA_ARGS__); \ \ object_property_set_description(obj, namestr, desc, NULL); >=20 >> =20 >> object_property_add_bool(obj, "kernel-irqchip", >> machine_get_kernel_irqchip, >> machine_set_kernel_irqchip, >> NULL); >> + object_property_set_description(obj, "kernel-irqchip", >> + "Use KVM in-kernel >> irqchip", >> + NULL); MACHINE_OPT(kvm_shadow_mem, "kvm-shadow-mem", "Use KVM in-kernel irqchip", _bool, NULL); Not sure it's a great improvement though. I certainly wouldn't mind to leave it as this patch does it. Alex