* [Qemu-devel] [PATCH] pc: reduce duplication in compat machine types
@ 2012-03-26 9:40 Michael S. Tsirkin
2012-03-26 12:51 ` Avi Kivity
2012-03-26 13:12 ` Andreas Färber
0 siblings, 2 replies; 5+ messages in thread
From: Michael S. Tsirkin @ 2012-03-26 9:40 UTC (permalink / raw)
To: qemu-devel
Cc: Anthony PERARD, Jan Kiszka, Anthony Liguori, Alexander Graf, Avi Kivity
Make it easier to add compat properties, by
adding macros for properties duplicated across
machine types.
Note: there could be bugs in compat properties,
this patch does not attempt to address them,
the code is bug for bug identical to the original.
Tested by: generated a preprocessed file, sorted and
compared to sorted original.
Lightly tested on x86_64.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
I've put the above on my branch as it's needed for
fixing balloon pci class. Let me know if there are
any objections.
hw/pc_piix.c | 288 +++++++++++++++-------------------------------------------
1 files changed, 73 insertions(+), 215 deletions(-)
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 3f99f9a..e39ef69 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -372,50 +372,69 @@ static QEMUMachine pc_machine_v1_1 = {
.is_default = 1,
};
+#define PC_COMPAT_1_0 \
+ {\
+ .driver = "pc-sysfw",\
+ .property = "rom_only",\
+ .value = stringify(1),\
+ }, {\
+ .driver = "isa-fdc",\
+ .property = "check_media_rate",\
+ .value = "off",\
+ }
+
static QEMUMachine pc_machine_v1_0 = {
.name = "pc-1.0",
.desc = "Standard PC",
.init = pc_init_pci,
.max_cpus = 255,
.compat_props = (GlobalProperty[]) {
- {
- .driver = "pc-sysfw",
- .property = "rom_only",
- .value = stringify(1),
- }, {
- .driver = "isa-fdc",
- .property = "check_media_rate",
- .value = "off",
- },
+ PC_COMPAT_1_0,
{ /* end of list */ }
},
};
+#define PC_COMPAT_0_15 \
+ PC_COMPAT_1_0
+
static QEMUMachine pc_machine_v0_15 = {
.name = "pc-0.15",
.desc = "Standard PC",
.init = pc_init_pci,
.max_cpus = 255,
.compat_props = (GlobalProperty[]) {
- {
- .driver = "pc-sysfw",
- .property = "rom_only",
- .value = stringify(1),
- }, {
- .driver = "isa-fdc",
- .property = "check_media_rate",
- .value = "off",
- },
+ PC_COMPAT_0_15,
{ /* end of list */ }
},
};
+#define PC_COMPAT_0_14 \
+ PC_COMPAT_0_15,\
+ {\
+ .driver = "virtio-blk-pci",\
+ .property = "event_idx",\
+ .value = "off",\
+ },{\
+ .driver = "virtio-serial-pci",\
+ .property = "event_idx",\
+ .value = "off",\
+ },{\
+ .driver = "virtio-net-pci",\
+ .property = "event_idx",\
+ .value = "off",\
+ },{\
+ .driver = "virtio-balloon-pci",\
+ .property = "event_idx",\
+ .value = "off",\
+ }
+
static QEMUMachine pc_machine_v0_14 = {
.name = "pc-0.14",
.desc = "Standard PC",
.init = pc_init_pci,
.max_cpus = 255,
.compat_props = (GlobalProperty[]) {
+ PC_COMPAT_0_14,
{
.driver = "qxl",
.property = "revision",
@@ -424,42 +443,30 @@ static QEMUMachine pc_machine_v0_14 = {
.driver = "qxl-vga",
.property = "revision",
.value = stringify(2),
- },{
- .driver = "virtio-blk-pci",
- .property = "event_idx",
- .value = "off",
- },{
- .driver = "virtio-serial-pci",
- .property = "event_idx",
- .value = "off",
- },{
- .driver = "virtio-net-pci",
- .property = "event_idx",
- .value = "off",
- },{
- .driver = "virtio-balloon-pci",
- .property = "event_idx",
- .value = "off",
- },{
- .driver = "isa-fdc",
- .property = "check_media_rate",
- .value = "off",
- },
- {
- .driver = "pc-sysfw",
- .property = "rom_only",
- .value = stringify(1),
},
{ /* end of list */ }
},
};
+#define PC_COMPAT_0_13 \
+ PC_COMPAT_0_14,\
+ {\
+ .driver = "PCI",\
+ .property = "command_serr_enable",\
+ .value = "off",\
+ },{\
+ .driver = "AC97",\
+ .property = "use_broken_id",\
+ .value = stringify(1),\
+ }
+
static QEMUMachine pc_machine_v0_13 = {
.name = "pc-0.13",
.desc = "Standard PC",
.init = pc_init_pci_no_kvmclock,
.max_cpus = 255,
.compat_props = (GlobalProperty[]) {
+ PC_COMPAT_0_13,
{
.driver = "virtio-9p-pci",
.property = "vectors",
@@ -472,59 +479,31 @@ static QEMUMachine pc_machine_v0_13 = {
.driver = "vmware-svga",
.property = "rombar",
.value = stringify(0),
- },{
- .driver = "PCI",
- .property = "command_serr_enable",
- .value = "off",
- },{
- .driver = "virtio-blk-pci",
- .property = "event_idx",
- .value = "off",
- },{
- .driver = "virtio-serial-pci",
- .property = "event_idx",
- .value = "off",
- },{
- .driver = "virtio-net-pci",
- .property = "event_idx",
- .value = "off",
- },{
- .driver = "virtio-balloon-pci",
- .property = "event_idx",
- .value = "off",
- },{
- .driver = "AC97",
- .property = "use_broken_id",
- .value = stringify(1),
- },{
- .driver = "isa-fdc",
- .property = "check_media_rate",
- .value = "off",
- },
- {
- .driver = "pc-sysfw",
- .property = "rom_only",
- .value = stringify(1),
},
{ /* end of list */ }
},
};
+#define PC_COMPAT_0_12 \
+ PC_COMPAT_0_13,\
+ {\
+ .driver = "virtio-serial-pci",\
+ .property = "max_ports",\
+ .value = stringify(1),\
+ },{\
+ .driver = "virtio-serial-pci",\
+ .property = "vectors",\
+ .value = stringify(0),\
+ }
+
static QEMUMachine pc_machine_v0_12 = {
.name = "pc-0.12",
.desc = "Standard PC",
.init = pc_init_pci_no_kvmclock,
.max_cpus = 255,
.compat_props = (GlobalProperty[]) {
+ PC_COMPAT_0_12,
{
- .driver = "virtio-serial-pci",
- .property = "max_ports",
- .value = stringify(1),
- },{
- .driver = "virtio-serial-pci",
- .property = "vectors",
- .value = stringify(0),
- },{
.driver = "VGA",
.property = "rombar",
.value = stringify(0),
@@ -532,63 +511,27 @@ static QEMUMachine pc_machine_v0_12 = {
.driver = "vmware-svga",
.property = "rombar",
.value = stringify(0),
- },{
- .driver = "PCI",
- .property = "command_serr_enable",
- .value = "off",
- },{
- .driver = "virtio-blk-pci",
- .property = "event_idx",
- .value = "off",
- },{
- .driver = "virtio-serial-pci",
- .property = "event_idx",
- .value = "off",
- },{
- .driver = "virtio-net-pci",
- .property = "event_idx",
- .value = "off",
- },{
- .driver = "virtio-balloon-pci",
- .property = "event_idx",
- .value = "off",
- },{
- .driver = "AC97",
- .property = "use_broken_id",
- .value = stringify(1),
- },{
- .driver = "isa-fdc",
- .property = "check_media_rate",
- .value = "off",
- },
- {
- .driver = "pc-sysfw",
- .property = "rom_only",
- .value = stringify(1),
},
{ /* end of list */ }
}
};
+#define PC_COMPAT_0_11 \
+ PC_COMPAT_0_12,\
+ {\
+ .driver = "virtio-blk-pci",\
+ .property = "vectors",\
+ .value = stringify(0),\
+ }
+
static QEMUMachine pc_machine_v0_11 = {
.name = "pc-0.11",
.desc = "Standard PC, qemu 0.11",
.init = pc_init_pci_no_kvmclock,
.max_cpus = 255,
.compat_props = (GlobalProperty[]) {
+ PC_COMPAT_0_11,
{
- .driver = "virtio-blk-pci",
- .property = "vectors",
- .value = stringify(0),
- },{
- .driver = "virtio-serial-pci",
- .property = "max_ports",
- .value = stringify(1),
- },{
- .driver = "virtio-serial-pci",
- .property = "vectors",
- .value = stringify(0),
- },{
.driver = "ide-drive",
.property = "ver",
.value = "0.11",
@@ -596,43 +539,6 @@ static QEMUMachine pc_machine_v0_11 = {
.driver = "scsi-disk",
.property = "ver",
.value = "0.11",
- },{
- .driver = "PCI",
- .property = "rombar",
- .value = stringify(0),
- },{
- .driver = "PCI",
- .property = "command_serr_enable",
- .value = "off",
- },{
- .driver = "virtio-blk-pci",
- .property = "event_idx",
- .value = "off",
- },{
- .driver = "virtio-serial-pci",
- .property = "event_idx",
- .value = "off",
- },{
- .driver = "virtio-net-pci",
- .property = "event_idx",
- .value = "off",
- },{
- .driver = "virtio-balloon-pci",
- .property = "event_idx",
- .value = "off",
- },{
- .driver = "AC97",
- .property = "use_broken_id",
- .value = stringify(1),
- },{
- .driver = "isa-fdc",
- .property = "check_media_rate",
- .value = "off",
- },
- {
- .driver = "pc-sysfw",
- .property = "rom_only",
- .value = stringify(1),
},
{ /* end of list */ }
}
@@ -644,6 +550,7 @@ static QEMUMachine pc_machine_v0_10 = {
.init = pc_init_pci_no_kvmclock,
.max_cpus = 255,
.compat_props = (GlobalProperty[]) {
+ PC_COMPAT_0_11,
{
.driver = "virtio-blk-pci",
.property = "class",
@@ -653,22 +560,10 @@ static QEMUMachine pc_machine_v0_10 = {
.property = "class",
.value = stringify(PCI_CLASS_DISPLAY_OTHER),
},{
- .driver = "virtio-serial-pci",
- .property = "max_ports",
- .value = stringify(1),
- },{
- .driver = "virtio-serial-pci",
- .property = "vectors",
- .value = stringify(0),
- },{
.driver = "virtio-net-pci",
.property = "vectors",
.value = stringify(0),
},{
- .driver = "virtio-blk-pci",
- .property = "vectors",
- .value = stringify(0),
- },{
.driver = "ide-drive",
.property = "ver",
.value = "0.10",
@@ -676,43 +571,6 @@ static QEMUMachine pc_machine_v0_10 = {
.driver = "scsi-disk",
.property = "ver",
.value = "0.10",
- },{
- .driver = "PCI",
- .property = "rombar",
- .value = stringify(0),
- },{
- .driver = "PCI",
- .property = "command_serr_enable",
- .value = "off",
- },{
- .driver = "virtio-blk-pci",
- .property = "event_idx",
- .value = "off",
- },{
- .driver = "virtio-serial-pci",
- .property = "event_idx",
- .value = "off",
- },{
- .driver = "virtio-net-pci",
- .property = "event_idx",
- .value = "off",
- },{
- .driver = "virtio-balloon-pci",
- .property = "event_idx",
- .value = "off",
- },{
- .driver = "AC97",
- .property = "use_broken_id",
- .value = stringify(1),
- },{
- .driver = "isa-fdc",
- .property = "check_media_rate",
- .value = "off",
- },
- {
- .driver = "pc-sysfw",
- .property = "rom_only",
- .value = stringify(1),
},
{ /* end of list */ }
},
--
1.7.9.111.gf3fb0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] pc: reduce duplication in compat machine types
2012-03-26 9:40 [Qemu-devel] [PATCH] pc: reduce duplication in compat machine types Michael S. Tsirkin
@ 2012-03-26 12:51 ` Avi Kivity
2012-03-26 13:32 ` Michael S. Tsirkin
2012-03-26 13:12 ` Andreas Färber
1 sibling, 1 reply; 5+ messages in thread
From: Avi Kivity @ 2012-03-26 12:51 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Anthony PERARD, Jan Kiszka, Anthony Liguori, qemu-devel, Alexander Graf
On 03/26/2012 11:40 AM, Michael S. Tsirkin wrote:
> Make it easier to add compat properties, by
> adding macros for properties duplicated across
> machine types.
>
> Note: there could be bugs in compat properties,
> this patch does not attempt to address them,
> the code is bug for bug identical to the original.
>
> Tested by: generated a preprocessed file, sorted and
> compared to sorted original.
> Lightly tested on x86_64.
>
>
> +#define PC_COMPAT_1_0 \
> + {\
> + .driver = "pc-sysfw",\
> + .property = "rom_only",\
> + .value = stringify(1),\
> + }, {\
> + .driver = "isa-fdc",\
> + .property = "check_media_rate",\
> + .value = "off",\
> + }
> +
Hmm. how about
> static QEMUMachine pc_machine_v1_0 = {
> .name = "pc-1.0",
> .desc = "Standard PC",
> .init = pc_init_pci,
> .max_cpus = 255,
> .compat_props = (GlobalProperty[]) {
> - {
> - .driver = "pc-sysfw",
> - .property = "rom_only",
> - .value = stringify(1),
> - }, {
> - .driver = "isa-fdc",
> - .property = "check_media_rate",
> - .value = "off",
> - },
> + PC_COMPAT_1_0,
+ .base_machine = &pc_machine_v1_1;
Then it would be easier to define machines differentially.
> { /* end of list */ }
> },
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] pc: reduce duplication in compat machine types
2012-03-26 9:40 [Qemu-devel] [PATCH] pc: reduce duplication in compat machine types Michael S. Tsirkin
2012-03-26 12:51 ` Avi Kivity
@ 2012-03-26 13:12 ` Andreas Färber
2012-03-26 13:23 ` Michael S. Tsirkin
1 sibling, 1 reply; 5+ messages in thread
From: Andreas Färber @ 2012-03-26 13:12 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Anthony Liguori, Jan Kiszka, qemu-devel, Alexander Graf,
Avi Kivity, Anthony PERARD
Am 26.03.2012 11:40, schrieb Michael S. Tsirkin:
> Make it easier to add compat properties, by
> adding macros for properties duplicated across
> machine types.
>
> Note: there could be bugs in compat properties,
> this patch does not attempt to address them,
> the code is bug for bug identical to the original.
>
> Tested by: generated a preprocessed file, sorted and
> compared to sorted original.
> Lightly tested on x86_64.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>
> I've put the above on my branch as it's needed for
> fixing balloon pci class. Let me know if there are
> any objections.
Better reusing code is certainly a good idea. I wonder though if we
might ever get into a situation where things change back and forth. I'd
hope that properties are processed top to bottom so that, e.g.,
PC_COMPAT_1_0, { ... } can overwrite 0_15 compat properties inherited
through 1_0.
Andreas
>
> hw/pc_piix.c | 288 +++++++++++++++-------------------------------------------
> 1 files changed, 73 insertions(+), 215 deletions(-)
>
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index 3f99f9a..e39ef69 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -372,50 +372,69 @@ static QEMUMachine pc_machine_v1_1 = {
> .is_default = 1,
> };
>
> +#define PC_COMPAT_1_0 \
> + {\
> + .driver = "pc-sysfw",\
> + .property = "rom_only",\
> + .value = stringify(1),\
> + }, {\
> + .driver = "isa-fdc",\
> + .property = "check_media_rate",\
> + .value = "off",\
> + }
> +
> static QEMUMachine pc_machine_v1_0 = {
> .name = "pc-1.0",
> .desc = "Standard PC",
> .init = pc_init_pci,
> .max_cpus = 255,
> .compat_props = (GlobalProperty[]) {
> - {
> - .driver = "pc-sysfw",
> - .property = "rom_only",
> - .value = stringify(1),
> - }, {
> - .driver = "isa-fdc",
> - .property = "check_media_rate",
> - .value = "off",
> - },
> + PC_COMPAT_1_0,
> { /* end of list */ }
> },
> };
[snip]
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] pc: reduce duplication in compat machine types
2012-03-26 13:12 ` Andreas Färber
@ 2012-03-26 13:23 ` Michael S. Tsirkin
0 siblings, 0 replies; 5+ messages in thread
From: Michael S. Tsirkin @ 2012-03-26 13:23 UTC (permalink / raw)
To: Andreas Färber
Cc: Anthony Liguori, Jan Kiszka, qemu-devel, Alexander Graf,
Avi Kivity, Anthony PERARD
On Mon, Mar 26, 2012 at 03:12:37PM +0200, Andreas Färber wrote:
> Am 26.03.2012 11:40, schrieb Michael S. Tsirkin:
> > Make it easier to add compat properties, by
> > adding macros for properties duplicated across
> > machine types.
> >
> > Note: there could be bugs in compat properties,
> > this patch does not attempt to address them,
> > the code is bug for bug identical to the original.
> >
> > Tested by: generated a preprocessed file, sorted and
> > compared to sorted original.
> > Lightly tested on x86_64.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >
> > I've put the above on my branch as it's needed for
> > fixing balloon pci class. Let me know if there are
> > any objections.
>
> Better reusing code is certainly a good idea. I wonder though if we
> might ever get into a situation where things change back and forth. I'd
> hope that properties are processed top to bottom so that, e.g.,
> PC_COMPAT_1_0, { ... } can overwrite 0_15 compat properties inherited
> through 1_0.
>
> Andreas
There are some properties like that currently.
For example, see ide-drive "ver" property or
VGA rombar property. I'm not sure that's intentional but I kept
it as is.
In that case we can simply keep it out of
the common macros, that's what I did. There's no rule that says
we must have everything in there.
> >
> > hw/pc_piix.c | 288 +++++++++++++++-------------------------------------------
> > 1 files changed, 73 insertions(+), 215 deletions(-)
> >
> > diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> > index 3f99f9a..e39ef69 100644
> > --- a/hw/pc_piix.c
> > +++ b/hw/pc_piix.c
> > @@ -372,50 +372,69 @@ static QEMUMachine pc_machine_v1_1 = {
> > .is_default = 1,
> > };
> >
> > +#define PC_COMPAT_1_0 \
> > + {\
> > + .driver = "pc-sysfw",\
> > + .property = "rom_only",\
> > + .value = stringify(1),\
> > + }, {\
> > + .driver = "isa-fdc",\
> > + .property = "check_media_rate",\
> > + .value = "off",\
> > + }
> > +
> > static QEMUMachine pc_machine_v1_0 = {
> > .name = "pc-1.0",
> > .desc = "Standard PC",
> > .init = pc_init_pci,
> > .max_cpus = 255,
> > .compat_props = (GlobalProperty[]) {
> > - {
> > - .driver = "pc-sysfw",
> > - .property = "rom_only",
> > - .value = stringify(1),
> > - }, {
> > - .driver = "isa-fdc",
> > - .property = "check_media_rate",
> > - .value = "off",
> > - },
> > + PC_COMPAT_1_0,
> > { /* end of list */ }
> > },
> > };
> [snip]
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] pc: reduce duplication in compat machine types
2012-03-26 12:51 ` Avi Kivity
@ 2012-03-26 13:32 ` Michael S. Tsirkin
0 siblings, 0 replies; 5+ messages in thread
From: Michael S. Tsirkin @ 2012-03-26 13:32 UTC (permalink / raw)
To: Avi Kivity
Cc: Anthony PERARD, Jan Kiszka, Anthony Liguori, qemu-devel, Alexander Graf
On Mon, Mar 26, 2012 at 02:51:38PM +0200, Avi Kivity wrote:
> On 03/26/2012 11:40 AM, Michael S. Tsirkin wrote:
> > Make it easier to add compat properties, by
> > adding macros for properties duplicated across
> > machine types.
> >
> > Note: there could be bugs in compat properties,
> > this patch does not attempt to address them,
> > the code is bug for bug identical to the original.
> >
> > Tested by: generated a preprocessed file, sorted and
> > compared to sorted original.
> > Lightly tested on x86_64.
> >
> >
> > +#define PC_COMPAT_1_0 \
> > + {\
> > + .driver = "pc-sysfw",\
> > + .property = "rom_only",\
> > + .value = stringify(1),\
> > + }, {\
> > + .driver = "isa-fdc",\
> > + .property = "check_media_rate",\
> > + .value = "off",\
> > + }
> > +
>
> Hmm. how about
>
> > static QEMUMachine pc_machine_v1_0 = {
> > .name = "pc-1.0",
> > .desc = "Standard PC",
> > .init = pc_init_pci,
> > .max_cpus = 255,
> > .compat_props = (GlobalProperty[]) {
> > - {
> > - .driver = "pc-sysfw",
> > - .property = "rom_only",
> > - .value = stringify(1),
> > - }, {
> > - .driver = "isa-fdc",
> > - .property = "check_media_rate",
> > - .value = "off",
> > - },
> > + PC_COMPAT_1_0,
>
> + .base_machine = &pc_machine_v1_1;
>
It's a lot of work, and the result won't be easier to
modify or debug than it already is, IMO.
> Then it would be easier to define machines differentially.
We add new 1 machine each release, so I'm not worried about
adding them easily, adding properies easily happens
between releases so I want to make is simpler.
> > { /* end of list */ }
> > },
>
> --
> error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-03-26 13:32 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-26 9:40 [Qemu-devel] [PATCH] pc: reduce duplication in compat machine types Michael S. Tsirkin
2012-03-26 12:51 ` Avi Kivity
2012-03-26 13:32 ` Michael S. Tsirkin
2012-03-26 13:12 ` Andreas Färber
2012-03-26 13:23 ` 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.