* [Qemu-devel] [PATCHv3] pc: reduce duplication, fix PIIX descriptions
@ 2013-09-01 7:38 Michael S. Tsirkin
0 siblings, 0 replies; only message in thread
From: Michael S. Tsirkin @ 2013-09-01 7:38 UTC (permalink / raw)
To: Anthony Liguori, qemu-devel
Cc: Paolo Bonzini, Andreas Färber, Eduardo Habkost
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
^ permalink raw reply related [flat|nested] only message in thread
only message in thread, other threads:[~2013-09-01 7:36 UTC | newest]
Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-01 7:38 [Qemu-devel] [PATCHv3] pc: reduce duplication, fix PIIX descriptions Michael S. Tsirkin
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.