[Qemu-devel] [PATCH] pc: reduce duplication in compat machine types

2012-03-26 Thread 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.

 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

2012-03-26 Thread Avi Kivity 
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

2012-03-26 Thread Andreas Färber 
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

2012-03-26 Thread Michael S. Tsirkin 
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

2012-03-26 Thread Michael S. Tsirkin 
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