[Qemu-devel] [PATCH RESEND v2 0/3] pc: Set hw_version on all machine classes

[Qemu-devel] [PATCH RESEND v2 0/3] pc: Set hw_version on all machine classes

[Eduardo Habkost]

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.

Eduardo Habkost (3):
  pc: Set hw_version on all machine classes
  osdep: Rename qemu_{get,set}_version() to qemu_{,set_}hw_version()
  megasas: Use qemu_hw_version() instead of QEMU_VERSION

 hw/arm/nseries.c     |  2 +-
 hw/i386/pc_piix.c    | 13 +++++++++++++
 hw/i386/pc_q35.c     | 10 ++++++++++
 hw/ide/core.c        |  2 +-
 hw/scsi/megasas.c    |  2 +-
 hw/scsi/scsi-bus.c   |  2 +-
 hw/scsi/scsi-disk.c  |  2 +-
 include/qemu/osdep.h |  4 ++--
 target-i386/cpu.c    |  2 +-
 util/osdep.c         | 10 +++++-----
 vl.c                 |  2 +-
 11 files changed, 37 insertions(+), 14 deletions(-)

-- 
2.1.0

[Qemu-devel] [PATCH RESEND v2 1/3] pc: Set hw_version on all machine classes

[Eduardo Habkost]

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.

Reported-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
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 393dcc4..07d0baa 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -472,6 +472,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;
 }
@@ -484,6 +485,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;
@@ -497,6 +499,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);
@@ -509,6 +512,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);
 }
 
@@ -519,6 +523,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);
 }
@@ -531,6 +536,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);
 }
 
@@ -541,6 +547,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);
 }
@@ -552,6 +559,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);
 }
 
@@ -562,6 +570,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);
 }
 
@@ -572,6 +581,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);
 }
@@ -604,6 +614,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);
 }
 
@@ -642,6 +653,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);
 }
 
@@ -684,6 +696,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 2f8f396..0fdae09 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);
 }
-- 
2.1.0

[Qemu-devel] [PATCH RESEND v2 2/3] osdep: Rename qemu_{get, set}_version() to qemu_{, set_}hw_version()

[Eduardo Habkost]

This makes the purpose of the function clearer: it is not about the
version of QEMU that's running, but the version string exposed in the
emulated hardware.

Cc: Andrzej Zaborowski <balrogg@gmail.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: John Snow <jsnow@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/arm/nseries.c     |  2 +-
 hw/ide/core.c        |  2 +-
 hw/scsi/scsi-bus.c   |  2 +-
 hw/scsi/scsi-disk.c  |  2 +-
 include/qemu/osdep.h |  4 ++--
 target-i386/cpu.c    |  2 +-
 util/osdep.c         | 10 +++++-----
 vl.c                 |  2 +-
 8 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c
index 6a6b3e6..2a8835e 100644
--- a/hw/arm/nseries.c
+++ b/hw/arm/nseries.c
@@ -1275,7 +1275,7 @@ static int n8x0_atag_setup(void *p, int model)
     strcpy((void *) w, "hw-build");		/* char component[12] */
     w += 6;
     strcpy((void *) w, "QEMU ");
-    pstrcat((void *) w, 12, qemu_get_version()); /* char version[12] */
+    pstrcat((void *) w, 12, qemu_hw_version()); /* char version[12] */
     w += 6;
 
     tag = (model == 810) ? "1.1.10-qemu" : "1.1.6-qemu";
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 317406d..364ba21 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2312,7 +2312,7 @@ int ide_init_drive(IDEState *s, BlockBackend *blk, IDEDriveKind kind,
     if (version) {
         pstrcpy(s->version, sizeof(s->version), version);
     } else {
-        pstrcpy(s->version, sizeof(s->version), qemu_get_version());
+        pstrcpy(s->version, sizeof(s->version), qemu_hw_version());
     }
 
     ide_reset(s);
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index d373c1b..fd1171e 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -453,7 +453,7 @@ static bool scsi_target_emulate_inquiry(SCSITargetReq *r)
         r->buf[7] = 0x10 | (r->req.bus->info->tcq ? 0x02 : 0); /* Sync, TCQ.  */
         memcpy(&r->buf[8], "QEMU    ", 8);
         memcpy(&r->buf[16], "QEMU TARGET     ", 16);
-        pstrcpy((char *) &r->buf[32], 4, qemu_get_version());
+        pstrcpy((char *) &r->buf[32], 4, qemu_hw_version());
     }
     return true;
 }
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index bada9a7..707e734 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2315,7 +2315,7 @@ static void scsi_realize(SCSIDevice *dev, Error **errp)
     }
 
     if (!s->version) {
-        s->version = g_strdup(qemu_get_version());
+        s->version = g_strdup(qemu_hw_version());
     }
     if (!s->vendor) {
         s->vendor = g_strdup("QEMU");
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index b568424..ab2d5d9 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -247,8 +247,8 @@ static inline void qemu_timersub(const struct timeval *val1,
 
 void qemu_set_cloexec(int fd);
 
-void qemu_set_version(const char *);
-const char *qemu_get_version(void);
+void qemu_set_hw_version(const char *);
+const char *qemu_hw_version(void);
 
 void fips_set_state(bool requested);
 bool fips_get_state(void);
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 9280bfc..9d0eedf 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2243,7 +2243,7 @@ void x86_cpudef_setup(void)
                 pstrcpy(def->model_id, sizeof(def->model_id),
                         "QEMU Virtual CPU version ");
                 pstrcat(def->model_id, sizeof(def->model_id),
-                        qemu_get_version());
+                        qemu_hw_version());
                 break;
             }
         }
diff --git a/util/osdep.c b/util/osdep.c
index 0092bb6..80c6bfe 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -52,7 +52,7 @@ extern int madvise(caddr_t, size_t, int);
 
 static bool fips_enabled = false;
 
-static const char *qemu_version = QEMU_VERSION;
+static const char *hw_version = QEMU_VERSION;
 
 int socket_set_cork(int fd, int v)
 {
@@ -311,14 +311,14 @@ int qemu_accept(int s, struct sockaddr *addr, socklen_t *addrlen)
     return ret;
 }
 
-void qemu_set_version(const char *version)
+void qemu_set_hw_version(const char *version)
 {
-    qemu_version = version;
+    hw_version = version;
 }
 
-const char *qemu_get_version(void)
+const char *qemu_hw_version(void)
 {
-    return qemu_version;
+    return hw_version;
 }
 
 void fips_set_state(bool requested)
diff --git a/vl.c b/vl.c
index f5f7c3f..d7cdf96 100644
--- a/vl.c
+++ b/vl.c
@@ -4052,7 +4052,7 @@ int main(int argc, char **argv, char **envp)
     cpu_exec_init_all();
 
     if (machine_class->hw_version) {
-        qemu_set_version(machine_class->hw_version);
+        qemu_set_hw_version(machine_class->hw_version);
     }
 
     /* Init CPU def lists, based on config
-- 
2.1.0

[Qemu-devel] [PATCH RESEND v2 3/3] megasas: Use qemu_hw_version() instead of QEMU_VERSION

[Eduardo Habkost]

Guest visible data shouldn't change with a simple QEMU upgrade, so use
qemu_hw_version() to ensure it won't change (as long as the machine
class being used has hw_version set).

Cc: Hannes Reinecke <hare@suse.de>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-block@nongnu.org
Reviewed-by: Hannes Reinecke <hare@suse.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/scsi/megasas.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index dcd724e..d7dc667 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -757,7 +757,7 @@ static int megasas_ctrl_get_info(MegasasState *s, MegasasCmd *cmd)
 
     memcpy(info.product_name, base_class->product_name, 24);
     snprintf(info.serial_number, 32, "%s", s->hba_serial);
-    snprintf(info.package_version, 0x60, "%s-QEMU", QEMU_VERSION);
+    snprintf(info.package_version, 0x60, "%s-QEMU", qemu_hw_version());
     memcpy(info.image_component[0].name, "APP", 3);
     snprintf(info.image_component[0].version, 10, "%s-QEMU",
              base_class->product_version);
-- 
2.1.0

Re: [Qemu-devel] [PATCH RESEND v2 0/3] pc: Set hw_version on all machine classes

[Paolo Bonzini]

On 30/10/2015 20:36, 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.
> 
> Eduardo Habkost (3):
>   pc: Set hw_version on all machine classes
>   osdep: Rename qemu_{get,set}_version() to qemu_{,set_}hw_version()
>   megasas: Use qemu_hw_version() instead of QEMU_VERSION
> 
>  hw/arm/nseries.c     |  2 +-
>  hw/i386/pc_piix.c    | 13 +++++++++++++
>  hw/i386/pc_q35.c     | 10 ++++++++++
>  hw/ide/core.c        |  2 +-
>  hw/scsi/megasas.c    |  2 +-
>  hw/scsi/scsi-bus.c   |  2 +-
>  hw/scsi/scsi-disk.c  |  2 +-
>  include/qemu/osdep.h |  4 ++--
>  target-i386/cpu.c    |  2 +-
>  util/osdep.c         | 10 +++++-----
>  vl.c                 |  2 +-
>  11 files changed, 37 insertions(+), 14 deletions(-)
> 

Michael, is it okay for you if I merge this patch series?

Paolo

Re: [Qemu-devel] [PATCH RESEND v2 1/3] pc: Set hw_version on all machine classes

[Michael S. Tsirkin]

On Fri, Oct 30, 2015 at 05:36:07PM -0200, 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.
> 
> Reported-by: Laszlo Ersek <lersek@redhat.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Ouch.

I really don't want even more churn with each version.

Can't we use the name supplied to DEFINE_PC_MACHINE at least for future
machine types?

Or maybe we should stop exposing the version to guests - does it really
have any value given it has been so unreliable historically?

How about:

---

diff --git a/util/osdep.c b/util/osdep.c
index 0092bb6..4dc635d 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -52,7 +52,7 @@ extern int madvise(caddr_t, size_t, int);
 
 static bool fips_enabled = false;
 
-static const char *qemu_version = QEMU_VERSION;
+static const char *qemu_version = "QEMU";
 
 int socket_set_cork(int fd, int v)
 {

Re: [Qemu-devel] [PATCH RESEND v2 0/3] pc: Set hw_version on all machine classes

[Michael S. Tsirkin]

On Mon, Nov 02, 2015 at 12:55:07PM +0100, Paolo Bonzini wrote:
> 
> 
> On 30/10/2015 20:36, 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.
> > 
> > Eduardo Habkost (3):
> >   pc: Set hw_version on all machine classes
> >   osdep: Rename qemu_{get,set}_version() to qemu_{,set_}hw_version()
> >   megasas: Use qemu_hw_version() instead of QEMU_VERSION
> > 
> >  hw/arm/nseries.c     |  2 +-
> >  hw/i386/pc_piix.c    | 13 +++++++++++++
> >  hw/i386/pc_q35.c     | 10 ++++++++++
> >  hw/ide/core.c        |  2 +-
> >  hw/scsi/megasas.c    |  2 +-
> >  hw/scsi/scsi-bus.c   |  2 +-
> >  hw/scsi/scsi-disk.c  |  2 +-
> >  include/qemu/osdep.h |  4 ++--
> >  target-i386/cpu.c    |  2 +-
> >  util/osdep.c         | 10 +++++-----
> >  vl.c                 |  2 +-
> >  11 files changed, 37 insertions(+), 14 deletions(-)
> > 
> 
> Michael, is it okay for you if I merge this patch series?
> 
> Paolo

Sorry about missing this the 1st time around.
Let's discuss the right thing to do here -
it's a bugfix so we can merge it after hard freeze.


-- 
MST

Re: [Qemu-devel] [PATCH RESEND v2 0/3] pc: Set hw_version on all machine classes

[Paolo Bonzini]

On 09/11/2015 21:46, Michael S. Tsirkin wrote:
> On Mon, Nov 02, 2015 at 12:55:07PM +0100, Paolo Bonzini wrote:
>> On 30/10/2015 20:36, 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.
>>>
>>> Eduardo Habkost (3):
>>>   pc: Set hw_version on all machine classes
>>>   osdep: Rename qemu_{get,set}_version() to qemu_{,set_}hw_version()
>>>   megasas: Use qemu_hw_version() instead of QEMU_VERSION
>>>
>>>  hw/arm/nseries.c     |  2 +-
>>>  hw/i386/pc_piix.c    | 13 +++++++++++++
>>>  hw/i386/pc_q35.c     | 10 ++++++++++
>>>  hw/ide/core.c        |  2 +-
>>>  hw/scsi/megasas.c    |  2 +-
>>>  hw/scsi/scsi-bus.c   |  2 +-
>>>  hw/scsi/scsi-disk.c  |  2 +-
>>>  include/qemu/osdep.h |  4 ++--
>>>  target-i386/cpu.c    |  2 +-
>>>  util/osdep.c         | 10 +++++-----
>>>  vl.c                 |  2 +-
>>>  11 files changed, 37 insertions(+), 14 deletions(-)
>>>
>>
>> Michael, is it okay for you if I merge this patch series?
> 
> Sorry about missing this the 1st time around.
> Let's discuss the right thing to do here -
> it's a bugfix so we can merge it after hard freeze.

It's already in...

Paolo

Re: [Qemu-devel] [PATCH RESEND v2 0/3] pc: Set hw_version on all machine classes

[Michael S. Tsirkin]

On Tue, Nov 10, 2015 at 04:01:51PM +0100, Paolo Bonzini wrote:
> 
> 
> On 09/11/2015 21:46, Michael S. Tsirkin wrote:
> > On Mon, Nov 02, 2015 at 12:55:07PM +0100, Paolo Bonzini wrote:
> >> On 30/10/2015 20:36, 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.
> >>>
> >>> Eduardo Habkost (3):
> >>>   pc: Set hw_version on all machine classes
> >>>   osdep: Rename qemu_{get,set}_version() to qemu_{,set_}hw_version()
> >>>   megasas: Use qemu_hw_version() instead of QEMU_VERSION
> >>>
> >>>  hw/arm/nseries.c     |  2 +-
> >>>  hw/i386/pc_piix.c    | 13 +++++++++++++
> >>>  hw/i386/pc_q35.c     | 10 ++++++++++
> >>>  hw/ide/core.c        |  2 +-
> >>>  hw/scsi/megasas.c    |  2 +-
> >>>  hw/scsi/scsi-bus.c   |  2 +-
> >>>  hw/scsi/scsi-disk.c  |  2 +-
> >>>  include/qemu/osdep.h |  4 ++--
> >>>  target-i386/cpu.c    |  2 +-
> >>>  util/osdep.c         | 10 +++++-----
> >>>  vl.c                 |  2 +-
> >>>  11 files changed, 37 insertions(+), 14 deletions(-)
> >>>
> >>
> >> Michael, is it okay for you if I merge this patch series?
> > 
> > Sorry about missing this the 1st time around.
> > Let's discuss the right thing to do here -
> > it's a bugfix so we can merge it after hard freeze.
> 
> It's already in...
> 
> Paolo

How about a patch that just drops QEMU_VERSION from there?

-- 
MST

Re: [Qemu-devel] [PATCH RESEND v2 1/3] pc: Set hw_version on all machine classes

[Eduardo Habkost]

On Mon, Nov 09, 2015 at 10:45:17PM +0200, Michael S. Tsirkin wrote:
> On Fri, Oct 30, 2015 at 05:36:07PM -0200, 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.
> > 
> > Reported-by: Laszlo Ersek <lersek@redhat.com>
> > Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> 
> Ouch.
> 
> I really don't want even more churn with each version.
> 
> Can't we use the name supplied to DEFINE_PC_MACHINE at least for future
> machine types?
> 
> Or maybe we should stop exposing the version to guests - does it really
> have any value given it has been so unreliable historically?

I'm all for stopping exposing it to guests. This is where we expose it,
currently:

hw/arm/nseries.c:    pstrcat((void *) w, 12, qemu_hw_version()); /* char version[12] */
hw/ide/core.c:        pstrcpy(s->version, sizeof(s->version), qemu_hw_version());
hw/scsi/megasas.c:    snprintf(info.package_version, 0x60, "%s-QEMU", qemu_hw_version());
hw/scsi/scsi-bus.c:        pstrcpy((char *) &r->buf[32], 4, qemu_hw_version());
hw/scsi/scsi-disk.c:        s->version = g_strdup(qemu_hw_version());
target-i386/cpu.c:                        qemu_hw_version());


> 
> How about:
> 
> ---
> 
> diff --git a/util/osdep.c b/util/osdep.c
> index 0092bb6..4dc635d 100644
> --- a/util/osdep.c
> +++ b/util/osdep.c
> @@ -52,7 +52,7 @@ extern int madvise(caddr_t, size_t, int);
>  
>  static bool fips_enabled = false;
>  
> -static const char *qemu_version = QEMU_VERSION;
> +static const char *qemu_version = "QEMU";

This would lead to stuff like "QEMU virtual CPU version QEMU", and
OMAP_TAG_VERSION_STR/hw-build set to "QEMU QEMU" on nseries. I believe
we must first change the IDE, SCSI and CPUID code to stop using
qemu_hw_version() on new machine-types.

After that, we could even make qemu_hw_version() call abort() on newer PC
machine-types (meaning only the old machines would be allowed to use it, to
keep compatibility).

-- 
Eduardo