From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59299) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VEDiy-0000No-G5 for qemu-devel@nongnu.org; Tue, 27 Aug 2013 03:30:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VEDir-0005Hf-5g for qemu-devel@nongnu.org; Tue, 27 Aug 2013 03:30:00 -0400 Received: from mail-wg0-f47.google.com ([74.125.82.47]:40639) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VEDiq-0005HR-Vl for qemu-devel@nongnu.org; Tue, 27 Aug 2013 03:29:53 -0400 Received: by mail-wg0-f47.google.com with SMTP id j13so3577759wgh.26 for ; Tue, 27 Aug 2013 00:29:37 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <521C555D.9010101@redhat.com> Date: Tue, 27 Aug 2013 09:29:33 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <20130827070110.GA16338@redhat.com> In-Reply-To: <20130827070110.GA16338@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] pc: reduce duplication, fix PIIX descriptions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: Markus Armbruster , Eduardo Habkost , qemu-devel@nongnu.org, Anthony Liguori , =?ISO-8859-1?Q?Andreas_F=E4rber?= Il 27/08/2013 09:01, Michael S. Tsirkin ha scritto: > We have a lot of code duplication between machine types, > this increases with each new machine type > and each new field. > > This has already introduced a minor bug: description > for pc-1.3 says "Standard PC" while description for > pc-1.4 is "Standard PC (i440FX + PIIX, 1996)" > which makes you think 1.3 is somehow more standard, > or newer, while in fact it's a revision of the same PC. > > This patch addresses this issue by using macros, along > the lines used by PC_COMPAT_X_X - only for > non-property options. > > Note: this conflicts (trivially) with patch > [PATCH v3 6/6] hw: Clean up bogus default boot order > by Markus. Sent separately for ease of review. If people like this > approach, I can rebase on top of that patch. > > The approach can extend to non-PC machine types. > > Cc: Markus Armbruster > Signed-off-by: Michael S. Tsirkin > --- > static QEMUMachine isapc_machine = { > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c > index 198c785..084ecac 100644 > --- a/hw/i386/pc_q35.c > +++ b/hw/i386/pc_q35.c > @@ -253,39 +253,41 @@ static void pc_q35_init_1_4(QEMUMachineInitArgs *args) > pc_q35_init(args); > } > > +#define PC_Q35_MACHINE_OPTIONS \ > + PC_DEFAULT_MACHINE_OPTIONS, \ > + .hot_add_cpu = pc_hot_add_cpu .desc is missing, right? Otherwise looks good. Paolo > + > +#define PC_Q35_1_6_MACHINE_OPTIONS PC_Q35_MACHINE_OPTIONS > static QEMUMachine pc_q35_machine_v1_6 = { > + PC_Q35_1_6_MACHINE_OPTIONS, > .name = "pc-q35-1.6", > .alias = "q35", > - .desc = "Standard PC (Q35 + ICH9, 2009)", > .init = pc_q35_init_1_6, > - .hot_add_cpu = pc_hot_add_cpu, > - .max_cpus = 255, > - DEFAULT_MACHINE_OPTIONS, > }; > > static QEMUMachine pc_q35_machine_v1_5 = { > + PC_Q35_1_6_MACHINE_OPTIONS, > .name = "pc-q35-1.5", > - .desc = "Standard PC (Q35 + ICH9, 2009)", > .init = pc_q35_init_1_5, > - .hot_add_cpu = pc_hot_add_cpu, > - .max_cpus = 255, > .compat_props = (GlobalProperty[]) { > PC_COMPAT_1_5, > { /* end of list */ } > }, > - DEFAULT_MACHINE_OPTIONS, > }; > > +#define PC_Q35_1_4_MACHINE_OPTIONS \ > + PC_Q35_1_6_MACHINE_OPTIONS, \ > + .hot_add_cpu = NULL > + > static QEMUMachine pc_q35_machine_v1_4 = { > + PC_Q35_1_4_MACHINE_OPTIONS, > .name = "pc-q35-1.4", > - .desc = "Standard PC (Q35 + ICH9, 2009)", > .init = pc_q35_init_1_4, > - .max_cpus = 255, > .compat_props = (GlobalProperty[]) { > PC_COMPAT_1_4, > { /* end of list */ } > }, > - DEFAULT_MACHINE_OPTIONS, > }; > > static void pc_q35_machine_init(void) > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index 475ba9e..4a38fad 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -325,4 +325,9 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t); > .value = stringify(0),\ > } > > +#define PC_DEFAULT_MACHINE_OPTIONS \ > + DEFAULT_MACHINE_OPTIONS, \ > + .hot_add_cpu = pc_hot_add_cpu, \ > + .max_cpus = 255 > + > #endif >