All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/3] pc: Set hw_version on all machine classes
@ 2015-09-22 20:16 Eduardo Habkost
  2015-09-22 20:16 ` [Qemu-devel] [PATCH 1/3] " Eduardo Habkost
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Eduardo Habkost @ 2015-09-22 20:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Marcel Apfelbaum, Laszlo Ersek, Michael S. Tsirkin

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.

This series is based on Michael's PCI tree, plus the "Set
broken_reserved_end on pc-*-2.4, not 2.5" patch I submitted
earlier today. Git tree for reference:
  git://github.com/ehabkost/qemu-hacks.git work/fix-hw-version

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH 1/3] pc: Set hw_version on all machine classes
  2015-09-22 20:16 [Qemu-devel] [PATCH 0/3] pc: Set hw_version on all machine classes Eduardo Habkost
@ 2015-09-22 20:16 ` Eduardo Habkost
  2015-09-22 20:26   ` Laszlo Ersek
  2015-09-22 20:16 ` [Qemu-devel] [PATCH 2/3] osdep: Rename qemu_{get, set}_version() to qemu_{, set_}hw_version() Eduardo Habkost
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Eduardo Habkost @ 2015-09-22 20:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Marcel Apfelbaum, Laszlo Ersek, Michael S. Tsirkin

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);
 }
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH 2/3] osdep: Rename qemu_{get, set}_version() to qemu_{, set_}hw_version()
  2015-09-22 20:16 [Qemu-devel] [PATCH 0/3] pc: Set hw_version on all machine classes Eduardo Habkost
  2015-09-22 20:16 ` [Qemu-devel] [PATCH 1/3] " Eduardo Habkost
@ 2015-09-22 20:16 ` Eduardo Habkost
  2015-09-22 20:21   ` John Snow
  2015-09-22 20:16 ` [Qemu-devel] [PATCH 3/3] megasas: Use qemu_hw_version() instead of QEMU_VERSION Eduardo Habkost
  2015-09-23  8:04 ` [Qemu-devel] [PATCH 0/3] pc: Set hw_version on all machine classes Igor Mammedov
  3 siblings, 1 reply; 14+ messages in thread
From: Eduardo Habkost @ 2015-09-22 20:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Marcel Apfelbaum, Laszlo Ersek,
	Michael S. Tsirkin, John Snow, Paolo Bonzini

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>
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 a659e85..ce05ffc 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 50449ca..c503c69 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2313,7 +2313,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 ffac8f4..3997007 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 ab3c876..b4ee266 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -243,8 +243,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 7c52714..f9284d5 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2231,7 +2231,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 3c6480d..204fb08 100644
--- a/vl.c
+++ b/vl.c
@@ -4078,7 +4078,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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH 3/3] megasas: Use qemu_hw_version() instead of QEMU_VERSION
  2015-09-22 20:16 [Qemu-devel] [PATCH 0/3] pc: Set hw_version on all machine classes Eduardo Habkost
  2015-09-22 20:16 ` [Qemu-devel] [PATCH 1/3] " Eduardo Habkost
  2015-09-22 20:16 ` [Qemu-devel] [PATCH 2/3] osdep: Rename qemu_{get, set}_version() to qemu_{, set_}hw_version() Eduardo Habkost
@ 2015-09-22 20:16 ` Eduardo Habkost
  2015-09-22 20:21   ` Hannes Reinecke
  2015-09-22 20:33   ` Laszlo Ersek
  2015-09-23  8:04 ` [Qemu-devel] [PATCH 0/3] pc: Set hw_version on all machine classes Igor Mammedov
  3 siblings, 2 replies; 14+ messages in thread
From: Eduardo Habkost @ 2015-09-22 20:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-block, Marcel Apfelbaum, Michael S. Tsirkin,
	Hannes Reinecke, Paolo Bonzini, Laszlo Ersek

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
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 a04369c..e0529b1 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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] megasas: Use qemu_hw_version() instead of QEMU_VERSION
  2015-09-22 20:16 ` [Qemu-devel] [PATCH 3/3] megasas: Use qemu_hw_version() instead of QEMU_VERSION Eduardo Habkost
