[1/3] pc: Set hw_version on all machine classes

In 2012, QEMU had a bug where it exposed QEMU version information to the
guest, meaning a QEMU upgrade would expose different hardware to the
guest OS even if the same machine-type is being used.

The bug was fixed by commit 93bfef4c6e4b23caea9d51e1099d06433d8835a4, on
all machines up to pc-1.0. But we kept introducing the same bug on all
newer machines since then. That means we are breaking guest ABI every
time QEMU was upgraded.

Fix this by setting the hw_version on all PC machines, making sure the
hardware won't change when upgrading QEMU.

Note that QEMU_VERSION was "1.0" in QEMU 1.0, but starting on QEMU
1.1.0, it started following the "x.y.0" pattern. We have to follow it,
to make sure we use the right QEMU_VERSION string from each QEMU
release.

The 2.5 machine classes could have hw_version unset, because the default
value for qemu_get_version() is QEMU_VERSION. But I decided to set it
explicitly to QEMU_VERSION so we don't forget to update it to "2.5.0"
after we release 2.5.0 and create a 2.6 machine class.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc_piix.c | 13 +++++++++++++
 hw/i386/pc_q35.c  | 10 ++++++++++
 2 files changed, 23 insertions(+)

On 09/22/15 22:16, Eduardo Habkost wrote:
> In 2012, QEMU had a bug where it exposed QEMU version information to the
> guest, meaning a QEMU upgrade would expose different hardware to the
> guest OS even if the same machine-type is being used.
> 
> The bug was fixed by commit 93bfef4c6e4b23caea9d51e1099d06433d8835a4, on
> all machines up to pc-1.0. But we kept introducing the same bug on all
> newer machines since then. That means we are breaking guest ABI every
> time QEMU was upgraded.
> 
> Fix this by setting the hw_version on all PC machines, making sure the
> hardware won't change when upgrading QEMU.
> 
> Note that QEMU_VERSION was "1.0" in QEMU 1.0, but starting on QEMU
> 1.1.0, it started following the "x.y.0" pattern. We have to follow it,
> to make sure we use the right QEMU_VERSION string from each QEMU
> release.
> 
> The 2.5 machine classes could have hw_version unset, because the default
> value for qemu_get_version() is QEMU_VERSION. But I decided to set it
> explicitly to QEMU_VERSION so we don't forget to update it to "2.5.0"
> after we release 2.5.0 and create a 2.6 machine class.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  hw/i386/pc_piix.c | 13 +++++++++++++
>  hw/i386/pc_q35.c  | 10 ++++++++++
>  2 files changed, 23 insertions(+)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 3ffb05f..0539467 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -467,6 +467,7 @@ static void pc_i440fx_machine_options(MachineClass *m)
>  static void pc_i440fx_2_5_machine_options(MachineClass *m)
>  {
>      pc_i440fx_machine_options(m);
> +    m->hw_version = QEMU_VERSION;
>      m->alias = "pc";
>      m->is_default = 1;
>  }
> @@ -479,6 +480,7 @@ static void pc_i440fx_2_4_machine_options(MachineClass *m)
>  {
>      PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
>      pc_i440fx_2_5_machine_options(m);
> +    m->hw_version = "2.4.0";
>      m->alias = NULL;
>      m->is_default = 0;
>      pcmc->broken_reserved_end = true;
> @@ -492,6 +494,7 @@ DEFINE_I440FX_MACHINE(v2_4, "pc-i440fx-2.4", NULL,
>  static void pc_i440fx_2_3_machine_options(MachineClass *m)
>  {
>      pc_i440fx_2_4_machine_options(m);
> +    m->hw_version = "2.3.0";
>      m->alias = NULL;
>      m->is_default = 0;
>      SET_MACHINE_COMPAT(m, PC_COMPAT_2_3);
> @@ -504,6 +507,7 @@ DEFINE_I440FX_MACHINE(v2_3, "pc-i440fx-2.3", pc_compat_2_3,
>  static void pc_i440fx_2_2_machine_options(MachineClass *m)
>  {
>      pc_i440fx_2_3_machine_options(m);
> +    m->hw_version = "2.2.0";
>      SET_MACHINE_COMPAT(m, PC_COMPAT_2_2);
>  }
>  
> @@ -514,6 +518,7 @@ DEFINE_I440FX_MACHINE(v2_2, "pc-i440fx-2.2", pc_compat_2_2,
>  static void pc_i440fx_2_1_machine_options(MachineClass *m)
>  {
>      pc_i440fx_2_2_machine_options(m);
> +    m->hw_version = "2.1.0";
>      m->default_display = NULL;
>      SET_MACHINE_COMPAT(m, PC_COMPAT_2_1);
>  }
> @@ -526,6 +531,7 @@ DEFINE_I440FX_MACHINE(v2_1, "pc-i440fx-2.1", pc_compat_2_1,
>  static void pc_i440fx_2_0_machine_options(MachineClass *m)
>  {
>      pc_i440fx_2_1_machine_options(m);
> +    m->hw_version = "2.0.0";
>      SET_MACHINE_COMPAT(m, PC_COMPAT_2_0);
>  }
>  
> @@ -536,6 +542,7 @@ DEFINE_I440FX_MACHINE(v2_0, "pc-i440fx-2.0", pc_compat_2_0,
>  static void pc_i440fx_1_7_machine_options(MachineClass *m)
>  {
>      pc_i440fx_2_0_machine_options(m);
> +    m->hw_version = "1.7.0";
>      m->default_machine_opts = NULL;
>      SET_MACHINE_COMPAT(m, PC_COMPAT_1_7);
>  }
> @@ -547,6 +554,7 @@ DEFINE_I440FX_MACHINE(v1_7, "pc-i440fx-1.7", pc_compat_1_7,
>  static void pc_i440fx_1_6_machine_options(MachineClass *m)
>  {
>      pc_i440fx_1_7_machine_options(m);
> +    m->hw_version = "1.6.0";
>      SET_MACHINE_COMPAT(m, PC_COMPAT_1_6);
>  }
>  
> @@ -557,6 +565,7 @@ DEFINE_I440FX_MACHINE(v1_6, "pc-i440fx-1.6", pc_compat_1_6,
>  static void pc_i440fx_1_5_machine_options(MachineClass *m)
>  {
>      pc_i440fx_1_6_machine_options(m);
> +    m->hw_version = "1.5.0";
>      SET_MACHINE_COMPAT(m, PC_COMPAT_1_5);
>  }
>  
> @@ -567,6 +576,7 @@ DEFINE_I440FX_MACHINE(v1_5, "pc-i440fx-1.5", pc_compat_1_5,
>  static void pc_i440fx_1_4_machine_options(MachineClass *m)
>  {
>      pc_i440fx_1_5_machine_options(m);
> +    m->hw_version = "1.4.0";
>      m->hot_add_cpu = NULL;
>      SET_MACHINE_COMPAT(m, PC_COMPAT_1_4);
>  }
> @@ -599,6 +609,7 @@ DEFINE_I440FX_MACHINE(v1_4, "pc-i440fx-1.4", pc_compat_1_4,
>  static void pc_i440fx_1_3_machine_options(MachineClass *m)
>  {
>      pc_i440fx_1_4_machine_options(m);
> +    m->hw_version = "1.3.0";
>      SET_MACHINE_COMPAT(m, PC_COMPAT_1_3);
>  }
>  
> @@ -637,6 +648,7 @@ DEFINE_I440FX_MACHINE(v1_3, "pc-1.3", pc_compat_1_3,
>  static void pc_i440fx_1_2_machine_options(MachineClass *m)
>  {
>      pc_i440fx_1_3_machine_options(m);
> +    m->hw_version = "1.2.0";
>      SET_MACHINE_COMPAT(m, PC_COMPAT_1_2);
>  }
>  
> @@ -679,6 +691,7 @@ DEFINE_I440FX_MACHINE(v1_2, "pc-1.2", pc_compat_1_2,
>  static void pc_i440fx_1_1_machine_options(MachineClass *m)
>  {
>      pc_i440fx_1_2_machine_options(m);
> +    m->hw_version = "1.1.0";
>      SET_MACHINE_COMPAT(m, PC_COMPAT_1_1);
>  }
>  
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 1b7d3b6..2aa1815 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -373,6 +373,7 @@ static void pc_q35_machine_options(MachineClass *m)
>  static void pc_q35_2_5_machine_options(MachineClass *m)
>  {
>      pc_q35_machine_options(m);
> +    m->hw_version = QEMU_VERSION;
>      m->alias = "q35";
>  }
>  
> @@ -383,6 +384,7 @@ static void pc_q35_2_4_machine_options(MachineClass *m)
>  {
>      PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
>      pc_q35_2_5_machine_options(m);
> +    m->hw_version = "2.4.0";
>      m->alias = NULL;
>      pcmc->broken_reserved_end = true;
>      SET_MACHINE_COMPAT(m, PC_COMPAT_2_4);
> @@ -395,6 +397,7 @@ DEFINE_Q35_MACHINE(v2_4, "pc-q35-2.4", NULL,
>  static void pc_q35_2_3_machine_options(MachineClass *m)
>  {
>      pc_q35_2_4_machine_options(m);
> +    m->hw_version = "2.3.0";
>      m->no_floppy = 0;
>      m->no_tco = 1;
>      m->alias = NULL;
> @@ -408,6 +411,7 @@ DEFINE_Q35_MACHINE(v2_3, "pc-q35-2.3", pc_compat_2_3,
>  static void pc_q35_2_2_machine_options(MachineClass *m)
>  {
>      pc_q35_2_3_machine_options(m);
> +    m->hw_version = "2.2.0";
>      SET_MACHINE_COMPAT(m, PC_COMPAT_2_2);
>  }
>  
> @@ -418,6 +422,7 @@ DEFINE_Q35_MACHINE(v2_2, "pc-q35-2.2", pc_compat_2_2,
>  static void pc_q35_2_1_machine_options(MachineClass *m)
>  {
>      pc_q35_2_2_machine_options(m);
> +    m->hw_version = "2.1.0";
>      m->default_display = NULL;
>      SET_MACHINE_COMPAT(m, PC_COMPAT_2_1);
>  }
> @@ -429,6 +434,7 @@ DEFINE_Q35_MACHINE(v2_1, "pc-q35-2.1", pc_compat_2_1,
>  static void pc_q35_2_0_machine_options(MachineClass *m)
>  {
>      pc_q35_2_1_machine_options(m);
> +    m->hw_version = "2.0.0";
>      SET_MACHINE_COMPAT(m, PC_COMPAT_2_0);
>  }
>  
> @@ -439,6 +445,7 @@ DEFINE_Q35_MACHINE(v2_0, "pc-q35-2.0", pc_compat_2_0,
>  static void pc_q35_1_7_machine_options(MachineClass *m)
>  {
>      pc_q35_2_0_machine_options(m);
> +    m->hw_version = "1.7.0";
>      m->default_machine_opts = NULL;
>      SET_MACHINE_COMPAT(m, PC_COMPAT_1_7);
>  }
> @@ -450,6 +457,7 @@ DEFINE_Q35_MACHINE(v1_7, "pc-q35-1.7", pc_compat_1_7,
>  static void pc_q35_1_6_machine_options(MachineClass *m)
>  {
>      pc_q35_machine_options(m);
> +    m->hw_version = "1.6.0";
>      SET_MACHINE_COMPAT(m, PC_COMPAT_1_6);
>  }
>  
> @@ -460,6 +468,7 @@ DEFINE_Q35_MACHINE(v1_6, "pc-q35-1.6", pc_compat_1_6,
>  static void pc_q35_1_5_machine_options(MachineClass *m)
>  {
>      pc_q35_1_6_machine_options(m);
> +    m->hw_version = "1.5.0";
>      SET_MACHINE_COMPAT(m, PC_COMPAT_1_5);
>  }
>  
> @@ -470,6 +479,7 @@ DEFINE_Q35_MACHINE(v1_5, "pc-q35-1.5", pc_compat_1_5,
>  static void pc_q35_1_4_machine_options(MachineClass *m)
>  {
>      pc_q35_1_5_machine_options(m);
> +    m->hw_version = "1.4.0";
>      m->hot_add_cpu = NULL;
>      SET_MACHINE_COMPAT(m, PC_COMPAT_1_4);
>  }
> 

Can you please add:

Reported-by: Laszlo Ersek <lersek@redhat.com>

to this patch? (If you agree, that is.)

Other than that, while I'm sure you'll need (and get) other reviews too:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo

On Tue, Sep 22, 2015 at 10:26:14PM +0200, Laszlo Ersek wrote:
> On 09/22/15 22:16, Eduardo Habkost wrote:
> > In 2012, QEMU had a bug where it exposed QEMU version information to the
> > guest, meaning a QEMU upgrade would expose different hardware to the
> > guest OS even if the same machine-type is being used.
> > 
> > The bug was fixed by commit 93bfef4c6e4b23caea9d51e1099d06433d8835a4, on
> > all machines up to pc-1.0. But we kept introducing the same bug on all
> > newer machines since then. That means we are breaking guest ABI every
> > time QEMU was upgraded.
> > 
> > Fix this by setting the hw_version on all PC machines, making sure the
> > hardware won't change when upgrading QEMU.
> > 
> > Note that QEMU_VERSION was "1.0" in QEMU 1.0, but starting on QEMU
> > 1.1.0, it started following the "x.y.0" pattern. We have to follow it,
> > to make sure we use the right QEMU_VERSION string from each QEMU
> > release.
> > 
> > The 2.5 machine classes could have hw_version unset, because the default
> > value for qemu_get_version() is QEMU_VERSION. But I decided to set it
> > explicitly to QEMU_VERSION so we don't forget to update it to "2.5.0"
> > after we release 2.5.0 and create a 2.6 machine class.
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
[...]
> 
> Can you please add:
> 
> Reported-by: Laszlo Ersek <lersek@redhat.com>
> 
> to this patch? (If you agree, that is.)

Of course! :)

(Sorry, while trying to finish and submit the series before the end of
the day, I forgot to give you credit for finding this 3-year-old bug)

> 
> Other than that, while I'm sure you'll need (and get) other reviews too:
> 
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks!