[Qemu-devel] [PATCH] pc: reduce duplication in compat machine types
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 m...@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, -
Re: [Qemu-devel] [PATCH] pc: reduce duplication in compat machine types
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
Re: [Qemu-devel] [PATCH] pc: reduce duplication in compat machine types
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 m...@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
Re: [Qemu-devel] [PATCH] pc: reduce duplication in compat machine types
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 m...@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
Re: [Qemu-devel] [PATCH] pc: reduce duplication in compat machine types
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