@ 2015-09-22 20:21   ` Hannes Reinecke
  2015-09-22 20:33   ` Laszlo Ersek
  1 sibling, 0 replies; 14+ messages in thread
From: Hannes Reinecke @ 2015-09-22 20:21 UTC (permalink / raw)
  To: Eduardo Habkost, qemu-devel
  Cc: Paolo Bonzini, Marcel Apfelbaum, Laszlo Ersek, qemu-block,
	Michael S. Tsirkin

On 09/22/2015 10:16 PM, Eduardo Habkost wrote:
> 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
> 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 a04369c..e0529b1 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);
> 
Ah. right. Should be okay, then.

Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 2/3] osdep: Rename qemu_{get, set}_version() to qemu_{, set_}hw_version()
  2015-09-22 20:16 ` [Qemu-devel] [PATCH 2/3] osdep: Rename qemu_{get, set}_version() to qemu_{, set_}hw_version() Eduardo Habkost
@ 2015-09-22 20:21   ` John Snow
  0 siblings, 0 replies; 14+ messages in thread
From: John Snow @ 2015-09-22 20:21 UTC (permalink / raw)
  To: Eduardo Habkost, qemu-devel
  Cc: Peter Maydell, Michael S. Tsirkin, Laszlo Ersek, Paolo Bonzini,
	Marcel Apfelbaum



On 09/22/2015 04:16 PM, Eduardo Habkost wrote:
> 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>
> 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 a659e85..ce05ffc 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 50449ca..c503c69 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -2313,7 +2313,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 ffac8f4..3997007 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 ab3c876..b4ee266 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -243,8 +243,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 7c52714..f9284d5 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -2231,7 +2231,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 3c6480d..204fb08 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4078,7 +4078,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
> 

Reviewed-by: John Snow <jsnow@redhat.com>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] pc: Set hw_version on all machine classes
  2015-09-22 20:16 ` [Qemu-devel] [PATCH 1/3] " Eduardo Habkost
@ 2015-09-22 20:26   ` Laszlo Ersek
  2015-09-22 20:34     ` Eduardo Habkost
  0 siblings, 1 reply; 14+ messages in thread
From: Laszlo Ersek @ 2015-09-22 20:26 UTC (permalink / raw)
  To: Eduardo Habkost, qemu-devel
  Cc: Paolo Bonzini, Marcel Apfelbaum, Michael S. Tsirkin

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] megasas: Use qemu_hw_version() instead of QEMU_VERSION
  2015-09-22 20:16 ` [Qemu-devel] [PATCH 3/3] megasas: Use qemu_hw_version() instead of QEMU_VERSION Eduardo Habkost
  2015-09-22 20:21   ` Hannes Reinecke
@ 2015-09-22 20:33   ` Laszlo Ersek
  2015-09-22 20:36     ` Eduardo Habkost
  1 sibling, 1 reply; 14+ messages in thread
From: Laszlo Ersek @ 2015-09-22 20:33 UTC (permalink / raw)
  To: Eduardo Habkost, qemu-devel
  Cc: Paolo Bonzini, Marcel Apfelbaum, Hannes Reinecke, qemu-block,
	Michael S. Tsirkin

On 09/22/15 22:16, Eduardo Habkost wrote:
> 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
> 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 a04369c..e0529b1 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);
> 

I assume you audited all uses of QEMU_VERSION, and this was the only one
exposed to the guest directly.

However, in "hw/usb/redirect.c", QEMU_VERSION is embedded in VERSION,
and the latter is then passed to usbredirparser_init() in
usbredir_create_parser().

I tried to look up the documentation for usbredirparser_init() in
"/usr/include/usbredirparser.h", but I still have no clue what that
"version" parameter controls.

Hm... from the source code of usbredir, and
<http://www.spice-space.org/page/UsbRedir> stating "usbredir is the name
of a network protocol for sending usb device traffic over a network
connection", it looks like the version number is embedded in the hello
message of that network protocol; so it shouldn't be exposed to the
guest indeed.

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

Thanks
Laszlo

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 1/3] pc: Set hw_version on all machine classes
  2015-09-22 20:26   ` Laszlo Ersek
