From: "Michael S. Tsirkin" <mst@redhat.com>
To: Anthony Liguori <anthony@codemonkey.ws>, qemu-devel@nongnu.org
Cc: "Paolo Bonzini" <pbonzini@redhat.com>,
"Andreas Färber" <afaerber@suse.de>,
"Eduardo Habkost" <ehabkost@redhat.com>
Subject: [Qemu-devel] [PATCHv3] pc: reduce duplication, fix PIIX descriptions
Date: Sun, 1 Sep 2013 10:38:19 +0300 [thread overview]
Message-ID: <20130901073819.GA31525@redhat.com> (raw)
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.
The approach can in the future extend to non-PC machine types.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
Eduardo suggested adding a comment for the desc field
documenting that it is not exposed to guest.
However, Markus sent a patch that actually will expose
desc to guests starting from 1.7, so this doesn't look
like a fundamental restriction.
Further, we have many fields not exposed to guests -
it seems arbitrary to document that for this specific
one only. It would make more sense to audit code and document fields
exposed to guests.
For this reason I didn't include Eduardo's suggestion.
Please review, if there are no objections, I'll apply this on top of
Markus's patch on my tree.
Changes from v2:
rebase on top of Markus' patch
Changes from v1:
add missing desc for Q35 to address Paolo's comment
---
hw/i386/pc_piix.c | 86 +++++++++++++++++++++-------------------------------
hw/i386/pc_q35.c | 25 ++++++++-------
include/hw/i386/pc.h | 8 +++++
3 files changed, 56 insertions(+), 63 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index aa0a39a..275e395 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -334,36 +334,39 @@ static void pc_xen_hvm_init(QEMUMachineInitArgs *args)
}
#endif
+#define PC_I440FX_MACHINE_OPTIONS \
+ PC_DEFAULT_MACHINE_OPTIONS, \
+ .desc = "Standard PC (i440FX + PIIX, 1996)", \
+ .hot_add_cpu = pc_hot_add_cpu
+
+#define PC_I440FX_1_6_MACHINE_OPTIONS PC_I440FX_MACHINE_OPTIONS
+
static QEMUMachine pc_i440fx_machine_v1_6 = {
+ PC_I440FX_1_6_MACHINE_OPTIONS,
.name = "pc-i440fx-1.6",
.alias = "pc",
- .desc = "Standard PC (i440FX + PIIX, 1996)",
.init = pc_init_pci_1_6,
- .hot_add_cpu = pc_hot_add_cpu,
- .max_cpus = 255,
.is_default = 1,
- .default_boot_order = "cad",
};
static QEMUMachine pc_i440fx_machine_v1_5 = {
+ PC_I440FX_1_6_MACHINE_OPTIONS,
.name = "pc-i440fx-1.5",
- .desc = "Standard PC (i440FX + PIIX, 1996)",
.init = pc_init_pci_1_5,
- .hot_add_cpu = pc_hot_add_cpu,
- .max_cpus = 255,
.compat_props = (GlobalProperty[]) {
PC_COMPAT_1_5,
{ /* end of list */ }
},
- .default_boot_order = "cad",
};
+#define PC_I440FX_1_4_MACHINE_OPTIONS \
+ PC_I440FX_1_6_MACHINE_OPTIONS, \
+ .hot_add_cpu = NULL
+
static QEMUMachine pc_i440fx_machine_v1_4 = {
+ PC_I440FX_1_4_MACHINE_OPTIONS,
.name = "pc-i440fx-1.4",
- .desc = "Standard PC (i440FX + PIIX, 1996)",
.init = pc_init_pci_1_4,
- .max_cpus = 255,
- .default_boot_order = "cad",
.compat_props = (GlobalProperty[]) {
PC_COMPAT_1_4,
{ /* end of list */ }
@@ -391,11 +394,9 @@ static QEMUMachine pc_i440fx_machine_v1_4 = {
}
static QEMUMachine pc_machine_v1_3 = {
+ PC_I440FX_1_4_MACHINE_OPTIONS,
.name = "pc-1.3",
- .desc = "Standard PC",
.init = pc_init_pci_1_3,
- .max_cpus = 255,
- .default_boot_order = "cad",
.compat_props = (GlobalProperty[]) {
PC_COMPAT_1_3,
{ /* end of list */ }
@@ -430,12 +431,13 @@ static QEMUMachine pc_machine_v1_3 = {
.value = "off",\
}
+#define PC_I440FX_1_2_MACHINE_OPTIONS \
+ PC_I440FX_1_4_MACHINE_OPTIONS, \
+ .init = pc_init_pci_1_2
+
static QEMUMachine pc_machine_v1_2 = {
+ PC_I440FX_1_2_MACHINE_OPTIONS,
.name = "pc-1.2",
- .desc = "Standard PC",
- .init = pc_init_pci_1_2,
- .max_cpus = 255,
- .default_boot_order = "cad",
.compat_props = (GlobalProperty[]) {
PC_COMPAT_1_2,
{ /* end of list */ }
@@ -475,11 +477,8 @@ static QEMUMachine pc_machine_v1_2 = {
}
static QEMUMachine pc_machine_v1_1 = {
+ PC_I440FX_1_2_MACHINE_OPTIONS,
.name = "pc-1.1",
- .desc = "Standard PC",
- .init = pc_init_pci_1_2,
- .max_cpus = 255,
- .default_boot_order = "cad",
.compat_props = (GlobalProperty[]) {
PC_COMPAT_1_1,
{ /* end of list */ }
@@ -507,11 +506,8 @@ static QEMUMachine pc_machine_v1_1 = {
}
static QEMUMachine pc_machine_v1_0 = {
+ PC_I440FX_1_2_MACHINE_OPTIONS,
.name = "pc-1.0",
- .desc = "Standard PC",
- .init = pc_init_pci_1_2,
- .max_cpus = 255,
- .default_boot_order = "cad",
.compat_props = (GlobalProperty[]) {
PC_COMPAT_1_0,
{ /* end of list */ }
@@ -523,11 +519,8 @@ static QEMUMachine pc_machine_v1_0 = {
PC_COMPAT_1_0
static QEMUMachine pc_machine_v0_15 = {
+ PC_I440FX_1_2_MACHINE_OPTIONS,
.name = "pc-0.15",
- .desc = "Standard PC",
- .init = pc_init_pci_1_2,
- .max_cpus = 255,
- .default_boot_order = "cad",
.compat_props = (GlobalProperty[]) {
PC_COMPAT_0_15,
{ /* end of list */ }
@@ -556,11 +549,8 @@ static QEMUMachine pc_machine_v0_15 = {
}
static QEMUMachine pc_machine_v0_14 = {
+ PC_I440FX_1_2_MACHINE_OPTIONS,
.name = "pc-0.14",
- .desc = "Standard PC",
- .init = pc_init_pci_1_2,
- .max_cpus = 255,
- .default_boot_order = "cad",
.compat_props = (GlobalProperty[]) {
PC_COMPAT_0_14,
{
@@ -589,12 +579,13 @@ static QEMUMachine pc_machine_v0_14 = {
.value = stringify(1),\
}
+#define PC_I440FX_0_13_MACHINE_OPTIONS \
+ PC_I440FX_1_2_MACHINE_OPTIONS, \
+ .init = pc_init_pci_no_kvmclock
+
static QEMUMachine pc_machine_v0_13 = {
+ PC_I440FX_0_13_MACHINE_OPTIONS,
.name = "pc-0.13",
- .desc = "Standard PC",
- .init = pc_init_pci_no_kvmclock,
- .max_cpus = 255,
- .default_boot_order = "cad",
.compat_props = (GlobalProperty[]) {
PC_COMPAT_0_13,
{
@@ -640,11 +631,8 @@ static QEMUMachine pc_machine_v0_13 = {
}
static QEMUMachine pc_machine_v0_12 = {
+ PC_I440FX_0_13_MACHINE_OPTIONS,
.name = "pc-0.12",
- .desc = "Standard PC",
- .init = pc_init_pci_no_kvmclock,
- .max_cpus = 255,
- .default_boot_order = "cad",
.compat_props = (GlobalProperty[]) {
PC_COMPAT_0_12,
{
@@ -674,11 +662,8 @@ static QEMUMachine pc_machine_v0_12 = {
}
static QEMUMachine pc_machine_v0_11 = {
+ PC_I440FX_0_13_MACHINE_OPTIONS,
.name = "pc-0.11",
- .desc = "Standard PC, qemu 0.11",
- .init = pc_init_pci_no_kvmclock,
- .max_cpus = 255,
- .default_boot_order = "cad",
.compat_props = (GlobalProperty[]) {
PC_COMPAT_0_11,
{
@@ -696,11 +681,8 @@ static QEMUMachine pc_machine_v0_11 = {
};
static QEMUMachine pc_machine_v0_10 = {
+ PC_I440FX_0_13_MACHINE_OPTIONS,
.name = "pc-0.10",
- .desc = "Standard PC, qemu 0.10",
- .init = pc_init_pci_no_kvmclock,
- .max_cpus = 255,
- .default_boot_order = "cad",
.compat_props = (GlobalProperty[]) {
PC_COMPAT_0_11,
{
@@ -730,11 +712,11 @@ static QEMUMachine pc_machine_v0_10 = {
};
static QEMUMachine isapc_machine = {
+ PC_COMMON_MACHINE_OPTIONS,
.name = "isapc",
.desc = "ISA-only PC",
.init = pc_init_isa,
.max_cpus = 1,
- .default_boot_order = "cad",
.compat_props = (GlobalProperty[]) {
{ /* end of list */ }
},
@@ -742,12 +724,12 @@ static QEMUMachine isapc_machine = {
#ifdef CONFIG_XEN
static QEMUMachine xenfv_machine = {
+ PC_COMMON_MACHINE_OPTIONS,
.name = "xenfv",
.desc = "Xen Fully-virtualized PC",
.init = pc_xen_hvm_init,
.max_cpus = HVM_MAX_VCPUS,
.default_machine_opts = "accel=xen",
- .default_boot_order = "cad",
};
#endif
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 291ad97..d7b7c3b 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -253,35 +253,38 @@ static void pc_q35_init_1_4(QEMUMachineInitArgs *args)
pc_q35_init(args);
}
+#define PC_Q35_MACHINE_OPTIONS \
+ PC_DEFAULT_MACHINE_OPTIONS, \
+ .desc = "Standard PC (Q35 + ICH9, 2009)", \
+ .hot_add_cpu = pc_hot_add_cpu
+
+#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_boot_order = "cad",
};
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,
- .default_boot_order = "cad",
.compat_props = (GlobalProperty[]) {
PC_COMPAT_1_5,
{ /* end of list */ }
},
};
+#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,
- .default_boot_order = "cad",
.compat_props = (GlobalProperty[]) {
PC_COMPAT_1_4,
{ /* end of list */ }
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 475ba9e..7fb04d8 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -325,4 +325,12 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
.value = stringify(0),\
}
+#define PC_COMMON_MACHINE_OPTIONS \
+ .default_boot_order = "cad"
+
+#define PC_DEFAULT_MACHINE_OPTIONS \
+ PC_COMMON_MACHINE_OPTIONS, \
+ .hot_add_cpu = pc_hot_add_cpu, \
+ .max_cpus = 255
+
#endif
--
MST
reply other threads:[~2013-09-01 7:36 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
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=20130901073819.GA31525@redhat.com \
--to=mst@redhat.com \
--cc=afaerber@suse.de \
--cc=anthony@codemonkey.ws \
--cc=ehabkost@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
/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).