@ 2015-09-22 20:34     ` Eduardo Habkost
  0 siblings, 0 replies; 14+ messages in thread
From: Eduardo Habkost @ 2015-09-22 20:34 UTC (permalink / raw)
  To: Laszlo Ersek
  Cc: Paolo Bonzini, Marcel Apfelbaum, qemu-devel, Michael S. Tsirkin

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!

-- 
Eduardo

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] megasas: Use qemu_hw_version() instead of QEMU_VERSION
  2015-09-22 20:33   ` Laszlo Ersek
@ 2015-09-22 20:36     ` Eduardo Habkost
  0 siblings, 0 replies; 14+ messages in thread
From: Eduardo Habkost @ 2015-09-22 20:36 UTC (permalink / raw)
  To: Laszlo Ersek
  Cc: qemu-block, Marcel Apfelbaum, Michael S. Tsirkin, qemu-devel,
	Hannes Reinecke, Paolo Bonzini

On Tue, Sep 22, 2015 at 10:33:32PM +0200, Laszlo Ersek wrote:
> On 09/22/15 22:16, Eduardo Habkost wrote:
> > 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
> > 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 a04369c..e0529b1 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);
> > 
> 
> I assume you audited all uses of QEMU_VERSION, and this was the only one
> exposed to the guest directly.
> 
> However, in "hw/usb/redirect.c", QEMU_VERSION is embedded in VERSION,
> and the latter is then passed to usbredirparser_init() in
> usbredir_create_parser().
> 
> I tried to look up the documentation for usbredirparser_init() in
> "/usr/include/usbredirparser.h", but I still have no clue what that
> "version" parameter controls.
> 
> Hm... from the source code of usbredir, and
> <http://www.spice-space.org/page/UsbRedir> stating "usbredir is the name
> of a network protocol for sending usb device traffic over a network
> connection", it looks like the version number is embedded in the hello
> message of that network protocol; so it shouldn't be exposed to the
> guest indeed.

Investigating how VERSION was used in usbredirparser was next on my
todo-list. Thanks for doing that. :)

-- 
Eduardo

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 0/3] pc: Set hw_version on all machine classes
  2015-09-22 20:16 [Qemu-devel] [PATCH 0/3] pc: Set hw_version on all machine classes Eduardo Habkost
                   ` (2 preceding siblings ...)
  2015-09-22 20:16 ` [Qemu-devel] [PATCH 3/3] megasas: Use qemu_hw_version() instead of QEMU_VERSION Eduardo Habkost
@ 2015-09-23  8:04 ` Igor Mammedov
  2015-09-23 10:37   ` Laszlo Ersek
  3 siblings, 1 reply; 14+ messages in thread
From: Igor Mammedov @ 2015-09-23  8:04 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Paolo Bonzini, Michael S. Tsirkin, Laszlo Ersek, qemu-devel,
	Marcel Apfelbaum

On Tue, 22 Sep 2015 17:16:24 -0300
Eduardo Habkost <ehabkost@redhat.com> 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.
> 
> This series is based on Michael's PCI tree, plus the "Set
> broken_reserved_end on pc-*-2.4, not 2.5" patch I submitted
> earlier today.
I haven't seen this patch on list, perhaps it needs to be resubmitted?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 0/3] pc: Set hw_version on all machine classes
  2015-09-23  8:04 ` [Qemu-devel] [PATCH 0/3] pc: Set hw_version on all machine classes Igor Mammedov
@ 2015-09-23 10:37   ` Laszlo Ersek
  2015-09-23 11:47     ` Igor Mammedov
  0 siblings, 1 reply; 14+ messages in thread
From: Laszlo Ersek @ 2015-09-23 10:37 UTC (permalink / raw)
  To: Igor Mammedov, Eduardo Habkost
  Cc: Paolo Bonzini, Michael S. Tsirkin, qemu-devel, Marcel Apfelbaum

On 09/23/15 10:04, Igor Mammedov wrote:
> On Tue, 22 Sep 2015 17:16:24 -0300
> Eduardo Habkost <ehabkost@redhat.com> 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.
>>
>> This series is based on Michael's PCI tree, plus the "Set
>> broken_reserved_end on pc-*-2.4, not 2.5" patch I submitted
>> earlier today.
> I haven't seen this patch on list, perhaps it needs to be resubmitted?

http://lists.nongnu.org/archive/html/qemu-devel/2015-09/msg05778.html

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 0/3] pc: Set hw_version on all machine classes
  2015-09-23 10:37   ` Laszlo Ersek
@ 2015-09-23 11:47     ` Igor Mammedov
  2015-09-23 12:31       ` Laszlo Ersek
  0 siblings, 1 reply; 14+ messages in thread
From: Igor Mammedov @ 2015-09-23 11:47 UTC (permalink / raw)
  To: Laszlo Ersek
  Cc: Paolo Bonzini, Marcel Apfelbaum, Michael S. Tsirkin,
	Eduardo Habkost, qemu-devel

On Wed, 23 Sep 2015 12:37:08 +0200
Laszlo Ersek <lersek@redhat.com> wrote:

> On 09/23/15 10:04, Igor Mammedov wrote:
> > On Tue, 22 Sep 2015 17:16:24 -0300
> > Eduardo Habkost <ehabkost@redhat.com> 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.
> >>
> >> This series is based on Michael's PCI tree, plus the "Set
> >> broken_reserved_end on pc-*-2.4, not 2.5" patch I submitted
> >> earlier today.
> > I haven't seen this patch on list, perhaps it needs to be resubmitted?
> 
> http://lists.nongnu.org/archive/html/qemu-devel/2015-09/msg05778.html
If I'm not mistaken it's link to this series and not to
"Set broken_reserved_end on pc-*-2.4, not 2.5" patch.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 0/3] pc: Set hw_version on all machine classes
  2015-09-23 11:47     ` Igor Mammedov
@ 2015-09-23 12:31       ` Laszlo Ersek
  0 siblings, 0 replies; 14+ messages in thread
From: Laszlo Ersek @ 2015-09-23 12:31 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: Paolo Bonzini, Marcel Apfelbaum, Michael S. Tsirkin,
	Eduardo Habkost, qemu-devel

On 09/23/15 13:47, Igor Mammedov wrote:
> On Wed, 23 Sep 2015 12:37:08 +0200
> Laszlo Ersek <lersek@redhat.com> wrote:
> 
>> On 09/23/15 10:04, Igor Mammedov wrote:
>>> On Tue, 22 Sep 2015 17:16:24 -0300
>>> Eduardo Habkost <ehabkost@redhat.com> 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.
>>>>
>>>> This series is based on Michael's PCI tree, plus the "Set
>>>> broken_reserved_end on pc-*-2.4, not 2.5" patch I submitted
>>>> earlier today.
>>> I haven't seen this patch on list, perhaps it needs to be resubmitted?
>>
>> http://lists.nongnu.org/archive/html/qemu-devel/2015-09/msg05778.html
> If I'm not mistaken it's link to this series and not to
> "Set broken_reserved_end on pc-*-2.4, not 2.5" patch.

Ugh, right. Sorry. I misunderstood what you meant with "this".

Laszlo

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2015-09-23 12:31 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-22 20:16 [Qemu-devel] [PATCH 0/3] pc: Set hw_version on all machine classes Eduardo Habkost
2015-09-22 20:16 ` [Qemu-devel] [PATCH 1/3] " Eduardo Habkost
2015-09-22 20:26   ` Laszlo Ersek
2015-09-22 20:34     ` Eduardo Habkost
2015-09-22 20:16 ` [Qemu-devel] [PATCH 2/3] osdep: Rename qemu_{get, set}_version() to qemu_{, set_}hw_version() Eduardo Habkost
2015-09-22 20:21   ` John Snow
2015-09-22 20:16 ` [Qemu-devel] [PATCH 3/3] megasas: Use qemu_hw_version() instead of QEMU_VERSION Eduardo Habkost
2015-09-22 20:21   ` Hannes Reinecke
2015-09-22 20:33   ` Laszlo Ersek
2015-09-22 20:36     ` Eduardo Habkost
2015-09-23  8:04 ` [Qemu-devel] [PATCH 0/3] pc: Set hw_version on all machine classes Igor Mammedov
2015-09-23 10:37   ` Laszlo Ersek
2015-09-23 11:47     ` Igor Mammedov
2015-09-23 12:31       ` Laszlo Ersek

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.