qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code
@ 2016-01-23 16:02 Eduardo Habkost
  2016-01-23 16:02 ` [Qemu-devel] [PATCH v2 1/5] q35: Remove old machine versions Eduardo Habkost
                   ` (11 more replies)
  0 siblings, 12 replies; 39+ messages in thread
From: Eduardo Habkost @ 2016-01-23 16:02 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael S. Tsirkin, Laszlo Ersek, Marcel Apfelbaum,
	Markus Armbruster, Igor Mammedov, Paolo Bonzini, John Snow

This is another attempt to remove old q35 machine code. Now I am
also removing unused compat code to demonstrate the benefit of
throwing away the old code that nobody uses.

Eduardo Habkost (5):
  q35: Remove old machine versions
  machine: Remove no_tco field
  ich9: Remove enable_tco arguments from init functions
  q35: Remove unused q35-acpi-dsdt.aml file
  q35: No need to check gigabyte_align

 Makefile                  |   2 +-
 hw/acpi/ich9.c            |   8 +--
 hw/i386/pc_q35.c          | 176 +---------------------------------------------
 hw/isa/lpc_ich9.c         |   4 +-
 include/hw/acpi/ich9.h    |   1 -
 include/hw/boards.h       |   1 -
 include/hw/i386/ich9.h    |   2 +-
 pc-bios/q35-acpi-dsdt.aml | Bin 7344 -> 0 bytes
 8 files changed, 9 insertions(+), 185 deletions(-)
 delete mode 100644 pc-bios/q35-acpi-dsdt.aml

-- 
2.1.0

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

* [Qemu-devel] [PATCH v2 1/5] q35: Remove old machine versions
  2016-01-23 16:02 [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code Eduardo Habkost
@ 2016-01-23 16:02 ` Eduardo Habkost
  2016-01-23 16:02 ` [Qemu-devel] [PATCH v2 2/5] machine: Remove no_tco field Eduardo Habkost
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 39+ messages in thread
From: Eduardo Habkost @ 2016-01-23 16:02 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael S. Tsirkin, Laszlo Ersek, Marcel Apfelbaum,
	Markus Armbruster, Igor Mammedov, Paolo Bonzini, John Snow

Migration with q35 was not possible before commit
04329029a8c539eb5f75dcb6d8b016f0c53a031a, because q35
unconditionally creates an ich9-ahci device, that was marked as
unmigratable. So all q35 machine classes before pc-q35-2.4 were
not migratable, so there's no point in keeping compatibility code
for them.

Remove all old pc-q35 machine classes and keep only pc-q35-2.4
and newer.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc_q35.c | 165 -------------------------------------------------------
 1 file changed, 165 deletions(-)

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 6128b02..cc81601 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -268,62 +268,6 @@ static void pc_q35_init(MachineState *machine)
     }
 }
 
-/* Looking for a pc_compat_2_4() function? It doesn't exist.
- * pc_compat_*() functions that run on machine-init time and
- * change global QEMU state are deprecated. Please don't create
- * one, and implement any pc-*-2.4 (and newer) compat code in
- * HW_COMPAT_*, PC_COMPAT_*, or * pc_*_machine_options().
- */
-
-static void pc_compat_2_3(MachineState *machine)
-{
-    PCMachineState *pcms = PC_MACHINE(machine);
-    savevm_skip_section_footers();
-    if (kvm_enabled()) {
-        pcms->smm = ON_OFF_AUTO_OFF;
-    }
-    global_state_set_optional();
-    savevm_skip_configuration();
-}
-
-static void pc_compat_2_2(MachineState *machine)
-{
-    pc_compat_2_3(machine);
-    machine->suppress_vmdesc = true;
-}
-
-static void pc_compat_2_1(MachineState *machine)
-{
-    pc_compat_2_2(machine);
-    x86_cpu_change_kvm_default("svm", NULL);
-}
-
-static void pc_compat_2_0(MachineState *machine)
-{
-    pc_compat_2_1(machine);
-}
-
-static void pc_compat_1_7(MachineState *machine)
-{
-    pc_compat_2_0(machine);
-    x86_cpu_change_kvm_default("x2apic", NULL);
-}
-
-static void pc_compat_1_6(MachineState *machine)
-{
-    pc_compat_1_7(machine);
-}
-
-static void pc_compat_1_5(MachineState *machine)
-{
-    pc_compat_1_6(machine);
-}
-
-static void pc_compat_1_4(MachineState *machine)
-{
-    pc_compat_1_5(machine);
-}
-
 #define DEFINE_Q35_MACHINE(suffix, name, compatfn, optionfn) \
     static void pc_init_##suffix(MachineState *machine) \
     { \
@@ -380,112 +324,3 @@ static void pc_q35_2_4_machine_options(MachineClass *m)
 
 DEFINE_Q35_MACHINE(v2_4, "pc-q35-2.4", NULL,
                    pc_q35_2_4_machine_options);
-
-
-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;
-    SET_MACHINE_COMPAT(m, PC_COMPAT_2_3);
-}
-
-DEFINE_Q35_MACHINE(v2_3, "pc-q35-2.3", pc_compat_2_3,
-                   pc_q35_2_3_machine_options);
-
-
-static void pc_q35_2_2_machine_options(MachineClass *m)
-{
-    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
-    pc_q35_2_3_machine_options(m);
-    m->hw_version = "2.2.0";
-    SET_MACHINE_COMPAT(m, PC_COMPAT_2_2);
-    pcmc->rsdp_in_ram = false;
-}
-
-DEFINE_Q35_MACHINE(v2_2, "pc-q35-2.2", pc_compat_2_2,
-                   pc_q35_2_2_machine_options);
-
-
-static void pc_q35_2_1_machine_options(MachineClass *m)
-{
-    PCMachineClass *pcmc = PC_MACHINE_CLASS(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);
-    pcmc->smbios_uuid_encoded = false;
-    pcmc->enforce_aligned_dimm = false;
-}
-
-DEFINE_Q35_MACHINE(v2_1, "pc-q35-2.1", pc_compat_2_1,
-                   pc_q35_2_1_machine_options);
-
-
-static void pc_q35_2_0_machine_options(MachineClass *m)
-{
-    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
-    pc_q35_2_1_machine_options(m);
-    m->hw_version = "2.0.0";
-    SET_MACHINE_COMPAT(m, PC_COMPAT_2_0);
-    pcmc->has_reserved_memory = false;
-    pcmc->smbios_legacy_mode = true;
-    pcmc->acpi_data_size = 0x10000;
-}
-
-DEFINE_Q35_MACHINE(v2_0, "pc-q35-2.0", pc_compat_2_0,
-                   pc_q35_2_0_machine_options);
-
-
-static void pc_q35_1_7_machine_options(MachineClass *m)
-{
-    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
-    pc_q35_2_0_machine_options(m);
-    m->hw_version = "1.7.0";
-    m->default_machine_opts = NULL;
-    m->option_rom_has_mr = true;
-    SET_MACHINE_COMPAT(m, PC_COMPAT_1_7);
-    pcmc->smbios_defaults = false;
-    pcmc->gigabyte_align = false;
-}
-
-DEFINE_Q35_MACHINE(v1_7, "pc-q35-1.7", pc_compat_1_7,
-                   pc_q35_1_7_machine_options);
-
-
-static void pc_q35_1_6_machine_options(MachineClass *m)
-{
-    PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
-    pc_q35_machine_options(m);
-    m->hw_version = "1.6.0";
-    m->rom_file_has_mr = false;
-    SET_MACHINE_COMPAT(m, PC_COMPAT_1_6);
-    pcmc->has_acpi_build = false;
-}
-
-DEFINE_Q35_MACHINE(v1_6, "pc-q35-1.6", pc_compat_1_6,
-                   pc_q35_1_6_machine_options);
-
-
-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);
-}
-
-DEFINE_Q35_MACHINE(v1_5, "pc-q35-1.5", pc_compat_1_5,
-                   pc_q35_1_5_machine_options);
-
-
-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);
-}
-
-DEFINE_Q35_MACHINE(v1_4, "pc-q35-1.4", pc_compat_1_4,
-                   pc_q35_1_4_machine_options);
-- 
2.1.0

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

* [Qemu-devel] [PATCH v2 2/5] machine: Remove no_tco field
  2016-01-23 16:02 [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code Eduardo Habkost
  2016-01-23 16:02 ` [Qemu-devel] [PATCH v2 1/5] q35: Remove old machine versions Eduardo Habkost
@ 2016-01-23 16:02 ` Eduardo Habkost
  2016-01-23 16:02 ` [Qemu-devel] [PATCH v2 3/5] ich9: Remove enable_tco arguments from init functions Eduardo Habkost
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 39+ messages in thread
From: Eduardo Habkost @ 2016-01-23 16:02 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael S. Tsirkin, Laszlo Ersek, Marcel Apfelbaum,
	Markus Armbruster, Igor Mammedov, Paolo Bonzini, John Snow

The field is always set to zero, so it is not necessary anymore.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc_q35.c    | 3 +--
 include/hw/boards.h | 1 -
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index cc81601..8773efb 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -234,7 +234,7 @@ static void pc_q35_init(MachineState *machine)
                          (pcms->vmport != ON_OFF_AUTO_ON), 0xff0104);
 
     /* connect pm stuff to lpc */
-    ich9_lpc_pm_init(lpc, pc_machine_is_smm_enabled(pcms), !mc->no_tco);
+    ich9_lpc_pm_init(lpc, pc_machine_is_smm_enabled(pcms), true);
 
     /* ahci and SATA device, for q35 1 ahci controller is built-in */
     ahci = pci_create_simple_multifunction(host_bus,
@@ -289,7 +289,6 @@ static void pc_q35_machine_options(MachineClass *m)
     m->default_machine_opts = "firmware=bios-256k.bin";
     m->default_display = "std";
     m->no_floppy = 1;
-    m->no_tco = 0;
 }
 
 static void pc_q35_2_6_machine_options(MachineClass *m)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 0f30959..de3b3bd 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -84,7 +84,6 @@ struct MachineClass {
         no_cdrom:1,
         no_sdcard:1,
         has_dynamic_sysbus:1,
-        no_tco:1,
         pci_allow_0_address:1;
     int is_default;
     const char *default_machine_opts;
-- 
2.1.0

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

* [Qemu-devel] [PATCH v2 3/5] ich9: Remove enable_tco arguments from init functions
  2016-01-23 16:02 [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code Eduardo Habkost
  2016-01-23 16:02 ` [Qemu-devel] [PATCH v2 1/5] q35: Remove old machine versions Eduardo Habkost
  2016-01-23 16:02 ` [Qemu-devel] [PATCH v2 2/5] machine: Remove no_tco field Eduardo Habkost
@ 2016-01-23 16:02 ` Eduardo Habkost
  2016-01-23 16:02 ` [Qemu-devel] [PATCH v2 4/5] q35: Remove unused q35-acpi-dsdt.aml file Eduardo Habkost
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 39+ messages in thread
From: Eduardo Habkost @ 2016-01-23 16:02 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael S. Tsirkin, Laszlo Ersek, Marcel Apfelbaum,
	Markus Armbruster, Igor Mammedov, Paolo Bonzini, John Snow

The enable_tco arguments are always true, so they are not needed
anymore.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/acpi/ich9.c         | 8 +++-----
 hw/i386/pc_q35.c       | 2 +-
 hw/isa/lpc_ich9.c      | 4 ++--
 include/hw/acpi/ich9.h | 1 -
 include/hw/i386/ich9.h | 2 +-
 5 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index 1c7fcfa..a4bd98c 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -239,7 +239,7 @@ static void pm_powerdown_req(Notifier *n, void *opaque)
 }
 
 void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
-                  bool smm_enabled, bool enable_tco,
+                  bool smm_enabled,
                   qemu_irq sci_irq)
 {
     memory_region_init(&pm->io, OBJECT(lpc_pci), "ich9-pm", ICH9_PMIO_SIZE);
@@ -263,10 +263,8 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
 
     pm->smm_enabled = smm_enabled;
 
-    pm->enable_tco = enable_tco;
-    if (pm->enable_tco) {
-        acpi_pm_tco_init(&pm->tco_regs, &pm->io);
-    }
+    pm->enable_tco = true;
+    acpi_pm_tco_init(&pm->tco_regs, &pm->io);
 
     pm->irq = sci_irq;
     qemu_register_reset(pm_reset, pm);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 8773efb..b61f7a6 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -234,7 +234,7 @@ static void pc_q35_init(MachineState *machine)
                          (pcms->vmport != ON_OFF_AUTO_ON), 0xff0104);
 
     /* connect pm stuff to lpc */
-    ich9_lpc_pm_init(lpc, pc_machine_is_smm_enabled(pcms), true);
+    ich9_lpc_pm_init(lpc, pc_machine_is_smm_enabled(pcms));
 
     /* ahci and SATA device, for q35 1 ahci controller is built-in */
     ahci = pci_create_simple_multifunction(host_bus,
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index ed9907d..362d187 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -368,13 +368,13 @@ static void ich9_set_sci(void *opaque, int irq_num, int level)
     }
 }
 
-void ich9_lpc_pm_init(PCIDevice *lpc_pci, bool smm_enabled, bool enable_tco)
+void ich9_lpc_pm_init(PCIDevice *lpc_pci, bool smm_enabled)
 {
     ICH9LPCState *lpc = ICH9_LPC_DEVICE(lpc_pci);
     qemu_irq sci_irq;
 
     sci_irq = qemu_allocate_irq(ich9_set_sci, lpc, 0);
-    ich9_pm_init(lpc_pci, &lpc->pm, smm_enabled, enable_tco, sci_irq);
+    ich9_pm_init(lpc_pci, &lpc->pm, smm_enabled, sci_irq);
     ich9_lpc_reset(&lpc->d.qdev);
 }
 
diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h
index 345fd8d..63fa198 100644
--- a/include/hw/acpi/ich9.h
+++ b/include/hw/acpi/ich9.h
@@ -62,7 +62,6 @@ typedef struct ICH9LPCPMRegs {
 
 void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
                   bool smm_enabled,
-                  bool enable_tco,
                   qemu_irq sci_irq);
 
 void ich9_pm_iospace_update(ICH9LPCPMRegs *pm, uint32_t pm_io_base);
diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
index b9d2b04..b411434 100644
--- a/include/hw/i386/ich9.h
+++ b/include/hw/i386/ich9.h
@@ -17,7 +17,7 @@
 void ich9_lpc_set_irq(void *opaque, int irq_num, int level);
 int ich9_lpc_map_irq(PCIDevice *pci_dev, int intx);
 PCIINTxRoute ich9_route_intx_pin_to_irq(void *opaque, int pirq_pin);
-void ich9_lpc_pm_init(PCIDevice *pci_lpc, bool smm_enabled, bool enable_tco);
+void ich9_lpc_pm_init(PCIDevice *pci_lpc, bool smm_enabled);
 I2CBus *ich9_smb_init(PCIBus *bus, int devfn, uint32_t smb_io_base);
 
 void ich9_generate_smi(void);
-- 
2.1.0

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

* [Qemu-devel] [PATCH v2 4/5] q35: Remove unused q35-acpi-dsdt.aml file
  2016-01-23 16:02 [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code Eduardo Habkost
                   ` (2 preceding siblings ...)
  2016-01-23 16:02 ` [Qemu-devel] [PATCH v2 3/5] ich9: Remove enable_tco arguments from init functions Eduardo Habkost
@ 2016-01-23 16:02 ` Eduardo Habkost
  2016-01-23 16:02 ` [Qemu-devel] [PATCH v2 5/5] q35: No need to check gigabyte_align Eduardo Habkost
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 39+ messages in thread
From: Eduardo Habkost @ 2016-01-23 16:02 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael S. Tsirkin, Laszlo Ersek, Marcel Apfelbaum,
	Markus Armbruster, Igor Mammedov, Paolo Bonzini, John Snow

The file was used only by older machine-types, and it is not
needed anymore.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 Makefile                  |   2 +-
 hw/i386/pc_q35.c          |   4 ----
 pc-bios/q35-acpi-dsdt.aml | Bin 7344 -> 0 bytes
 3 files changed, 1 insertion(+), 5 deletions(-)
 delete mode 100644 pc-bios/q35-acpi-dsdt.aml

diff --git a/Makefile b/Makefile
index d0de2d4..9357574 100644
--- a/Makefile
+++ b/Makefile
@@ -390,7 +390,7 @@ bepo    cz
 ifdef INSTALL_BLOBS
 BLOBS=bios.bin bios-256k.bin sgabios.bin vgabios.bin vgabios-cirrus.bin \
 vgabios-stdvga.bin vgabios-vmware.bin vgabios-qxl.bin vgabios-virtio.bin \
-acpi-dsdt.aml q35-acpi-dsdt.aml \
+acpi-dsdt.aml \
 ppc_rom.bin openbios-sparc32 openbios-sparc64 openbios-ppc QEMU,tcx.bin QEMU,cgthree.bin \
 pxe-e1000.rom pxe-eepro100.rom pxe-ne2k_pci.rom \
 pxe-pcnet.rom pxe-rtl8139.rom pxe-virtio.rom \
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index b61f7a6..aed4432 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -116,10 +116,6 @@ static void pc_q35_init(MachineState *machine)
     }
 
     pc_cpus_init(pcms);
-    if (!pcmc->has_acpi_build) {
-        /* only machine types 1.7 & older need this */
-        pc_acpi_init("q35-acpi-dsdt.aml");
-    }
 
     kvmclock_create();
 
diff --git a/pc-bios/q35-acpi-dsdt.aml b/pc-bios/q35-acpi-dsdt.aml
deleted file mode 100644
index d71b3a328ced5ce2cb16d153cfa7cda0aca966a4..0000000000000000000000000000000000000000
GIT binary patch
literal 0
HcmV?d00001

literal 7344
zcmb7JOK%(38ND;4q|uO)h7v8y@)KILX!?jAj?xxrVRD9~Xj9UNl${hbz>%C3Dhw2a
z>i}_#0J2)tjutIqH9?njz;)Vx&@8+3u8VHkZI>wuYi0F(-^VN8JBol<i<x`woO8c9
zcOGYW6}6(xzg|$v885#Zhf0;-jnJc%tCXs*Z8oMxC$9wc>}}QFjmou-lXJ69`%#YD
z*p$8E52EtTn)BzK<ntuC|2T2Bx@!B2;Ir+m?r)zY+lojZY<GR18o~EEt#YTm8;7k;
zC#Syj^V}{wx%`(jEc(mW;;>%!g7rL8_iB0Id9SllkMgDGzUOs9X=S`lDQYUObF&^f
zUh8HYD=*w^HdHYh?X0)L?EYO9M(-^2sv`$Gg(*L1ul{rC#pMgvK7Z$a>8qdp<Cpt)
z{VWKJ*n8M7bqUZo)L5dS9@56YNZVfU^x#*{$2hXU?1kR+(*w<u?tl(<iA?>^^3qEs
z7W#LCwKG2=Lu@8&R$7Jfi2v&+W6I5Arj<IPjZ<?D^M`rIL%);HP}d6_KFlBEu_Ge>
z_wiY!<hWA4Iy%Y`6@RYcBaR^TucM>Cy_EJ>mmXkxn#)%UOaA=+_g~57@Gf>o^e6Cm
zOZ^X2vE+BcW=rkUiTk(X*3zOhlenUrJ8@%au^Z2~I&>I~N{0@1@7?Dn`jw-I)5{;k
z3%5F|PlNrDPRSpSTgz9aIUQGY^E+{4`D!;_pb@!2BXXnBp+mo;0Z5<plVH`8+oj^I
z#Fb5byEwQ>aC_k9U5PhW73T8!%bd!#9|eS`ZdKNzD{(XGTq!PhTAL-+rJkfqU0WX|
zevtI?pB^OYAjusa^;K{D(}OMbr>(sD_y30as_#$WW^A?!1%LeaqcmmDU#cw4SFn71
zt=oHV+^Us2e6OOsql!4ug-G?ev8WqM)Cd|QhHibh-g+gx3=eU=x#9W=Jv5Ctj9h=$
zciWGyP+NuTtEG%`uq$KNh&Llwxzz|PCRbd4kI1tiS5#Q4tT|yBn?{UHHNqxnoPRT;
z?@+4T-uq#5GsKz8lb6GjMrLRHGc~8Hx+-wz=!U(g?kRWpfoku)eWM&5(=(akC{O9}
zEU_?AiE^NNPrZlzoQuh7?|pZ5{pPW7Vk#~4{GmA&5b=5zj*j02BdRzfSB(rLy<uFb
zzIv8Du6W*`sX1R~zRsv@&3Tb|fr**BrryYM(Vt_Ej*DiGhoTAjhM@v~tmb@}`7q;o
zbGxjsWHHwZdUl2!c@OC`q8UDIn8VCrMtO4q4^yRK7E{Cc?(sOdNT-hD9=aIE!#G}C
z538z}?_lqAcL)W4xarX448s5LJ11^##*;UGvpbo~;$~KjwHp;BZ3peDn#@zb?*h~`
z$N;RU$+2wy=m_r=;SCU}KpdB2tOHd;Qeu4OB1(iiK%~@ESFsXQRsxa|XTw~)0fjq2
zq}0@mC_!Z<ASrQ1%*A6U+yNSRWNKEF`s4vkpF@_<Av{a3d=80HpFE)HbJ+4Z%t}oi
zwtWJcK1VE{BdpXuM{J*frcclE>9JBziD&x+G=1WQ1!sDuM=hVDwogFQr*HZ6EuX&a
z6VUXDm_kj=315|*<uhmd1T=l()oEPMyyY`*`vf$7j#)mDtH$*lvwZ@ZKF2Me<Cf2H
z+b5vubHegDVfmb}eFB<31IuS%`3!8IfTqua<+EV<EZ9B)O`nsN&q>SYr0o;X^f_hu
zoU(jQ***bHpGC`O(ehceeFB<3&saXsSU%6#J^@Xi)0WR^%jdN16VUXDuPx)gJ!|<q
zYx@K=ea=`uXDpvHwogFQ=Q&oE2G7blR+a}ZyK|xpUqMnv1MaM)oVAp*wi3`(p0||e
zE#-Mz31}+u3j$}L&-Q|)ykIK<P31*PdC^i{w3UFS@)9d`%3NZle)nAxrGCc)l2X5U
zE?YG(TQx7+H36ljhX5@Z0a|O-QJTZlK^uh!0SbV!0x|%)C<0V?{PJ_~?ZiZ#n66MY
zB&D{702MAPT~_K~p#T*sD=9z$^=lmg3Z%~f-#-SG0#vB1qyPmp0#vv<Kn*Gds8Cr+
z0Sc(AAV7u7N(87ur2rKwD=9z$wNC`7a9N1}HK-JzLS-cdD4_O<02MAP5ugT@0#vB1
zqyPofJ`te8WhDaCpi+Pem6a5rfZ8VlRJg1}fErW^P@%Gt0u)gDM1TsHl?YISN&zZV
zR#JchYM%&D;j$6|YEUUah000_P(bYy0V-TpB0vo)1*lM2NdXF|eIh`G%Sr^OL8Sl{
zDk~{K0kuyAsBl?{05zx-ph9IO1t_5Qi2xNYD-oaul>$_#tfT-1)IJfQ!eu1_)Syy;
z3YC=<pn%#Z0#vxHM1UGp3Q(c4k^&S^`$T{Wmz4-mgGvD^R8~@e0&1TKP~ox?0cubw
zK!wUm3Q$1p69Fn*Rw6(RDg~%eSxEs3sC^<ph096=s6nLw6)Gz!KmoN+1gLOXi2yaI
z6re(7B?Ty;_K5%$E-MkB29*L-sH~&_1=Kzfpu%M(0@R>VfC`nB6rh0GCjt~mM-l=Q
z$P7?GGe7~205y~dP(w)pYA7i{4J889P$EDLB?YLVqyROP2v9?b05y~ppoWqH)KDTo
z4J889P*Q*zN(xXzi2w!CR~G>age%6ANdXFo%b+1C_1q#rf%M!WK!Nn!Qh);LxupOF
zj8t*9g9uf${6jxh`}BkK*)Z)&?MFX-Gef^pY2ATVIh?q;Q9|NwlxY*vCh`=PYBnm=
zuF|GPn-#A(Ro}p3VsG@Jzq^AqnbE$VD_*T{gw0V9n)8KmKljP78s#;y2Wp@C{2^@X
z-G@8=grXKsqB-B*YmCr0jQ%a2$LbEP9eFtIR$<zo`ea;<W{_@JaZ`#`+^{e<A!{6T
z3OJimvCxU?E%h<A{ax;v;FYsJoe<-*omR8u?C%WDfyz_=pn5q~9m@*ex_a5DUgqj$
zoFA>*>FVX;H&-vW_g2Y!n5yo(uTefU%7<J&OqCB$DIX5XN9podz5CYgBcpu8<)c*j
z=#=u&pnN4=KKmNwD@OSWm#?JCS57Hk8I-T4%MZOq`KnRA%H^x6^3_wyR|n;5>GH#`
zQNCuBuW|WWs(kH~^0h(vm2~-$*C@YYlwaZUE2;7;r<7l5?^Uw&Dod3gJoWnBfv*Z$
zW#a`<yRSIR&+}lI^y8Zv2A>!whLJvdnDld)8V27RCx($edzkd|lo|#fA18*9K6{w-
zbCDVbUm_=lkv@Bv^!=S02A?G-hLJvdnDl*`8V27eCx($edzke7mKp{hEGLGMzQHg-
zZJbtDi*JSTYRSRBy25yUZAT^DLeBRTepS%lR(I#gmhURwWmGo-%BucWaW;&RS+7*R
zp#-be&ocP0fFH({>u!6_|2r1^oBVIGzxY?$kMaz#>0Aqn*N4#v75-7t>b#Htw{=zW
zMRn-2<T0JaVAw=$_56p4KcYU`cDA48(fvODr-7e@+ueuTj=IyEdwQ^Sr+bG_JyAXX
z;EuDlS^Y)gJo9+(XFf=Rb+1#4<rWw#g>ij;etubD5*qXKsu*Cmpl-{5EpSHMzbJ>7
jWOr3|Z^>>TeOu6vW%m==m1I|zT_`(zH&CsL?5gVjcvrSt

-- 
2.1.0

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

* [Qemu-devel] [PATCH v2 5/5] q35: No need to check gigabyte_align
  2016-01-23 16:02 [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code Eduardo Habkost
                   ` (3 preceding siblings ...)
  2016-01-23 16:02 ` [Qemu-devel] [PATCH v2 4/5] q35: Remove unused q35-acpi-dsdt.aml file Eduardo Habkost
@ 2016-01-23 16:02 ` Eduardo Habkost
  2016-01-25 11:27 ` [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code Laszlo Ersek
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 39+ messages in thread
From: Eduardo Habkost @ 2016-01-23 16:02 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael S. Tsirkin, Laszlo Ersek, Marcel Apfelbaum,
	Markus Armbruster, Igor Mammedov, Paolo Bonzini, John Snow

gigabyte_align is always true on q35, so we don't need the
!gigabyte_align compat code anymore.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc_q35.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index aed4432..20722f2 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -81,11 +81,9 @@ static void pc_q35_init(MachineState *machine)
      * If it doesn't, we need to split it in chunks below and above 4G.
      * In any case, try to make sure that guest addresses aligned at
      * 1G boundaries get mapped to host addresses aligned at 1G boundaries.
-     * For old machine types, use whatever split we used historically to avoid
-     * breaking migration.
      */
     if (machine->ram_size >= 0xb0000000) {
-        lowmem = pcmc->gigabyte_align ? 0x80000000 : 0xb0000000;
+        lowmem = 0x80000000;
     } else {
         lowmem = 0xb0000000;
     }
-- 
2.1.0

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

* Re: [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code
  2016-01-23 16:02 [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code Eduardo Habkost
                   ` (4 preceding siblings ...)
  2016-01-23 16:02 ` [Qemu-devel] [PATCH v2 5/5] q35: No need to check gigabyte_align Eduardo Habkost
@ 2016-01-25 11:27 ` Laszlo Ersek
  2016-01-26 10:15   ` Igor Mammedov
  2016-01-26 10:17 ` Igor Mammedov
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 39+ messages in thread
From: Laszlo Ersek @ 2016-01-25 11:27 UTC (permalink / raw)
  To: Eduardo Habkost, qemu-devel
  Cc: Marcel Apfelbaum, Michael S. Tsirkin, Markus Armbruster,
	Igor Mammedov, Paolo Bonzini, John Snow

On 01/23/16 17:02, Eduardo Habkost wrote:
> This is another attempt to remove old q35 machine code. Now I am
> also removing unused compat code to demonstrate the benefit of
> throwing away the old code that nobody uses.
> 
> Eduardo Habkost (5):
>   q35: Remove old machine versions
>   machine: Remove no_tco field
>   ich9: Remove enable_tco arguments from init functions
>   q35: Remove unused q35-acpi-dsdt.aml file
>   q35: No need to check gigabyte_align
> 
>  Makefile                  |   2 +-
>  hw/acpi/ich9.c            |   8 +--
>  hw/i386/pc_q35.c          | 176 +---------------------------------------------
>  hw/isa/lpc_ich9.c         |   4 +-
>  include/hw/acpi/ich9.h    |   1 -
>  include/hw/boards.h       |   1 -
>  include/hw/i386/ich9.h    |   2 +-
>  pc-bios/q35-acpi-dsdt.aml | Bin 7344 -> 0 bytes
>  8 files changed, 9 insertions(+), 185 deletions(-)
>  delete mode 100644 pc-bios/q35-acpi-dsdt.aml
> 

I read / skimmed the earlier discussion:

http://thread.gmane.org/gmane.comp.emulators.qemu/356340
http://thread.gmane.org/gmane.comp.emulators.qemu/382574

For patches 1, 2, 3, 5:

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

For patch 4:

How about removing the following two files in addition:
- hw/i386/q35-acpi-dsdt.dsl
- hw/i386/q35-acpi-dsdt.hex.generated

and updating the references in "hw/i386/Makefile.objs"?

Thanks
Laszlo

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

* Re: [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code
  2016-01-25 11:27 ` [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code Laszlo Ersek
@ 2016-01-26 10:15   ` Igor Mammedov
  2016-01-26 14:49     ` Laszlo Ersek
  0 siblings, 1 reply; 39+ messages in thread
From: Igor Mammedov @ 2016-01-26 10:15 UTC (permalink / raw)
  To: Laszlo Ersek
  Cc: Eduardo Habkost, Michael S. Tsirkin, Marcel Apfelbaum,
	Markus Armbruster, qemu-devel, Paolo Bonzini, John Snow

On Mon, 25 Jan 2016 12:27:47 +0100
Laszlo Ersek <lersek@redhat.com> wrote:

> On 01/23/16 17:02, Eduardo Habkost wrote:
> > This is another attempt to remove old q35 machine code. Now I am
> > also removing unused compat code to demonstrate the benefit of
> > throwing away the old code that nobody uses.
> > 
> > Eduardo Habkost (5):
> >   q35: Remove old machine versions
> >   machine: Remove no_tco field
> >   ich9: Remove enable_tco arguments from init functions
> >   q35: Remove unused q35-acpi-dsdt.aml file
> >   q35: No need to check gigabyte_align
> > 
> >  Makefile                  |   2 +-
> >  hw/acpi/ich9.c            |   8 +--
> >  hw/i386/pc_q35.c          | 176 +---------------------------------------------
> >  hw/isa/lpc_ich9.c         |   4 +-
> >  include/hw/acpi/ich9.h    |   1 -
> >  include/hw/boards.h       |   1 -
> >  include/hw/i386/ich9.h    |   2 +-
> >  pc-bios/q35-acpi-dsdt.aml | Bin 7344 -> 0 bytes
> >  8 files changed, 9 insertions(+), 185 deletions(-)
> >  delete mode 100644 pc-bios/q35-acpi-dsdt.aml
> >   
> 
> I read / skimmed the earlier discussion:
> 
> http://thread.gmane.org/gmane.comp.emulators.qemu/356340
> http://thread.gmane.org/gmane.comp.emulators.qemu/382574
> 
> For patches 1, 2, 3, 5:
> 
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> 
> For patch 4:
> 
> How about removing the following two files in addition:
> - hw/i386/q35-acpi-dsdt.dsl
> - hw/i386/q35-acpi-dsdt.hex.generated
above files don't exist in current master as there where removed
by DSDT->AML conversion series. And well, they weren't related
to legacy code removed here anyway.

> 
> and updating the references in "hw/i386/Makefile.objs"?
> 
> Thanks
> Laszlo

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

* Re: [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code
  2016-01-23 16:02 [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code Eduardo Habkost
                   ` (5 preceding siblings ...)
  2016-01-25 11:27 ` [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code Laszlo Ersek
@ 2016-01-26 10:17 ` Igor Mammedov
  2016-01-26 14:59 ` [Qemu-devel] [PATCH 6/5] pc: Remove unused pc_acpi_init() function Eduardo Habkost
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 39+ messages in thread
From: Igor Mammedov @ 2016-01-26 10:17 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Marcel Apfelbaum, John Snow, Michael S. Tsirkin,
	Markus Armbruster, qemu-devel, Paolo Bonzini, Laszlo Ersek

On Sat, 23 Jan 2016 14:02:08 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:

> This is another attempt to remove old q35 machine code. Now I am
> also removing unused compat code to demonstrate the benefit of
> throwing away the old code that nobody uses.
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> 
> Eduardo Habkost (5):
>   q35: Remove old machine versions
>   machine: Remove no_tco field
>   ich9: Remove enable_tco arguments from init functions
>   q35: Remove unused q35-acpi-dsdt.aml file
>   q35: No need to check gigabyte_align
> 
>  Makefile                  |   2 +-
>  hw/acpi/ich9.c            |   8 +--
>  hw/i386/pc_q35.c          | 176 +---------------------------------------------
>  hw/isa/lpc_ich9.c         |   4 +-
>  include/hw/acpi/ich9.h    |   1 -
>  include/hw/boards.h       |   1 -
>  include/hw/i386/ich9.h    |   2 +-
>  pc-bios/q35-acpi-dsdt.aml | Bin 7344 -> 0 bytes
>  8 files changed, 9 insertions(+), 185 deletions(-)
>  delete mode 100644 pc-bios/q35-acpi-dsdt.aml
> 

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

* Re: [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code
  2016-01-26 10:15   ` Igor Mammedov
@ 2016-01-26 14:49     ` Laszlo Ersek
  0 siblings, 0 replies; 39+ messages in thread
From: Laszlo Ersek @ 2016-01-26 14:49 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: Eduardo Habkost, Michael S. Tsirkin, Marcel Apfelbaum,
	Markus Armbruster, qemu-devel, Paolo Bonzini, John Snow

On 01/26/16 11:15, Igor Mammedov wrote:
> On Mon, 25 Jan 2016 12:27:47 +0100
> Laszlo Ersek <lersek@redhat.com> wrote:
> 
>> On 01/23/16 17:02, Eduardo Habkost wrote:
>>> This is another attempt to remove old q35 machine code. Now I am
>>> also removing unused compat code to demonstrate the benefit of
>>> throwing away the old code that nobody uses.
>>>
>>> Eduardo Habkost (5):
>>>   q35: Remove old machine versions
>>>   machine: Remove no_tco field
>>>   ich9: Remove enable_tco arguments from init functions
>>>   q35: Remove unused q35-acpi-dsdt.aml file
>>>   q35: No need to check gigabyte_align
>>>
>>>  Makefile                  |   2 +-
>>>  hw/acpi/ich9.c            |   8 +--
>>>  hw/i386/pc_q35.c          | 176 +---------------------------------------------
>>>  hw/isa/lpc_ich9.c         |   4 +-
>>>  include/hw/acpi/ich9.h    |   1 -
>>>  include/hw/boards.h       |   1 -
>>>  include/hw/i386/ich9.h    |   2 +-
>>>  pc-bios/q35-acpi-dsdt.aml | Bin 7344 -> 0 bytes
>>>  8 files changed, 9 insertions(+), 185 deletions(-)
>>>  delete mode 100644 pc-bios/q35-acpi-dsdt.aml
>>>   
>>
>> I read / skimmed the earlier discussion:
>>
>> http://thread.gmane.org/gmane.comp.emulators.qemu/356340
>> http://thread.gmane.org/gmane.comp.emulators.qemu/382574
>>
>> For patches 1, 2, 3, 5:
>>
>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>>
>> For patch 4:
>>
>> How about removing the following two files in addition:
>> - hw/i386/q35-acpi-dsdt.dsl
>> - hw/i386/q35-acpi-dsdt.hex.generated
> above files don't exist in current master as there where removed
> by DSDT->AML conversion series.

Sigh. That's right. For some reason I looked at a wrong branch in my tree.

For patch 4 too:

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

> And well, they weren't related
> to legacy code removed here anyway.

Sigh again. You are right. "pc-bios/q35-acpi-dsdt.aml" came from
SeaBIOS's "src/fw/q35-acpi-dsdt.dsl", apparently, *not* from QEMU's
"hw/i386/q35-acpi-dsdt.dsl"

It's just great that up to 9fc6502606, QEMU and SeaBIOS had had an
independent, but identically named (and more or less same purpose) file!

>> and updating the references in "hw/i386/Makefile.objs"?
>>
>> Thanks
>> Laszlo
> 

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

* [Qemu-devel] [PATCH 6/5] pc: Remove unused pc_acpi_init() function
  2016-01-23 16:02 [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code Eduardo Habkost
                   ` (6 preceding siblings ...)
  2016-01-26 10:17 ` Igor Mammedov
@ 2016-01-26 14:59 ` Eduardo Habkost
  2016-01-26 17:27   ` Laszlo Ersek
  2016-01-26 14:59 ` [Qemu-devel] [PATCH 7/5] acpi: Remove unused acpi_table_add_builtin() function Eduardo Habkost
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 39+ messages in thread
From: Eduardo Habkost @ 2016-01-26 14:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marcel Apfelbaum, John Snow, Michael S. Tsirkin,
	Markus Armbruster, Paolo Bonzini, Igor Mammedov, Laszlo Ersek

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Additional code we can remove after removing the old q35
machines.
---
 hw/i386/pc.c         | 28 ----------------------------
 include/hw/i386/pc.h |  1 -
 2 files changed, 29 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 78cf8fa..f68c0c6 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1233,34 +1233,6 @@ void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory,
                                         pci_address_space, -1);
 }
 
-void pc_acpi_init(const char *default_dsdt)
-{
-    char *filename;
-
-    if (acpi_tables != NULL) {
-        /* manually set via -acpitable, leave it alone */
-        return;
-    }
-
-    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, default_dsdt);
-    if (filename == NULL) {
-        fprintf(stderr, "WARNING: failed to find %s\n", default_dsdt);
-    } else {
-        QemuOpts *opts = qemu_opts_create(qemu_find_opts("acpi"), NULL, 0,
-                                          &error_abort);
-        Error *err = NULL;
-
-        qemu_opt_set(opts, "file", filename, &error_abort);
-
-        acpi_table_add_builtin(opts, &err);
-        if (err) {
-            error_reportf_err(err, "WARNING: failed to load %s: ",
-                              filename);
-        }
-        g_free(filename);
-    }
-}
-
 FWCfgState *xen_load_linux(PCMachineState *pcms,
                            PcGuestInfo *guest_info)
 {
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 65e8f24..3bc9e62 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -230,7 +230,6 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
 
 void pc_cpus_init(PCMachineState *pcms);
 void pc_hot_add_cpu(const int64_t id, Error **errp);
-void pc_acpi_init(const char *default_dsdt);
 
 PcGuestInfo *pc_guest_info_init(PCMachineState *pcms);
 
-- 
2.1.0

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

* [Qemu-devel] [PATCH 7/5] acpi: Remove unused acpi_table_add_builtin() function
  2016-01-23 16:02 [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code Eduardo Habkost
                   ` (7 preceding siblings ...)
  2016-01-26 14:59 ` [Qemu-devel] [PATCH 6/5] pc: Remove unused pc_acpi_init() function Eduardo Habkost
@ 2016-01-26 14:59 ` Eduardo Habkost
  2016-01-26 17:27   ` Laszlo Ersek
  2016-01-27 17:08   ` Igor Mammedov
  2016-02-04 16:01 ` [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code Michael S. Tsirkin
                   ` (2 subsequent siblings)
  11 siblings, 2 replies; 39+ messages in thread
From: Eduardo Habkost @ 2016-01-26 14:59 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marcel Apfelbaum, John Snow, Michael S. Tsirkin,
	Markus Armbruster, Paolo Bonzini, Igor Mammedov, Laszlo Ersek

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Additional code removal.
---
 hw/acpi/core.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/hw/acpi/core.c b/hw/acpi/core.c
index 21e113d..c2a5383 100644
--- a/hw/acpi/core.c
+++ b/hw/acpi/core.c
@@ -309,14 +309,6 @@ out:
     error_propagate(errp, err);
 }
 
-static bool acpi_table_builtin = false;
-
-void acpi_table_add_builtin(const QemuOpts *opts, Error **errp)
-{
-    acpi_table_builtin = true;
-    acpi_table_add(opts, errp);
-}
-
 unsigned acpi_table_len(void *current)
 {
     struct acpi_table_header *hdr = current - sizeof(hdr->_length);
@@ -332,7 +324,7 @@ void *acpi_table_hdr(void *h)
 
 uint8_t *acpi_table_first(void)
 {
-    if (acpi_table_builtin || !acpi_tables) {
+    if (!acpi_tables) {
         return NULL;
     }
     return acpi_table_hdr(acpi_tables + ACPI_TABLE_PFX_SIZE);
-- 
2.1.0

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

* Re: [Qemu-devel] [PATCH 6/5] pc: Remove unused pc_acpi_init() function
  2016-01-26 14:59 ` [Qemu-devel] [PATCH 6/5] pc: Remove unused pc_acpi_init() function Eduardo Habkost
@ 2016-01-26 17:27   ` Laszlo Ersek
  0 siblings, 0 replies; 39+ messages in thread
From: Laszlo Ersek @ 2016-01-26 17:27 UTC (permalink / raw)
  To: Eduardo Habkost, qemu-devel
  Cc: Marcel Apfelbaum, Michael S. Tsirkin, Markus Armbruster,
	Paolo Bonzini, Igor Mammedov, John Snow

On 01/26/16 15:59, Eduardo Habkost wrote:
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Additional code we can remove after removing the old q35
> machines.
> ---
>  hw/i386/pc.c         | 28 ----------------------------
>  include/hw/i386/pc.h |  1 -
>  2 files changed, 29 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 78cf8fa..f68c0c6 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1233,34 +1233,6 @@ void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory,
>                                          pci_address_space, -1);
>  }
>  
> -void pc_acpi_init(const char *default_dsdt)
> -{
> -    char *filename;
> -
> -    if (acpi_tables != NULL) {
> -        /* manually set via -acpitable, leave it alone */
> -        return;
> -    }
> -
> -    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, default_dsdt);
> -    if (filename == NULL) {
> -        fprintf(stderr, "WARNING: failed to find %s\n", default_dsdt);
> -    } else {
> -        QemuOpts *opts = qemu_opts_create(qemu_find_opts("acpi"), NULL, 0,
> -                                          &error_abort);
> -        Error *err = NULL;
> -
> -        qemu_opt_set(opts, "file", filename, &error_abort);
> -
> -        acpi_table_add_builtin(opts, &err);
> -        if (err) {
> -            error_reportf_err(err, "WARNING: failed to load %s: ",
> -                              filename);
> -        }
> -        g_free(filename);
> -    }
> -}
> -
>  FWCfgState *xen_load_linux(PCMachineState *pcms,
>                             PcGuestInfo *guest_info)
>  {
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 65e8f24..3bc9e62 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -230,7 +230,6 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
>  
>  void pc_cpus_init(PCMachineState *pcms);
>  void pc_hot_add_cpu(const int64_t id, Error **errp);
> -void pc_acpi_init(const char *default_dsdt);
>  
>  PcGuestInfo *pc_guest_info_init(PCMachineState *pcms);
>  
> 

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

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

* Re: [Qemu-devel] [PATCH 7/5] acpi: Remove unused acpi_table_add_builtin() function
  2016-01-26 14:59 ` [Qemu-devel] [PATCH 7/5] acpi: Remove unused acpi_table_add_builtin() function Eduardo Habkost
@ 2016-01-26 17:27   ` Laszlo Ersek
  2016-01-27 17:08   ` Igor Mammedov
  1 sibling, 0 replies; 39+ messages in thread
From: Laszlo Ersek @ 2016-01-26 17:27 UTC (permalink / raw)
  To: Eduardo Habkost, qemu-devel
  Cc: Marcel Apfelbaum, Michael S. Tsirkin, Markus Armbruster,
	Paolo Bonzini, Igor Mammedov, John Snow

On 01/26/16 15:59, Eduardo Habkost wrote:
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Additional code removal.
> ---
>  hw/acpi/core.c | 10 +---------
>  1 file changed, 1 insertion(+), 9 deletions(-)
> 
> diff --git a/hw/acpi/core.c b/hw/acpi/core.c
> index 21e113d..c2a5383 100644
> --- a/hw/acpi/core.c
> +++ b/hw/acpi/core.c
> @@ -309,14 +309,6 @@ out:
>      error_propagate(errp, err);
>  }
>  
> -static bool acpi_table_builtin = false;
> -
> -void acpi_table_add_builtin(const QemuOpts *opts, Error **errp)
> -{
> -    acpi_table_builtin = true;
> -    acpi_table_add(opts, errp);
> -}
> -
>  unsigned acpi_table_len(void *current)
>  {
>      struct acpi_table_header *hdr = current - sizeof(hdr->_length);
> @@ -332,7 +324,7 @@ void *acpi_table_hdr(void *h)
>  
>  uint8_t *acpi_table_first(void)
>  {
> -    if (acpi_table_builtin || !acpi_tables) {
> +    if (!acpi_tables) {
>          return NULL;
>      }
>      return acpi_table_hdr(acpi_tables + ACPI_TABLE_PFX_SIZE);
> 

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

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

* Re: [Qemu-devel] [PATCH 7/5] acpi: Remove unused acpi_table_add_builtin() function
  2016-01-26 14:59 ` [Qemu-devel] [PATCH 7/5] acpi: Remove unused acpi_table_add_builtin() function Eduardo Habkost
  2016-01-26 17:27   ` Laszlo Ersek
@ 2016-01-27 17:08   ` Igor Mammedov
  1 sibling, 0 replies; 39+ messages in thread
From: Igor Mammedov @ 2016-01-27 17:08 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Michael S. Tsirkin, Laszlo Ersek, Marcel Apfelbaum,
	Markus Armbruster, qemu-devel, Paolo Bonzini, John Snow

On Tue, 26 Jan 2016 12:59:46 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:

> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
> Additional code removal.
> ---
>  hw/acpi/core.c | 10 +---------
>  1 file changed, 1 insertion(+), 9 deletions(-)
> 
> diff --git a/hw/acpi/core.c b/hw/acpi/core.c
> index 21e113d..c2a5383 100644
> --- a/hw/acpi/core.c
> +++ b/hw/acpi/core.c
> @@ -309,14 +309,6 @@ out:
>      error_propagate(errp, err);
>  }
>  
> -static bool acpi_table_builtin = false;
> -
> -void acpi_table_add_builtin(const QemuOpts *opts, Error **errp)
> -{
> -    acpi_table_builtin = true;
> -    acpi_table_add(opts, errp);
> -}
> -
>  unsigned acpi_table_len(void *current)
>  {
>      struct acpi_table_header *hdr = current - sizeof(hdr->_length);
> @@ -332,7 +324,7 @@ void *acpi_table_hdr(void *h)
>  
>  uint8_t *acpi_table_first(void)
>  {
> -    if (acpi_table_builtin || !acpi_tables) {
> +    if (!acpi_tables) {
>          return NULL;
>      }
>      return acpi_table_hdr(acpi_tables + ACPI_TABLE_PFX_SIZE);

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

* Re: [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code
  2016-01-23 16:02 [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code Eduardo Habkost
                   ` (8 preceding siblings ...)
  2016-01-26 14:59 ` [Qemu-devel] [PATCH 7/5] acpi: Remove unused acpi_table_add_builtin() function Eduardo Habkost
@ 2016-02-04 16:01 ` Michael S. Tsirkin
  2016-02-04 17:16   ` Eduardo Habkost
  2016-02-18 12:39 ` Markus Armbruster
  2016-02-23 13:00 ` Michael S. Tsirkin
  11 siblings, 1 reply; 39+ messages in thread
From: Michael S. Tsirkin @ 2016-02-04 16:01 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Marcel Apfelbaum, Laszlo Ersek, qemu-devel, Markus Armbruster,
	Igor Mammedov, Paolo Bonzini, John Snow

On Sat, Jan 23, 2016 at 02:02:08PM -0200, Eduardo Habkost wrote:
> This is another attempt to remove old q35 machine code. Now I am
> also removing unused compat code to demonstrate the benefit of
> throwing away the old code that nobody uses.

The same thing I said applies - we don't know that nobody uses old q35
machine types.
We do know we don't need to migrate to/from them,
so we can drop compat code.
But please add aliases so people can still start these machines.


> 
> Eduardo Habkost (5):
>   q35: Remove old machine versions
>   machine: Remove no_tco field
>   ich9: Remove enable_tco arguments from init functions
>   q35: Remove unused q35-acpi-dsdt.aml file
>   q35: No need to check gigabyte_align
> 
>  Makefile                  |   2 +-
>  hw/acpi/ich9.c            |   8 +--
>  hw/i386/pc_q35.c          | 176 +---------------------------------------------
>  hw/isa/lpc_ich9.c         |   4 +-
>  include/hw/acpi/ich9.h    |   1 -
>  include/hw/boards.h       |   1 -
>  include/hw/i386/ich9.h    |   2 +-
>  pc-bios/q35-acpi-dsdt.aml | Bin 7344 -> 0 bytes
>  8 files changed, 9 insertions(+), 185 deletions(-)
>  delete mode 100644 pc-bios/q35-acpi-dsdt.aml
> 
> -- 
> 2.1.0

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

* Re: [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code
  2016-02-04 16:01 ` [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code Michael S. Tsirkin
@ 2016-02-04 17:16   ` Eduardo Habkost
  2016-02-04 18:02     ` Michael S. Tsirkin
  0 siblings, 1 reply; 39+ messages in thread
From: Eduardo Habkost @ 2016-02-04 17:16 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Marcel Apfelbaum, Laszlo Ersek, qemu-devel, Markus Armbruster,
	Igor Mammedov, Paolo Bonzini, John Snow

On Thu, Feb 04, 2016 at 06:01:50PM +0200, Michael S. Tsirkin wrote:
> On Sat, Jan 23, 2016 at 02:02:08PM -0200, Eduardo Habkost wrote:
> > This is another attempt to remove old q35 machine code. Now I am
> > also removing unused compat code to demonstrate the benefit of
> > throwing away the old code that nobody uses.
> 
> The same thing I said applies - we don't know that nobody uses old q35
> machine types.
> We do know we don't need to migrate to/from them,
> so we can drop compat code.
> But please add aliases so people can still start these machines.

If people use them, they can easily update their configurations.

I will copy and paste the reply Markus sent 4 months ago below.

On Mon, Sep 14, 2015 at 09:18:47AM +0200, Markus Armbruster wrote:
> We've been through this before, but we can go through it once more.
> Choices:
> 
> A. Remove old machine type
> 
>    A guest using it can't be started.  Easy to understand on the host.
>    An error message advising to switch to a newer machine type would be
>    a nice touch.
> 
>    This is a clean break in backward compatibility.  To be mentioned in
>    release notes, obviously.
> 
> B. Change old machine type in a guest-visible way
> 
>    Depending on the nature of the change and the guest, a guest using it
>    either doesn't notice, copes with it successfully, or fails in
>    guest-specific ways.  If the latter, the failure can be "guest
>    hangs", which is much harder to figure out than A.
> 
>    Unless we can *demonstrate* that nothing bad happens for all the
>    guests people actually use with the old machine types, this is a
>    different kind of backward compatibility break.
> 
>    Demonstrating this is feels infeasible to me, but you're welcome to
>    try.
> 
> I could call the difference between the two a tradeoff, but since we've
> been through this before, I'll be more blunt: choosing B robs Peter (the
> guy with guests where badness happens) to pay Paul (the guy with guests
> that cope).  Paul is saved the inconvenience of having to read release
> notes or his logs, and change machine types.  Peter pays for that with
> figuring out WTF his guests are doing now.
> 
> As a user, I'd pick a clean break in backward compatibility over a hack
> that preserves effective compatibility when it works, but breaks it
> uncleanly when it doesn't.
> 
> As a developer, I'm insisting on it.
> 
> So, if you want B, the onus is on *you* to show us why nothing bad will
> happen.
> 

-- 
Eduardo

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

* Re: [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code
  2016-02-04 17:16   ` Eduardo Habkost
@ 2016-02-04 18:02     ` Michael S. Tsirkin
  2016-02-04 19:09       ` Eduardo Habkost
  2016-02-05  8:55       ` Markus Armbruster
  0 siblings, 2 replies; 39+ messages in thread
From: Michael S. Tsirkin @ 2016-02-04 18:02 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Marcel Apfelbaum, Laszlo Ersek, qemu-devel, Markus Armbruster,
	Igor Mammedov, Paolo Bonzini, John Snow

On Thu, Feb 04, 2016 at 03:16:17PM -0200, Eduardo Habkost wrote:
> On Thu, Feb 04, 2016 at 06:01:50PM +0200, Michael S. Tsirkin wrote:
> > On Sat, Jan 23, 2016 at 02:02:08PM -0200, Eduardo Habkost wrote:
> > > This is another attempt to remove old q35 machine code. Now I am
> > > also removing unused compat code to demonstrate the benefit of
> > > throwing away the old code that nobody uses.
> > 
> > The same thing I said applies - we don't know that nobody uses old q35
> > machine types.
> > We do know we don't need to migrate to/from them,
> > so we can drop compat code.
> > But please add aliases so people can still start these machines.
> 
> If people use them, they can easily update their configurations.
> I will copy and paste the reply Markus sent 4 months ago below.
> 
> On Mon, Sep 14, 2015 at 09:18:47AM +0200, Markus Armbruster wrote:
> > We've been through this before, but we can go through it once more.
> > Choices:
> > 
> > A. Remove old machine type
> > 
> >    A guest using it can't be started.  Easy to understand on the host.
> >    An error message advising to switch to a newer machine type would be
> >    a nice touch.
> > 
> >    This is a clean break in backward compatibility.  To be mentioned in
> >    release notes, obviously.
> > 
> > B. Change old machine type in a guest-visible way
> > 
> >    Depending on the nature of the change and the guest, a guest using it
> >    either doesn't notice, copes with it successfully, or fails in
> >    guest-specific ways.  If the latter, the failure can be "guest
> >    hangs", which is much harder to figure out than A.
> > 
> >    Unless we can *demonstrate* that nothing bad happens for all the
> >    guests people actually use with the old machine types, this is a
> >    different kind of backward compatibility break.
> > 
> >    Demonstrating this is feels infeasible to me, but you're welcome to
> >    try.
> > 
> > I could call the difference between the two a tradeoff, but since we've
> > been through this before, I'll be more blunt: choosing B robs Peter (the
> > guy with guests where badness happens) to pay Paul (the guy with guests
> > that cope).  Paul is saved the inconvenience of having to read release
> > notes or his logs, and change machine types.  Peter pays for that with
> > figuring out WTF his guests are doing now.
> > 
> > As a user, I'd pick a clean break in backward compatibility over a hack
> > that preserves effective compatibility when it works, but breaks it
> > uncleanly when it doesn't.
> > 
> > As a developer, I'm insisting on it.
> > 
> > So, if you want B, the onus is on *you* to show us why nothing bad will
> > happen.
> > 

I agree with the conclusion for option B.  But I think the correct
solution is not A, it is to analyse changes, maybe even test, and show
that nothing bad can happen.

Because A suffers from exactly the same problem if people just blindly
switch to a new machine type.

> -- 
> Eduardo

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

* Re: [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code
  2016-02-04 18:02     ` Michael S. Tsirkin
@ 2016-02-04 19:09       ` Eduardo Habkost
  2016-02-04 22:14         ` Michael S. Tsirkin
  2016-02-05  8:55       ` Markus Armbruster
  1 sibling, 1 reply; 39+ messages in thread
From: Eduardo Habkost @ 2016-02-04 19:09 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Marcel Apfelbaum, Laszlo Ersek, qemu-devel, Markus Armbruster,
	Igor Mammedov, Paolo Bonzini, John Snow

On Thu, Feb 04, 2016 at 08:02:30PM +0200, Michael S. Tsirkin wrote:
> On Thu, Feb 04, 2016 at 03:16:17PM -0200, Eduardo Habkost wrote:
> > On Thu, Feb 04, 2016 at 06:01:50PM +0200, Michael S. Tsirkin wrote:
> > > On Sat, Jan 23, 2016 at 02:02:08PM -0200, Eduardo Habkost wrote:
> > > > This is another attempt to remove old q35 machine code. Now I am
> > > > also removing unused compat code to demonstrate the benefit of
> > > > throwing away the old code that nobody uses.
> > > 
> > > The same thing I said applies - we don't know that nobody uses old q35
> > > machine types.
> > > We do know we don't need to migrate to/from them,
> > > so we can drop compat code.
> > > But please add aliases so people can still start these machines.
> > 
> > If people use them, they can easily update their configurations.
> > I will copy and paste the reply Markus sent 4 months ago below.
> > 
> > On Mon, Sep 14, 2015 at 09:18:47AM +0200, Markus Armbruster wrote:
> > > We've been through this before, but we can go through it once more.
> > > Choices:
> > > 
> > > A. Remove old machine type
> > > 
> > >    A guest using it can't be started.  Easy to understand on the host.
> > >    An error message advising to switch to a newer machine type would be
> > >    a nice touch.
> > > 
> > >    This is a clean break in backward compatibility.  To be mentioned in
> > >    release notes, obviously.
> > > 
> > > B. Change old machine type in a guest-visible way
> > > 
> > >    Depending on the nature of the change and the guest, a guest using it
> > >    either doesn't notice, copes with it successfully, or fails in
> > >    guest-specific ways.  If the latter, the failure can be "guest
> > >    hangs", which is much harder to figure out than A.
> > > 
> > >    Unless we can *demonstrate* that nothing bad happens for all the
> > >    guests people actually use with the old machine types, this is a
> > >    different kind of backward compatibility break.
> > > 
> > >    Demonstrating this is feels infeasible to me, but you're welcome to
> > >    try.
> > > 
> > > I could call the difference between the two a tradeoff, but since we've
> > > been through this before, I'll be more blunt: choosing B robs Peter (the
> > > guy with guests where badness happens) to pay Paul (the guy with guests
> > > that cope).  Paul is saved the inconvenience of having to read release
> > > notes or his logs, and change machine types.  Peter pays for that with
> > > figuring out WTF his guests are doing now.
> > > 
> > > As a user, I'd pick a clean break in backward compatibility over a hack
> > > that preserves effective compatibility when it works, but breaks it
> > > uncleanly when it doesn't.
> > > 
> > > As a developer, I'm insisting on it.
> > > 
> > > So, if you want B, the onus is on *you* to show us why nothing bad will
> > > happen.
> > > 
> 
> I agree with the conclusion for option B.  But I think the correct
> solution is not A, it is to analyse changes, maybe even test, and show
> that nothing bad can happen.

Do you volunteer for that work?

> 
> Because A suffers from exactly the same problem if people just blindly
> switch to a new machine type.

Anything can happen if people change their configurations
blindly.

Nobody should change configuration blindly, and that's also why
we shouldn't run a different machine when the user is asking for
an old one. We don't know why the user is asking for an old
machine and we can't make decisions for the user. Management
software might know why an old machine is being used and might be
able to help update the config, but QEMU doesn't.

-- 
Eduardo

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

* Re: [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code
  2016-02-04 19:09       ` Eduardo Habkost
@ 2016-02-04 22:14         ` Michael S. Tsirkin
  2016-02-05  9:09           ` Markus Armbruster
  2016-02-05 14:46           ` Eduardo Habkost
  0 siblings, 2 replies; 39+ messages in thread
From: Michael S. Tsirkin @ 2016-02-04 22:14 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Marcel Apfelbaum, Laszlo Ersek, qemu-devel, Markus Armbruster,
	Igor Mammedov, Paolo Bonzini, John Snow

On Thu, Feb 04, 2016 at 05:09:44PM -0200, Eduardo Habkost wrote:
> On Thu, Feb 04, 2016 at 08:02:30PM +0200, Michael S. Tsirkin wrote:
> > On Thu, Feb 04, 2016 at 03:16:17PM -0200, Eduardo Habkost wrote:
> > > On Thu, Feb 04, 2016 at 06:01:50PM +0200, Michael S. Tsirkin wrote:
> > > > On Sat, Jan 23, 2016 at 02:02:08PM -0200, Eduardo Habkost wrote:
> > > > > This is another attempt to remove old q35 machine code. Now I am
> > > > > also removing unused compat code to demonstrate the benefit of
> > > > > throwing away the old code that nobody uses.
> > > > 
> > > > The same thing I said applies - we don't know that nobody uses old q35
> > > > machine types.
> > > > We do know we don't need to migrate to/from them,
> > > > so we can drop compat code.
> > > > But please add aliases so people can still start these machines.
> > > 
> > > If people use them, they can easily update their configurations.
> > > I will copy and paste the reply Markus sent 4 months ago below.
> > > 
> > > On Mon, Sep 14, 2015 at 09:18:47AM +0200, Markus Armbruster wrote:
> > > > We've been through this before, but we can go through it once more.
> > > > Choices:
> > > > 
> > > > A. Remove old machine type
> > > > 
> > > >    A guest using it can't be started.  Easy to understand on the host.
> > > >    An error message advising to switch to a newer machine type would be
> > > >    a nice touch.
> > > > 
> > > >    This is a clean break in backward compatibility.  To be mentioned in
> > > >    release notes, obviously.
> > > > 
> > > > B. Change old machine type in a guest-visible way
> > > > 
> > > >    Depending on the nature of the change and the guest, a guest using it
> > > >    either doesn't notice, copes with it successfully, or fails in
> > > >    guest-specific ways.  If the latter, the failure can be "guest
> > > >    hangs", which is much harder to figure out than A.
> > > > 
> > > >    Unless we can *demonstrate* that nothing bad happens for all the
> > > >    guests people actually use with the old machine types, this is a
> > > >    different kind of backward compatibility break.
> > > > 
> > > >    Demonstrating this is feels infeasible to me, but you're welcome to
> > > >    try.
> > > > 
> > > > I could call the difference between the two a tradeoff, but since we've
> > > > been through this before, I'll be more blunt: choosing B robs Peter (the
> > > > guy with guests where badness happens) to pay Paul (the guy with guests
> > > > that cope).  Paul is saved the inconvenience of having to read release
> > > > notes or his logs, and change machine types.  Peter pays for that with
> > > > figuring out WTF his guests are doing now.
> > > > 
> > > > As a user, I'd pick a clean break in backward compatibility over a hack
> > > > that preserves effective compatibility when it works, but breaks it
> > > > uncleanly when it doesn't.
> > > > 
> > > > As a developer, I'm insisting on it.
> > > > 
> > > > So, if you want B, the onus is on *you* to show us why nothing bad will
> > > > happen.
> > > > 
> > 
> > I agree with the conclusion for option B.  But I think the correct
> > solution is not A, it is to analyse changes, maybe even test, and show
> > that nothing bad can happen.
> 
> Do you volunteer for that work?

Nope, sorry. It's your idea, your patchset.  I am saying, look for some
low-hanging fruit.  Find some compat options we can drop without
breaking guests, drop just these.  Are there options we need for piix
anyway? No point in dropping them at all.

For example, the builtin AML can be dropped since we
always use a bios with acpi support now.
It is also trivial to test.

Memory layout is probably ok to change.

Maybe more.

> > 
> > Because A suffers from exactly the same problem if people just blindly
> > switch to a new machine type.
> 
> Anything can happen if people change their configurations
> blindly.
> 
> Nobody should change configuration blindly, and that's also why
> we shouldn't run a different machine when the user is asking for
> an old one. We don't know why the user is asking for an old
> machine and we can't make decisions for the user. Management
> software might know why an old machine is being used and might be
> able to help update the config, but QEMU doesn't.

What guidance do we provide? Try it and see if it works?  What exactly
do we ask user to test? If QEMU developers can't find out whether
switching a machine type is safe, what hope is there that management
developers can?

> -- 
> Eduardo

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

* Re: [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code
  2016-02-04 18:02     ` Michael S. Tsirkin
  2016-02-04 19:09       ` Eduardo Habkost
@ 2016-02-05  8:55       ` Markus Armbruster
  2016-02-07 10:17         ` Michael S. Tsirkin
  1 sibling, 1 reply; 39+ messages in thread
From: Markus Armbruster @ 2016-02-05  8:55 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Eduardo Habkost, Marcel Apfelbaum, John Snow, qemu-devel,
	Paolo Bonzini, Igor Mammedov, Laszlo Ersek

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Thu, Feb 04, 2016 at 03:16:17PM -0200, Eduardo Habkost wrote:
>> On Thu, Feb 04, 2016 at 06:01:50PM +0200, Michael S. Tsirkin wrote:
>> > On Sat, Jan 23, 2016 at 02:02:08PM -0200, Eduardo Habkost wrote:
>> > > This is another attempt to remove old q35 machine code. Now I am
>> > > also removing unused compat code to demonstrate the benefit of
>> > > throwing away the old code that nobody uses.
>> > 
>> > The same thing I said applies - we don't know that nobody uses old q35
>> > machine types.
>> > We do know we don't need to migrate to/from them,
>> > so we can drop compat code.
>> > But please add aliases so people can still start these machines.
>> 
>> If people use them, they can easily update their configurations.
>> I will copy and paste the reply Markus sent 4 months ago below.
>> 
>> On Mon, Sep 14, 2015 at 09:18:47AM +0200, Markus Armbruster wrote:
>> > We've been through this before, but we can go through it once more.
>> > Choices:
>> > 
>> > A. Remove old machine type
>> > 
>> >    A guest using it can't be started.  Easy to understand on the host.
>> >    An error message advising to switch to a newer machine type would be
>> >    a nice touch.
>> > 
>> >    This is a clean break in backward compatibility.  To be mentioned in
>> >    release notes, obviously.
>> > 
>> > B. Change old machine type in a guest-visible way
>> > 
>> >    Depending on the nature of the change and the guest, a guest using it
>> >    either doesn't notice, copes with it successfully, or fails in
>> >    guest-specific ways.  If the latter, the failure can be "guest
>> >    hangs", which is much harder to figure out than A.
>> > 
>> >    Unless we can *demonstrate* that nothing bad happens for all the
>> >    guests people actually use with the old machine types, this is a
>> >    different kind of backward compatibility break.
>> > 
>> >    Demonstrating this is feels infeasible to me, but you're welcome to
>> >    try.
>> > 
>> > I could call the difference between the two a tradeoff, but since we've
>> > been through this before, I'll be more blunt: choosing B robs Peter (the
>> > guy with guests where badness happens) to pay Paul (the guy with guests
>> > that cope).  Paul is saved the inconvenience of having to read release
>> > notes or his logs, and change machine types.  Peter pays for that with
>> > figuring out WTF his guests are doing now.
>> > 
>> > As a user, I'd pick a clean break in backward compatibility over a hack
>> > that preserves effective compatibility when it works, but breaks it
>> > uncleanly when it doesn't.
>> > 
>> > As a developer, I'm insisting on it.
>> > 
>> > So, if you want B, the onus is on *you* to show us why nothing bad will
>> > happen.
>> > 
>
> I agree with the conclusion for option B.  But I think the correct
> solution is not A, it is to analyse changes, maybe even test, and show
> that nothing bad can happen.

What exactly are you proposing to do?  Who should be doing it?  And what
other work are you willing to sacrifice to get this task done?

> Because A suffers from exactly the same problem if people just blindly
> switch to a new machine type.

PEBKAC

Users switch machine types for any number of reasons, good and bad.  You
can't stop the ones switching for bad reasons by keeping some barely
working machine type around forever.

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

* Re: [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code
  2016-02-04 22:14         ` Michael S. Tsirkin
@ 2016-02-05  9:09           ` Markus Armbruster
  2016-02-05 14:46           ` Eduardo Habkost
  1 sibling, 0 replies; 39+ messages in thread
From: Markus Armbruster @ 2016-02-05  9:09 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Eduardo Habkost, Marcel Apfelbaum, John Snow, qemu-devel,
	Paolo Bonzini, Igor Mammedov, Laszlo Ersek

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Thu, Feb 04, 2016 at 05:09:44PM -0200, Eduardo Habkost wrote:
>> On Thu, Feb 04, 2016 at 08:02:30PM +0200, Michael S. Tsirkin wrote:
>> > On Thu, Feb 04, 2016 at 03:16:17PM -0200, Eduardo Habkost wrote:
>> > > On Thu, Feb 04, 2016 at 06:01:50PM +0200, Michael S. Tsirkin wrote:
>> > > > On Sat, Jan 23, 2016 at 02:02:08PM -0200, Eduardo Habkost wrote:
>> > > > > This is another attempt to remove old q35 machine code. Now I am
>> > > > > also removing unused compat code to demonstrate the benefit of
>> > > > > throwing away the old code that nobody uses.
>> > > > 
>> > > > The same thing I said applies - we don't know that nobody uses old q35
>> > > > machine types.
>> > > > We do know we don't need to migrate to/from them,
>> > > > so we can drop compat code.
>> > > > But please add aliases so people can still start these machines.
>> > > 
>> > > If people use them, they can easily update their configurations.
>> > > I will copy and paste the reply Markus sent 4 months ago below.
>> > > 
>> > > On Mon, Sep 14, 2015 at 09:18:47AM +0200, Markus Armbruster wrote:
>> > > > We've been through this before, but we can go through it once more.
>> > > > Choices:
>> > > > 
>> > > > A. Remove old machine type
>> > > > 
>> > > >    A guest using it can't be started.  Easy to understand on the host.
>> > > >    An error message advising to switch to a newer machine type would be
>> > > >    a nice touch.
>> > > > 
>> > > >    This is a clean break in backward compatibility.  To be mentioned in
>> > > >    release notes, obviously.
>> > > > 
>> > > > B. Change old machine type in a guest-visible way
>> > > > 
>> > > >    Depending on the nature of the change and the guest, a guest using it
>> > > >    either doesn't notice, copes with it successfully, or fails in
>> > > >    guest-specific ways.  If the latter, the failure can be "guest
>> > > >    hangs", which is much harder to figure out than A.
>> > > > 
>> > > >    Unless we can *demonstrate* that nothing bad happens for all the
>> > > >    guests people actually use with the old machine types, this is a
>> > > >    different kind of backward compatibility break.
>> > > > 
>> > > >    Demonstrating this is feels infeasible to me, but you're welcome to
>> > > >    try.
>> > > > 
>> > > > I could call the difference between the two a tradeoff, but since we've
>> > > > been through this before, I'll be more blunt: choosing B robs Peter (the
>> > > > guy with guests where badness happens) to pay Paul (the guy with guests
>> > > > that cope).  Paul is saved the inconvenience of having to read release
>> > > > notes or his logs, and change machine types.  Peter pays for that with
>> > > > figuring out WTF his guests are doing now.
>> > > > 
>> > > > As a user, I'd pick a clean break in backward compatibility over a hack
>> > > > that preserves effective compatibility when it works, but breaks it
>> > > > uncleanly when it doesn't.
>> > > > 
>> > > > As a developer, I'm insisting on it.
>> > > > 
>> > > > So, if you want B, the onus is on *you* to show us why nothing bad will
>> > > > happen.
>> > > > 
>> > 
>> > I agree with the conclusion for option B.  But I think the correct
>> > solution is not A, it is to analyse changes, maybe even test, and show
>> > that nothing bad can happen.
>> 
>> Do you volunteer for that work?
>
> Nope, sorry. It's your idea, your patchset.  I am saying, look for some
> low-hanging fruit.  Find some compat options we can drop without
> breaking guests, drop just these.  Are there options we need for piix
> anyway? No point in dropping them at all.
>
> For example, the builtin AML can be dropped since we
> always use a bios with acpi support now.
> It is also trivial to test.

Are you seriously proposing to *change* the old machine types in ways
that survive (some) testing?

If yes, that's still B, no matter how hard you swear it isn't.

> Memory layout is probably ok to change.
>
> Maybe more.

Unbounded, ill-specified wild goose chase.  Ridiculous waste of
resources.

>> > Because A suffers from exactly the same problem if people just blindly
>> > switch to a new machine type.
>> 
>> Anything can happen if people change their configurations
>> blindly.
>> 
>> Nobody should change configuration blindly, and that's also why
>> we shouldn't run a different machine when the user is asking for
>> an old one. We don't know why the user is asking for an old
>> machine and we can't make decisions for the user. Management
>> software might know why an old machine is being used and might be
>> able to help update the config, but QEMU doesn't.
>
> What guidance do we provide? Try it and see if it works?  What exactly
> do we ask user to test? If QEMU developers can't find out whether
> switching a machine type is safe, what hope is there that management
> developers can?

Switching to a different virtual machine type is exactly the same as
switching to a different real machine type.  You have to shut off to do
it.  Whether things still work afterwards is anybody's guess, but decent
OSes make an effort to cope, and usually succeed.

That's pretty much all the guidance real hardware manufacturers provide,
and it's certainly all we should provide.

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

* Re: [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code
  2016-02-04 22:14         ` Michael S. Tsirkin
  2016-02-05  9:09           ` Markus Armbruster
@ 2016-02-05 14:46           ` Eduardo Habkost
  2016-02-06 18:34             ` Michael S. Tsirkin
  1 sibling, 1 reply; 39+ messages in thread
From: Eduardo Habkost @ 2016-02-05 14:46 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Marcel Apfelbaum, Laszlo Ersek, qemu-devel, Markus Armbruster,
	Igor Mammedov, Paolo Bonzini, John Snow

On Fri, Feb 05, 2016 at 12:14:16AM +0200, Michael S. Tsirkin wrote:
> On Thu, Feb 04, 2016 at 05:09:44PM -0200, Eduardo Habkost wrote:
> > On Thu, Feb 04, 2016 at 08:02:30PM +0200, Michael S. Tsirkin wrote:
> > > On Thu, Feb 04, 2016 at 03:16:17PM -0200, Eduardo Habkost wrote:
> > > > On Thu, Feb 04, 2016 at 06:01:50PM +0200, Michael S. Tsirkin wrote:
> > > > > On Sat, Jan 23, 2016 at 02:02:08PM -0200, Eduardo Habkost wrote:
> > > > > > This is another attempt to remove old q35 machine code. Now I am
> > > > > > also removing unused compat code to demonstrate the benefit of
> > > > > > throwing away the old code that nobody uses.
> > > > > 
> > > > > The same thing I said applies - we don't know that nobody uses old q35
> > > > > machine types.
> > > > > We do know we don't need to migrate to/from them,
> > > > > so we can drop compat code.
> > > > > But please add aliases so people can still start these machines.
> > > > 
> > > > If people use them, they can easily update their configurations.
> > > > I will copy and paste the reply Markus sent 4 months ago below.
> > > > 
> > > > On Mon, Sep 14, 2015 at 09:18:47AM +0200, Markus Armbruster wrote:
> > > > > We've been through this before, but we can go through it once more.
> > > > > Choices:
> > > > > 
> > > > > A. Remove old machine type
> > > > > 
> > > > >    A guest using it can't be started.  Easy to understand on the host.
> > > > >    An error message advising to switch to a newer machine type would be
> > > > >    a nice touch.
> > > > > 
> > > > >    This is a clean break in backward compatibility.  To be mentioned in
> > > > >    release notes, obviously.
> > > > > 
> > > > > B. Change old machine type in a guest-visible way
> > > > > 
> > > > >    Depending on the nature of the change and the guest, a guest using it
> > > > >    either doesn't notice, copes with it successfully, or fails in
> > > > >    guest-specific ways.  If the latter, the failure can be "guest
> > > > >    hangs", which is much harder to figure out than A.
> > > > > 
> > > > >    Unless we can *demonstrate* that nothing bad happens for all the
> > > > >    guests people actually use with the old machine types, this is a
> > > > >    different kind of backward compatibility break.
> > > > > 
> > > > >    Demonstrating this is feels infeasible to me, but you're welcome to
> > > > >    try.
> > > > > 
> > > > > I could call the difference between the two a tradeoff, but since we've
> > > > > been through this before, I'll be more blunt: choosing B robs Peter (the
> > > > > guy with guests where badness happens) to pay Paul (the guy with guests
> > > > > that cope).  Paul is saved the inconvenience of having to read release
> > > > > notes or his logs, and change machine types.  Peter pays for that with
> > > > > figuring out WTF his guests are doing now.
> > > > > 
> > > > > As a user, I'd pick a clean break in backward compatibility over a hack
> > > > > that preserves effective compatibility when it works, but breaks it
> > > > > uncleanly when it doesn't.
> > > > > 
> > > > > As a developer, I'm insisting on it.
> > > > > 
> > > > > So, if you want B, the onus is on *you* to show us why nothing bad will
> > > > > happen.
> > > > > 
> > > 
> > > I agree with the conclusion for option B.  But I think the correct
> > > solution is not A, it is to analyse changes, maybe even test, and show
> > > that nothing bad can happen.
> > 
> > Do you volunteer for that work?
> 
> Nope, sorry. It's your idea, your patchset.

It's your idea. You are the one proposing to waste resources
keeping an old machine-type name "working" just because you don't
want users (who we don't even know if they actually exist) to
update their configurations on a QEMU upgrade.

I am proposing the opposite: dropping support to a feature that
people are unlikely to be using, in a very clear way.


> I am saying, look
> for some low-hanging fruit.  Find some compat options we can
> drop without breaking guests, drop just these.  Are there
> options we need for piix anyway? No point in dropping them at
> all.
> 
> For example, the builtin AML can be dropped since we always use
> a bios with acpi support now.  It is also trivial to test.
> 
> Memory layout is probably ok to change.
> 
> Maybe more.
> 
> > > 
> > > Because A suffers from exactly the same problem if people
> > > just blindly switch to a new machine type.
> > 
> > Anything can happen if people change their configurations
> > blindly.
> > 
> > Nobody should change configuration blindly, and that's also
> > why we shouldn't run a different machine when the user is
> > asking for an old one. We don't know why the user is asking
> > for an old machine and we can't make decisions for the user.
> > Management software might know why an old machine is being
> > used and might be able to help update the config, but QEMU
> > doesn't.
> 
> What guidance do we provide? Try it and see if it works?  What
> exactly do we ask user to test? If QEMU developers can't find
> out whether switching a machine type is safe, what hope is
> there that management developers can?

Exactly the same guidance vendors already provide for people that
want to change machine types today. It depends on who wrote the
config files and why, and we can't and shouldn't make any
guesses.

-- 
Eduardo

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

* Re: [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code
  2016-02-05 14:46           ` Eduardo Habkost
@ 2016-02-06 18:34             ` Michael S. Tsirkin
  2016-02-11 15:51               ` Eduardo Habkost
  0 siblings, 1 reply; 39+ messages in thread
From: Michael S. Tsirkin @ 2016-02-06 18:34 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Marcel Apfelbaum, Laszlo Ersek, qemu-devel, Markus Armbruster,
	Igor Mammedov, Paolo Bonzini, John Snow

On Fri, Feb 05, 2016 at 12:46:11PM -0200, Eduardo Habkost wrote:
> On Fri, Feb 05, 2016 at 12:14:16AM +0200, Michael S. Tsirkin wrote:
> > On Thu, Feb 04, 2016 at 05:09:44PM -0200, Eduardo Habkost wrote:
> > > On Thu, Feb 04, 2016 at 08:02:30PM +0200, Michael S. Tsirkin wrote:
> > > > On Thu, Feb 04, 2016 at 03:16:17PM -0200, Eduardo Habkost wrote:
> > > > > On Thu, Feb 04, 2016 at 06:01:50PM +0200, Michael S. Tsirkin wrote:
> > > > > > On Sat, Jan 23, 2016 at 02:02:08PM -0200, Eduardo Habkost wrote:
> > > > > > > This is another attempt to remove old q35 machine code. Now I am
> > > > > > > also removing unused compat code to demonstrate the benefit of
> > > > > > > throwing away the old code that nobody uses.
> > > > > > 
> > > > > > The same thing I said applies - we don't know that nobody uses old q35
> > > > > > machine types.
> > > > > > We do know we don't need to migrate to/from them,
> > > > > > so we can drop compat code.
> > > > > > But please add aliases so people can still start these machines.
> > > > > 
> > > > > If people use them, they can easily update their configurations.
> > > > > I will copy and paste the reply Markus sent 4 months ago below.
> > > > > 
> > > > > On Mon, Sep 14, 2015 at 09:18:47AM +0200, Markus Armbruster wrote:
> > > > > > We've been through this before, but we can go through it once more.
> > > > > > Choices:
> > > > > > 
> > > > > > A. Remove old machine type
> > > > > > 
> > > > > >    A guest using it can't be started.  Easy to understand on the host.
> > > > > >    An error message advising to switch to a newer machine type would be
> > > > > >    a nice touch.
> > > > > > 
> > > > > >    This is a clean break in backward compatibility.  To be mentioned in
> > > > > >    release notes, obviously.
> > > > > > 
> > > > > > B. Change old machine type in a guest-visible way
> > > > > > 
> > > > > >    Depending on the nature of the change and the guest, a guest using it
> > > > > >    either doesn't notice, copes with it successfully, or fails in
> > > > > >    guest-specific ways.  If the latter, the failure can be "guest
> > > > > >    hangs", which is much harder to figure out than A.
> > > > > > 
> > > > > >    Unless we can *demonstrate* that nothing bad happens for all the
> > > > > >    guests people actually use with the old machine types, this is a
> > > > > >    different kind of backward compatibility break.
> > > > > > 
> > > > > >    Demonstrating this is feels infeasible to me, but you're welcome to
> > > > > >    try.
> > > > > > 
> > > > > > I could call the difference between the two a tradeoff, but since we've
> > > > > > been through this before, I'll be more blunt: choosing B robs Peter (the
> > > > > > guy with guests where badness happens) to pay Paul (the guy with guests
> > > > > > that cope).  Paul is saved the inconvenience of having to read release
> > > > > > notes or his logs, and change machine types.  Peter pays for that with
> > > > > > figuring out WTF his guests are doing now.
> > > > > > 
> > > > > > As a user, I'd pick a clean break in backward compatibility over a hack
> > > > > > that preserves effective compatibility when it works, but breaks it
> > > > > > uncleanly when it doesn't.
> > > > > > 
> > > > > > As a developer, I'm insisting on it.
> > > > > > 
> > > > > > So, if you want B, the onus is on *you* to show us why nothing bad will
> > > > > > happen.
> > > > > > 
> > > > 
> > > > I agree with the conclusion for option B.  But I think the correct
> > > > solution is not A, it is to analyse changes, maybe even test, and show
> > > > that nothing bad can happen.
> > > 
> > > Do you volunteer for that work?
> > 
> > Nope, sorry. It's your idea, your patchset.
> 
> It's your idea. You are the one proposing to waste resources
> keeping an old machine-type name "working" just because you don't
> want users (who we don't even know if they actually exist) to
> update their configurations on a QEMU upgrade.
> 
> I am proposing the opposite: dropping support to a feature that
> people are unlikely to be using, in a very clear way.

What will happen with installed VMs? Will they break?

> 
> > I am saying, look
> > for some low-hanging fruit.  Find some compat options we can
> > drop without breaking guests, drop just these.  Are there
> > options we need for piix anyway? No point in dropping them at
> > all.
> > 
> > For example, the builtin AML can be dropped since we always use
> > a bios with acpi support now.  It is also trivial to test.
> > 
> > Memory layout is probably ok to change.
> > 
> > Maybe more.
> > 
> > > > 
> > > > Because A suffers from exactly the same problem if people
> > > > just blindly switch to a new machine type.
> > > 
> > > Anything can happen if people change their configurations
> > > blindly.
> > > 
> > > Nobody should change configuration blindly, and that's also
> > > why we shouldn't run a different machine when the user is
> > > asking for an old one. We don't know why the user is asking
> > > for an old machine and we can't make decisions for the user.
> > > Management software might know why an old machine is being
> > > used and might be able to help update the config, but QEMU
> > > doesn't.
> > 
> > What guidance do we provide? Try it and see if it works?  What
> > exactly do we ask user to test? If QEMU developers can't find
> > out whether switching a machine type is safe, what hope is
> > there that management developers can?
> 
> Exactly the same guidance vendors already provide for people that
> want to change machine types today. It depends on who wrote the
> config files and why, and we can't and shouldn't make any
> guesses.

AFAIK that's basically "don't do it".

> -- 
> Eduardo

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

* Re: [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code
  2016-02-05  8:55       ` Markus Armbruster
@ 2016-02-07 10:17         ` Michael S. Tsirkin
  2016-02-08 11:59           ` Markus Armbruster
  0 siblings, 1 reply; 39+ messages in thread
From: Michael S. Tsirkin @ 2016-02-07 10:17 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Eduardo Habkost, Marcel Apfelbaum, John Snow, qemu-devel,
	Paolo Bonzini, Igor Mammedov, Laszlo Ersek

On Fri, Feb 05, 2016 at 09:55:08AM +0100, Markus Armbruster wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Thu, Feb 04, 2016 at 03:16:17PM -0200, Eduardo Habkost wrote:
> >> On Thu, Feb 04, 2016 at 06:01:50PM +0200, Michael S. Tsirkin wrote:
> >> > On Sat, Jan 23, 2016 at 02:02:08PM -0200, Eduardo Habkost wrote:
> >> > > This is another attempt to remove old q35 machine code. Now I am
> >> > > also removing unused compat code to demonstrate the benefit of
> >> > > throwing away the old code that nobody uses.
> >> > 
> >> > The same thing I said applies - we don't know that nobody uses old q35
> >> > machine types.
> >> > We do know we don't need to migrate to/from them,
> >> > so we can drop compat code.
> >> > But please add aliases so people can still start these machines.
> >> 
> >> If people use them, they can easily update their configurations.
> >> I will copy and paste the reply Markus sent 4 months ago below.
> >> 
> >> On Mon, Sep 14, 2015 at 09:18:47AM +0200, Markus Armbruster wrote:
> >> > We've been through this before, but we can go through it once more.
> >> > Choices:
> >> > 
> >> > A. Remove old machine type
> >> > 
> >> >    A guest using it can't be started.  Easy to understand on the host.
> >> >    An error message advising to switch to a newer machine type would be
> >> >    a nice touch.
> >> > 
> >> >    This is a clean break in backward compatibility.  To be mentioned in
> >> >    release notes, obviously.
> >> > 
> >> > B. Change old machine type in a guest-visible way
> >> > 
> >> >    Depending on the nature of the change and the guest, a guest using it
> >> >    either doesn't notice, copes with it successfully, or fails in
> >> >    guest-specific ways.  If the latter, the failure can be "guest
> >> >    hangs", which is much harder to figure out than A.
> >> > 
> >> >    Unless we can *demonstrate* that nothing bad happens for all the
> >> >    guests people actually use with the old machine types, this is a
> >> >    different kind of backward compatibility break.
> >> > 
> >> >    Demonstrating this is feels infeasible to me, but you're welcome to
> >> >    try.
> >> > 
> >> > I could call the difference between the two a tradeoff, but since we've
> >> > been through this before, I'll be more blunt: choosing B robs Peter (the
> >> > guy with guests where badness happens) to pay Paul (the guy with guests
> >> > that cope).  Paul is saved the inconvenience of having to read release
> >> > notes or his logs, and change machine types.  Peter pays for that with
> >> > figuring out WTF his guests are doing now.
> >> > 
> >> > As a user, I'd pick a clean break in backward compatibility over a hack
> >> > that preserves effective compatibility when it works, but breaks it
> >> > uncleanly when it doesn't.
> >> > 
> >> > As a developer, I'm insisting on it.
> >> > 
> >> > So, if you want B, the onus is on *you* to show us why nothing bad will
> >> > happen.
> >> > 
> >
> > I agree with the conclusion for option B.  But I think the correct
> > solution is not A, it is to analyse changes, maybe even test, and show
> > that nothing bad can happen.
> 
> What exactly are you proposing to do?

I'm not sure. But if someone says "we drop machine X, people can use
machine Y instead" then clearly, there should be some data
included about how well does Y function in place of X:
based on code analysis, testing, or both.


Some ideas for things that seem rather safe based on code analysis:

1. We should be able to explicitly disable migration for old machine
types that could not migrate historically so users don't try to migrate
them now that they can.

2. Drop stuff that isn't guest OS visible, that we
keep around for migration or old bios support.
has_acpi_build - old bios support will allow removing the dsdt AML.
option_rom_has_mr - not guest visible.


Nothing is 100% safe - e.g. people that use their own bios will get
bitten by removal of AML.


>  Who should be doing it?

Whoever's interested.

>  And what
>  other work are you willing to sacrifice to get this task done?

I don't much care about getting this done - most stuff has to
be there for piix anyway.

> > Because A suffers from exactly the same problem if people just blindly
> > switch to a new machine type.
> 
> PEBKAC
> 
> Users switch machine types for any number of reasons, good and bad.  You
> can't stop the ones switching for bad reasons by keeping some barely
> working machine type around forever.

If you tell people to switch to a new machine type, then switching is
not PEBKAC and you need a tested migration path that does not involve
reinstalling the gust OS for most users.

-- 
MST

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

* Re: [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code
  2016-02-07 10:17         ` Michael S. Tsirkin
@ 2016-02-08 11:59           ` Markus Armbruster
  0 siblings, 0 replies; 39+ messages in thread
From: Markus Armbruster @ 2016-02-08 11:59 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Eduardo Habkost, Marcel Apfelbaum, Laszlo Ersek, qemu-devel,
	Igor Mammedov, Paolo Bonzini, John Snow

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Fri, Feb 05, 2016 at 09:55:08AM +0100, Markus Armbruster wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> 
>> > On Thu, Feb 04, 2016 at 03:16:17PM -0200, Eduardo Habkost wrote:
>> >> On Thu, Feb 04, 2016 at 06:01:50PM +0200, Michael S. Tsirkin wrote:
>> >> > On Sat, Jan 23, 2016 at 02:02:08PM -0200, Eduardo Habkost wrote:
>> >> > > This is another attempt to remove old q35 machine code. Now I am
>> >> > > also removing unused compat code to demonstrate the benefit of
>> >> > > throwing away the old code that nobody uses.
>> >> > 
>> >> > The same thing I said applies - we don't know that nobody uses old q35
>> >> > machine types.
>> >> > We do know we don't need to migrate to/from them,
>> >> > so we can drop compat code.
>> >> > But please add aliases so people can still start these machines.
>> >> 
>> >> If people use them, they can easily update their configurations.
>> >> I will copy and paste the reply Markus sent 4 months ago below.
>> >> 
>> >> On Mon, Sep 14, 2015 at 09:18:47AM +0200, Markus Armbruster wrote:
>> >> > We've been through this before, but we can go through it once more.
>> >> > Choices:
>> >> > 
>> >> > A. Remove old machine type
>> >> > 
>> >> >    A guest using it can't be started.  Easy to understand on the host.
>> >> >    An error message advising to switch to a newer machine type would be
>> >> >    a nice touch.
>> >> > 
>> >> >    This is a clean break in backward compatibility.  To be mentioned in
>> >> >    release notes, obviously.
>> >> > 
>> >> > B. Change old machine type in a guest-visible way
>> >> > 
>> >> >    Depending on the nature of the change and the guest, a guest using it
>> >> >    either doesn't notice, copes with it successfully, or fails in
>> >> >    guest-specific ways.  If the latter, the failure can be "guest
>> >> >    hangs", which is much harder to figure out than A.
>> >> > 
>> >> >    Unless we can *demonstrate* that nothing bad happens for all the
>> >> >    guests people actually use with the old machine types, this is a
>> >> >    different kind of backward compatibility break.
>> >> > 
>> >> >    Demonstrating this is feels infeasible to me, but you're welcome to
>> >> >    try.
>> >> > 
>> >> > I could call the difference between the two a tradeoff, but since we've
>> >> > been through this before, I'll be more blunt: choosing B robs Peter (the
>> >> > guy with guests where badness happens) to pay Paul (the guy with guests
>> >> > that cope).  Paul is saved the inconvenience of having to read release
>> >> > notes or his logs, and change machine types.  Peter pays for that with
>> >> > figuring out WTF his guests are doing now.
>> >> > 
>> >> > As a user, I'd pick a clean break in backward compatibility over a hack
>> >> > that preserves effective compatibility when it works, but breaks it
>> >> > uncleanly when it doesn't.
>> >> > 
>> >> > As a developer, I'm insisting on it.
>> >> > 
>> >> > So, if you want B, the onus is on *you* to show us why nothing bad will
>> >> > happen.
>> >> > 
>> >
>> > I agree with the conclusion for option B.  But I think the correct
>> > solution is not A, it is to analyse changes, maybe even test, and show
>> > that nothing bad can happen.
>> 
>> What exactly are you proposing to do?
>
> I'm not sure. But if someone says "we drop machine X, people can use
> machine Y instead" then clearly, there should be some data
> included about how well does Y function in place of X:
> based on code analysis, testing, or both.

No, that's not what we say.  We say: machine type X is no longer
supported in new versions of QEMU.  You can either stay on older QEMU
versions, or you can migrate to newer virtual hardware.

Migrating to new hardware (virtual or physical) is a well understood
problem.  You can go with a fresh install on new hardware.  You can also
try to move your old disk or disk contents to new hardware.  The latter
usually works when the new hardware isn't too different.  But if it
breaks, you get to keep the pieces.

Since "stay on old QEMU or migrate to another machine type"
inconveniences users, we make an effort[*] to keep those machine types
alive that people actually use seriously.

The old q35 machine types are experimental and barely work.  There is no
evidence of non-experimental use.  Keeping them alive would therefore be
a misuse of scarce resources.

> Some ideas for things that seem rather safe based on code analysis:
>
> 1. We should be able to explicitly disable migration for old machine
> types that could not migrate historically so users don't try to migrate
> them now that they can.

Nobody should should be migrating these machine types for the simple
reason that nobody should be using them.

> 2. Drop stuff that isn't guest OS visible, that we
> keep around for migration or old bios support.
> has_acpi_build - old bios support will allow removing the dsdt AML.
> option_rom_has_mr - not guest visible.
>
>
> Nothing is 100% safe - e.g. people that use their own bios will get
> bitten by removal of AML.

Keeping these types is a pointless waste of maintenance resources.
Messing with them additionally wastes development resources.

>>  Who should be doing it?
>
> Whoever's interested.

I'm having a hard time believing what I read, so I'm asking you to
correct my reading: as the PC maintainer, you demand that "somebody"
goes on an unbounded, ill-defined, "I'm not sure" research trip before
we can get rid of experimental machine types that barely worked and
nobody should be using.  Without any evidence of actual use, let alone
harm.  What am I misunderstanding?

>>  And what
>>  other work are you willing to sacrifice to get this task done?
>
> I don't much care about getting this done - most stuff has to
> be there for piix anyway.
>
>> > Because A suffers from exactly the same problem if people just blindly
>> > switch to a new machine type.
>> 
>> PEBKAC
>> 
>> Users switch machine types for any number of reasons, good and bad.  You
>> can't stop the ones switching for bad reasons by keeping some barely
>> working machine type around forever.
>
> If you tell people to switch to a new machine type, then switching is
> not PEBKAC and you need a tested migration path that does not involve
> reinstalling the gust OS for most users.

You're missing my point entirely.

Switching machine types without understanding the risks is PEBKAC.

I'm not proposing to tell our users to blindly switch machine types.  I'm
proposing to tell users of the old q35 machine types (if they exist at
all, which I doubt) to migrate to a suitable new machine type.


[*] "Make an effort" because we lack the resources to systematically
catch regressions before they happen.  If you need stability, use a
downstream that does the necessary work.

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

* Re: [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code
  2016-02-06 18:34             ` Michael S. Tsirkin
@ 2016-02-11 15:51               ` Eduardo Habkost
  2016-02-11 16:33                 ` Michael S. Tsirkin
  0 siblings, 1 reply; 39+ messages in thread
From: Eduardo Habkost @ 2016-02-11 15:51 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Marcel Apfelbaum, Laszlo Ersek, qemu-devel, Markus Armbruster,
	Igor Mammedov, Paolo Bonzini, John Snow

On Sat, Feb 06, 2016 at 08:34:07PM +0200, Michael S. Tsirkin wrote:
> On Fri, Feb 05, 2016 at 12:46:11PM -0200, Eduardo Habkost wrote:
> > On Fri, Feb 05, 2016 at 12:14:16AM +0200, Michael S. Tsirkin wrote:
> > > On Thu, Feb 04, 2016 at 05:09:44PM -0200, Eduardo Habkost wrote:
> > > > On Thu, Feb 04, 2016 at 08:02:30PM +0200, Michael S. Tsirkin wrote:
> > > > > On Thu, Feb 04, 2016 at 03:16:17PM -0200, Eduardo Habkost wrote:
> > > > > > On Thu, Feb 04, 2016 at 06:01:50PM +0200, Michael S. Tsirkin wrote:
> > > > > > > On Sat, Jan 23, 2016 at 02:02:08PM -0200, Eduardo Habkost wrote:
> > > > > > > > This is another attempt to remove old q35 machine code. Now I am
> > > > > > > > also removing unused compat code to demonstrate the benefit of
> > > > > > > > throwing away the old code that nobody uses.
> > > > > > > 
> > > > > > > The same thing I said applies - we don't know that nobody uses old q35
> > > > > > > machine types.
> > > > > > > We do know we don't need to migrate to/from them,
> > > > > > > so we can drop compat code.
> > > > > > > But please add aliases so people can still start these machines.
> > > > > > 
> > > > > > If people use them, they can easily update their configurations.
> > > > > > I will copy and paste the reply Markus sent 4 months ago below.
> > > > > > 
> > > > > > On Mon, Sep 14, 2015 at 09:18:47AM +0200, Markus Armbruster wrote:
> > > > > > > We've been through this before, but we can go through it once more.
> > > > > > > Choices:
> > > > > > > 
> > > > > > > A. Remove old machine type
> > > > > > > 
> > > > > > >    A guest using it can't be started.  Easy to understand on the host.
> > > > > > >    An error message advising to switch to a newer machine type would be
> > > > > > >    a nice touch.
> > > > > > > 
> > > > > > >    This is a clean break in backward compatibility.  To be mentioned in
> > > > > > >    release notes, obviously.
> > > > > > > 
> > > > > > > B. Change old machine type in a guest-visible way
> > > > > > > 
> > > > > > >    Depending on the nature of the change and the guest, a guest using it
> > > > > > >    either doesn't notice, copes with it successfully, or fails in
> > > > > > >    guest-specific ways.  If the latter, the failure can be "guest
> > > > > > >    hangs", which is much harder to figure out than A.
> > > > > > > 
> > > > > > >    Unless we can *demonstrate* that nothing bad happens for all the
> > > > > > >    guests people actually use with the old machine types, this is a
> > > > > > >    different kind of backward compatibility break.
> > > > > > > 
> > > > > > >    Demonstrating this is feels infeasible to me, but you're welcome to
> > > > > > >    try.
> > > > > > > 
> > > > > > > I could call the difference between the two a tradeoff, but since we've
> > > > > > > been through this before, I'll be more blunt: choosing B robs Peter (the
> > > > > > > guy with guests where badness happens) to pay Paul (the guy with guests
> > > > > > > that cope).  Paul is saved the inconvenience of having to read release
> > > > > > > notes or his logs, and change machine types.  Peter pays for that with
> > > > > > > figuring out WTF his guests are doing now.
> > > > > > > 
> > > > > > > As a user, I'd pick a clean break in backward compatibility over a hack
> > > > > > > that preserves effective compatibility when it works, but breaks it
> > > > > > > uncleanly when it doesn't.
> > > > > > > 
> > > > > > > As a developer, I'm insisting on it.
> > > > > > > 
> > > > > > > So, if you want B, the onus is on *you* to show us why nothing bad will
> > > > > > > happen.
> > > > > > > 
> > > > > 
> > > > > I agree with the conclusion for option B.  But I think the correct
> > > > > solution is not A, it is to analyse changes, maybe even test, and show
> > > > > that nothing bad can happen.
> > > > 
> > > > Do you volunteer for that work?
> > > 
> > > Nope, sorry. It's your idea, your patchset.
> > 
> > It's your idea. You are the one proposing to waste resources
> > keeping an old machine-type name "working" just because you don't
> > want users (who we don't even know if they actually exist) to
> > update their configurations on a QEMU upgrade.
> > 
> > I am proposing the opposite: dropping support to a feature that
> > people are unlikely to be using, in a very clear way.
> 
> What will happen with installed VMs? Will they break?

They won't start unless the QEMU command-line is changed, because
they are using a feature QEMU won't support anymore. Why is that
a problem?

Why do you want to waste so much time keeping a feature that
people are not even supposed to be using?

> 
> > 
> > > I am saying, look
> > > for some low-hanging fruit.  Find some compat options we can
> > > drop without breaking guests, drop just these.  Are there
> > > options we need for piix anyway? No point in dropping them at
> > > all.
> > > 
> > > For example, the builtin AML can be dropped since we always use
> > > a bios with acpi support now.  It is also trivial to test.
> > > 
> > > Memory layout is probably ok to change.
> > > 
> > > Maybe more.
> > > 
> > > > > 
> > > > > Because A suffers from exactly the same problem if people
> > > > > just blindly switch to a new machine type.
> > > > 
> > > > Anything can happen if people change their configurations
> > > > blindly.
> > > > 
> > > > Nobody should change configuration blindly, and that's also
> > > > why we shouldn't run a different machine when the user is
> > > > asking for an old one. We don't know why the user is asking
> > > > for an old machine and we can't make decisions for the user.
> > > > Management software might know why an old machine is being
> > > > used and might be able to help update the config, but QEMU
> > > > doesn't.
> > > 
> > > What guidance do we provide? Try it and see if it works?  What
> > > exactly do we ask user to test? If QEMU developers can't find
> > > out whether switching a machine type is safe, what hope is
> > > there that management developers can?
> > 
> > Exactly the same guidance vendors already provide for people that
> > want to change machine types today. It depends on who wrote the
> > config files and why, and we can't and shouldn't make any
> > guesses.
> 
> AFAIK that's basically "don't do it".

So, are you saying that changing a machine is a risky thing, but
at the same time you are proposing that QEMU does that silently
for the user?

Really, just because you believe somebody out there is using an
experimental machine-type and is too afraid to change to a newer
one, you want to make QEMU developers waste their time testing
and maintaing old compat code and old machine-types forever?

-- 
Eduardo

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

* Re: [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code
  2016-02-11 15:51               ` Eduardo Habkost
@ 2016-02-11 16:33                 ` Michael S. Tsirkin
  2016-02-11 17:24                   ` Eduardo Habkost
                                     ` (3 more replies)
  0 siblings, 4 replies; 39+ messages in thread
From: Michael S. Tsirkin @ 2016-02-11 16:33 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Marcel Apfelbaum, Laszlo Ersek, qemu-devel, Markus Armbruster,
	Igor Mammedov, Paolo Bonzini, John Snow

On Thu, Feb 11, 2016 at 01:51:30PM -0200, Eduardo Habkost wrote:
> On Sat, Feb 06, 2016 at 08:34:07PM +0200, Michael S. Tsirkin wrote:
> > On Fri, Feb 05, 2016 at 12:46:11PM -0200, Eduardo Habkost wrote:
> > > On Fri, Feb 05, 2016 at 12:14:16AM +0200, Michael S. Tsirkin wrote:
> > > > On Thu, Feb 04, 2016 at 05:09:44PM -0200, Eduardo Habkost wrote:
> > > > > On Thu, Feb 04, 2016 at 08:02:30PM +0200, Michael S. Tsirkin wrote:
> > > > > > On Thu, Feb 04, 2016 at 03:16:17PM -0200, Eduardo Habkost wrote:
> > > > > > > On Thu, Feb 04, 2016 at 06:01:50PM +0200, Michael S. Tsirkin wrote:
> > > > > > > > On Sat, Jan 23, 2016 at 02:02:08PM -0200, Eduardo Habkost wrote:
> > > > > > > > > This is another attempt to remove old q35 machine code. Now I am
> > > > > > > > > also removing unused compat code to demonstrate the benefit of
> > > > > > > > > throwing away the old code that nobody uses.
> > > > > > > > 
> > > > > > > > The same thing I said applies - we don't know that nobody uses old q35
> > > > > > > > machine types.
> > > > > > > > We do know we don't need to migrate to/from them,
> > > > > > > > so we can drop compat code.
> > > > > > > > But please add aliases so people can still start these machines.
> > > > > > > 
> > > > > > > If people use them, they can easily update their configurations.
> > > > > > > I will copy and paste the reply Markus sent 4 months ago below.
> > > > > > > 
> > > > > > > On Mon, Sep 14, 2015 at 09:18:47AM +0200, Markus Armbruster wrote:
> > > > > > > > We've been through this before, but we can go through it once more.
> > > > > > > > Choices:
> > > > > > > > 
> > > > > > > > A. Remove old machine type
> > > > > > > > 
> > > > > > > >    A guest using it can't be started.  Easy to understand on the host.
> > > > > > > >    An error message advising to switch to a newer machine type would be
> > > > > > > >    a nice touch.
> > > > > > > > 
> > > > > > > >    This is a clean break in backward compatibility.  To be mentioned in
> > > > > > > >    release notes, obviously.
> > > > > > > > 
> > > > > > > > B. Change old machine type in a guest-visible way
> > > > > > > > 
> > > > > > > >    Depending on the nature of the change and the guest, a guest using it
> > > > > > > >    either doesn't notice, copes with it successfully, or fails in
> > > > > > > >    guest-specific ways.  If the latter, the failure can be "guest
> > > > > > > >    hangs", which is much harder to figure out than A.
> > > > > > > > 
> > > > > > > >    Unless we can *demonstrate* that nothing bad happens for all the
> > > > > > > >    guests people actually use with the old machine types, this is a
> > > > > > > >    different kind of backward compatibility break.
> > > > > > > > 
> > > > > > > >    Demonstrating this is feels infeasible to me, but you're welcome to
> > > > > > > >    try.
> > > > > > > > 
> > > > > > > > I could call the difference between the two a tradeoff, but since we've
> > > > > > > > been through this before, I'll be more blunt: choosing B robs Peter (the
> > > > > > > > guy with guests where badness happens) to pay Paul (the guy with guests
> > > > > > > > that cope).  Paul is saved the inconvenience of having to read release
> > > > > > > > notes or his logs, and change machine types.  Peter pays for that with
> > > > > > > > figuring out WTF his guests are doing now.
> > > > > > > > 
> > > > > > > > As a user, I'd pick a clean break in backward compatibility over a hack
> > > > > > > > that preserves effective compatibility when it works, but breaks it
> > > > > > > > uncleanly when it doesn't.
> > > > > > > > 
> > > > > > > > As a developer, I'm insisting on it.
> > > > > > > > 
> > > > > > > > So, if you want B, the onus is on *you* to show us why nothing bad will
> > > > > > > > happen.
> > > > > > > > 
> > > > > > 
> > > > > > I agree with the conclusion for option B.  But I think the correct
> > > > > > solution is not A, it is to analyse changes, maybe even test, and show
> > > > > > that nothing bad can happen.
> > > > > 
> > > > > Do you volunteer for that work?
> > > > 
> > > > Nope, sorry. It's your idea, your patchset.
> > > 
> > > It's your idea. You are the one proposing to waste resources
> > > keeping an old machine-type name "working" just because you don't
> > > want users (who we don't even know if they actually exist) to
> > > update their configurations on a QEMU upgrade.
> > > 
> > > I am proposing the opposite: dropping support to a feature that
> > > people are unlikely to be using, in a very clear way.
> > 
> > What will happen with installed VMs? Will they break?
> 
> They won't start unless the QEMU command-line is changed, because
> they are using a feature QEMU won't support anymore. Why is that
> a problem?

We don't support installing one machine type, then switching.
So they won't start unless guest is re-installed.
Not nice.

> Why do you want to waste so much time keeping a feature that
> people are not even supposed to be using?

Why aren't people supposed to use this feature?

> > 
> > > 
> > > > I am saying, look
> > > > for some low-hanging fruit.  Find some compat options we can
> > > > drop without breaking guests, drop just these.  Are there
> > > > options we need for piix anyway? No point in dropping them at
> > > > all.
> > > > 
> > > > For example, the builtin AML can be dropped since we always use
> > > > a bios with acpi support now.  It is also trivial to test.
> > > > 
> > > > Memory layout is probably ok to change.
> > > > 
> > > > Maybe more.
> > > > 
> > > > > > 
> > > > > > Because A suffers from exactly the same problem if people
> > > > > > just blindly switch to a new machine type.
> > > > > 
> > > > > Anything can happen if people change their configurations
> > > > > blindly.
> > > > > 
> > > > > Nobody should change configuration blindly, and that's also
> > > > > why we shouldn't run a different machine when the user is
> > > > > asking for an old one. We don't know why the user is asking
> > > > > for an old machine and we can't make decisions for the user.
> > > > > Management software might know why an old machine is being
> > > > > used and might be able to help update the config, but QEMU
> > > > > doesn't.
> > > > 
> > > > What guidance do we provide? Try it and see if it works?  What
> > > > exactly do we ask user to test? If QEMU developers can't find
> > > > out whether switching a machine type is safe, what hope is
> > > > there that management developers can?
> > > 
> > > Exactly the same guidance vendors already provide for people that
> > > want to change machine types today. It depends on who wrote the
> > > config files and why, and we can't and shouldn't make any
> > > guesses.
> > 
> > AFAIK that's basically "don't do it".
> 
> So, are you saying that changing a machine is a risky thing, but
> at the same time you are proposing that QEMU does that silently
> for the user?

We do this all the time. Any change in qemu changes something.
Compatibility is a question of testing.

> Really, just because you believe somebody out there is using an
> experimental machine-type and is too afraid to change to a newer
> one, you want to make QEMU developers waste their time testing
> and maintaing old compat code and old machine-types forever?

What exactly made it experimental?

> -- 
> Eduardo

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

* Re: [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code
  2016-02-11 16:33                 ` Michael S. Tsirkin
@ 2016-02-11 17:24                   ` Eduardo Habkost
  2016-02-11 20:49                   ` Paolo Bonzini
                                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 39+ messages in thread
From: Eduardo Habkost @ 2016-02-11 17:24 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Marcel Apfelbaum, Laszlo Ersek, qemu-devel, Markus Armbruster,
	Igor Mammedov, Paolo Bonzini, John Snow

On Thu, Feb 11, 2016 at 06:33:14PM +0200, Michael S. Tsirkin wrote:
> On Thu, Feb 11, 2016 at 01:51:30PM -0200, Eduardo Habkost wrote:
> > On Sat, Feb 06, 2016 at 08:34:07PM +0200, Michael S. Tsirkin wrote:
> > > On Fri, Feb 05, 2016 at 12:46:11PM -0200, Eduardo Habkost wrote:
> > > > On Fri, Feb 05, 2016 at 12:14:16AM +0200, Michael S. Tsirkin wrote:
> > > > > On Thu, Feb 04, 2016 at 05:09:44PM -0200, Eduardo Habkost wrote:
> > > > > > On Thu, Feb 04, 2016 at 08:02:30PM +0200, Michael S. Tsirkin wrote:
> > > > > > > On Thu, Feb 04, 2016 at 03:16:17PM -0200, Eduardo Habkost wrote:
> > > > > > > > On Thu, Feb 04, 2016 at 06:01:50PM +0200, Michael S. Tsirkin wrote:
> > > > > > > > > On Sat, Jan 23, 2016 at 02:02:08PM -0200, Eduardo Habkost wrote:
> > > > > > > > > > This is another attempt to remove old q35 machine code. Now I am
> > > > > > > > > > also removing unused compat code to demonstrate the benefit of
> > > > > > > > > > throwing away the old code that nobody uses.
> > > > > > > > > 
> > > > > > > > > The same thing I said applies - we don't know that nobody uses old q35
> > > > > > > > > machine types.
> > > > > > > > > We do know we don't need to migrate to/from them,
> > > > > > > > > so we can drop compat code.
> > > > > > > > > But please add aliases so people can still start these machines.
> > > > > > > > 
> > > > > > > > If people use them, they can easily update their configurations.
> > > > > > > > I will copy and paste the reply Markus sent 4 months ago below.
> > > > > > > > 
> > > > > > > > On Mon, Sep 14, 2015 at 09:18:47AM +0200, Markus Armbruster wrote:
> > > > > > > > > We've been through this before, but we can go through it once more.
> > > > > > > > > Choices:
> > > > > > > > > 
> > > > > > > > > A. Remove old machine type
> > > > > > > > > 
> > > > > > > > >    A guest using it can't be started.  Easy to understand on the host.
> > > > > > > > >    An error message advising to switch to a newer machine type would be
> > > > > > > > >    a nice touch.
> > > > > > > > > 
> > > > > > > > >    This is a clean break in backward compatibility.  To be mentioned in
> > > > > > > > >    release notes, obviously.
> > > > > > > > > 
> > > > > > > > > B. Change old machine type in a guest-visible way
> > > > > > > > > 
> > > > > > > > >    Depending on the nature of the change and the guest, a guest using it
> > > > > > > > >    either doesn't notice, copes with it successfully, or fails in
> > > > > > > > >    guest-specific ways.  If the latter, the failure can be "guest
> > > > > > > > >    hangs", which is much harder to figure out than A.
> > > > > > > > > 
> > > > > > > > >    Unless we can *demonstrate* that nothing bad happens for all the
> > > > > > > > >    guests people actually use with the old machine types, this is a
> > > > > > > > >    different kind of backward compatibility break.
> > > > > > > > > 
> > > > > > > > >    Demonstrating this is feels infeasible to me, but you're welcome to
> > > > > > > > >    try.
> > > > > > > > > 
> > > > > > > > > I could call the difference between the two a tradeoff, but since we've
> > > > > > > > > been through this before, I'll be more blunt: choosing B robs Peter (the
> > > > > > > > > guy with guests where badness happens) to pay Paul (the guy with guests
> > > > > > > > > that cope).  Paul is saved the inconvenience of having to read release
> > > > > > > > > notes or his logs, and change machine types.  Peter pays for that with
> > > > > > > > > figuring out WTF his guests are doing now.
> > > > > > > > > 
> > > > > > > > > As a user, I'd pick a clean break in backward compatibility over a hack
> > > > > > > > > that preserves effective compatibility when it works, but breaks it
> > > > > > > > > uncleanly when it doesn't.
> > > > > > > > > 
> > > > > > > > > As a developer, I'm insisting on it.
> > > > > > > > > 
> > > > > > > > > So, if you want B, the onus is on *you* to show us why nothing bad will
> > > > > > > > > happen.
> > > > > > > > > 
> > > > > > > 
> > > > > > > I agree with the conclusion for option B.  But I think the correct
> > > > > > > solution is not A, it is to analyse changes, maybe even test, and show
> > > > > > > that nothing bad can happen.
> > > > > > 
> > > > > > Do you volunteer for that work?
> > > > > 
> > > > > Nope, sorry. It's your idea, your patchset.
> > > > 
> > > > It's your idea. You are the one proposing to waste resources
> > > > keeping an old machine-type name "working" just because you don't
> > > > want users (who we don't even know if they actually exist) to
> > > > update their configurations on a QEMU upgrade.
> > > > 
> > > > I am proposing the opposite: dropping support to a feature that
> > > > people are unlikely to be using, in a very clear way.
> > > 
> > > What will happen with installed VMs? Will they break?
> > 
> > They won't start unless the QEMU command-line is changed, because
> > they are using a feature QEMU won't support anymore. Why is that
> > a problem?
> 
> We don't support installing one machine type, then switching.
> So they won't start unless guest is re-installed.
> Not nice.

Really? Who is "we"? I believe some vendors support changing the
machine-type when necessary. I'm sure most OSes are capable of
handling a hardware upgrade.

> 
> > Why do you want to waste so much time keeping a feature that
> > people are not even supposed to be using?
> 
> Why aren't people supposed to use this feature?

Because (AFAICS) most developers involved in that code don't want
to waste time supporting it, and we already provide better
alternatives.

> 
> > > 
> > > > 
> > > > > I am saying, look
> > > > > for some low-hanging fruit.  Find some compat options we can
> > > > > drop without breaking guests, drop just these.  Are there
> > > > > options we need for piix anyway? No point in dropping them at
> > > > > all.
> > > > > 
> > > > > For example, the builtin AML can be dropped since we always use
> > > > > a bios with acpi support now.  It is also trivial to test.
> > > > > 
> > > > > Memory layout is probably ok to change.
> > > > > 
> > > > > Maybe more.
> > > > > 
> > > > > > > 
> > > > > > > Because A suffers from exactly the same problem if people
> > > > > > > just blindly switch to a new machine type.
> > > > > > 
> > > > > > Anything can happen if people change their configurations
> > > > > > blindly.
> > > > > > 
> > > > > > Nobody should change configuration blindly, and that's also
> > > > > > why we shouldn't run a different machine when the user is
> > > > > > asking for an old one. We don't know why the user is asking
> > > > > > for an old machine and we can't make decisions for the user.
> > > > > > Management software might know why an old machine is being
> > > > > > used and might be able to help update the config, but QEMU
> > > > > > doesn't.
> > > > > 
> > > > > What guidance do we provide? Try it and see if it works?  What
> > > > > exactly do we ask user to test? If QEMU developers can't find
> > > > > out whether switching a machine type is safe, what hope is
> > > > > there that management developers can?
> > > > 
> > > > Exactly the same guidance vendors already provide for people that
> > > > want to change machine types today. It depends on who wrote the
> > > > config files and why, and we can't and shouldn't make any
> > > > guesses.
> > > 
> > > AFAIK that's basically "don't do it".
> > 
> > So, are you saying that changing a machine is a risky thing, but
> > at the same time you are proposing that QEMU does that silently
> > for the user?
> 
> We do this all the time. Any change in qemu changes something.
> Compatibility is a question of testing.

Testing and reviewing code is costly.

> 
> > Really, just because you believe somebody out there is using an
> > experimental machine-type and is too afraid to change to a newer
> > one, you want to make QEMU developers waste their time testing
> > and maintaing old compat code and old machine-types forever?
> 
> What exactly made it experimental?

The fact that nobody declared it as fully supported.

-- 
Eduardo

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

* Re: [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code
  2016-02-11 16:33                 ` Michael S. Tsirkin
  2016-02-11 17:24                   ` Eduardo Habkost
@ 2016-02-11 20:49                   ` Paolo Bonzini
  2016-02-11 20:49                   ` Paolo Bonzini
  2016-02-12 10:58                   ` Markus Armbruster
  3 siblings, 0 replies; 39+ messages in thread
From: Paolo Bonzini @ 2016-02-11 20:49 UTC (permalink / raw)
  To: Michael S. Tsirkin, Eduardo Habkost
  Cc: Marcel Apfelbaum, Laszlo Ersek, qemu-devel, Markus Armbruster,
	Igor Mammedov, John Snow



On 11/02/2016 17:33, Michael S. Tsirkin wrote:
>> They won't start unless the QEMU command-line is changed, because
>> they are using a feature QEMU won't support anymore. Why is that
>> a problem?
> 
> We don't support installing one machine type, then switching.

Uhm, we definitely support people using qemu directly with no -M option
at all with installed VMs.  That's the same as switching machine types
every time a new QEMU is released.

At least RHEV supports switching machine types, even RHEL6->RHEL7.
There are constraints on when to do that because it affects the whole
datacenter, but it is done routinely because each RHEV release only
supports two machine types IIRC.

Paolo

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

* Re: [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code
  2016-02-11 16:33                 ` Michael S. Tsirkin
  2016-02-11 17:24                   ` Eduardo Habkost
  2016-02-11 20:49                   ` Paolo Bonzini
@ 2016-02-11 20:49                   ` Paolo Bonzini
  2016-02-12 10:58                   ` Markus Armbruster
  3 siblings, 0 replies; 39+ messages in thread
From: Paolo Bonzini @ 2016-02-11 20:49 UTC (permalink / raw)
  To: Michael S. Tsirkin, Eduardo Habkost
  Cc: Marcel Apfelbaum, Laszlo Ersek, qemu-devel, Markus Armbruster,
	Igor Mammedov, John Snow



On 11/02/2016 17:33, Michael S. Tsirkin wrote:
>> They won't start unless the QEMU command-line is changed, because
>> they are using a feature QEMU won't support anymore. Why is that
>> a problem?
> 
> We don't support installing one machine type, then switching.

Uhm, we definitely support people using qemu directly with no -M option
at all with installed VMs.  That's the same as switching machine types
every time a new QEMU is released.

At least RHEV supports switching machine types, even RHEL6->RHEL7.
There are constraints on when to do that because it affects the whole
datacenter, but it is done routinely because each RHEV release only
supports two machine types IIRC.

Paolo

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

* Re: [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code
  2016-02-11 16:33                 ` Michael S. Tsirkin
                                     ` (2 preceding siblings ...)
  2016-02-11 20:49                   ` Paolo Bonzini
@ 2016-02-12 10:58                   ` Markus Armbruster
  2016-02-18 11:18                     ` Markus Armbruster
  3 siblings, 1 reply; 39+ messages in thread
From: Markus Armbruster @ 2016-02-12 10:58 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Eduardo Habkost, Marcel Apfelbaum, John Snow, qemu-devel,
	Paolo Bonzini, Igor Mammedov, Laszlo Ersek

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Thu, Feb 11, 2016 at 01:51:30PM -0200, Eduardo Habkost wrote:
>> On Sat, Feb 06, 2016 at 08:34:07PM +0200, Michael S. Tsirkin wrote:
>> > On Fri, Feb 05, 2016 at 12:46:11PM -0200, Eduardo Habkost wrote:
>> > > On Fri, Feb 05, 2016 at 12:14:16AM +0200, Michael S. Tsirkin wrote:
>> > > > On Thu, Feb 04, 2016 at 05:09:44PM -0200, Eduardo Habkost wrote:
>> > > > > On Thu, Feb 04, 2016 at 08:02:30PM +0200, Michael S. Tsirkin wrote:
>> > > > > > On Thu, Feb 04, 2016 at 03:16:17PM -0200, Eduardo Habkost wrote:
>> > > > > > > On Thu, Feb 04, 2016 at 06:01:50PM +0200, Michael S. Tsirkin wrote:
>> > > > > > > > On Sat, Jan 23, 2016 at 02:02:08PM -0200, Eduardo Habkost wrote:
>> > > > > > > > > This is another attempt to remove old q35 machine code. Now I am
>> > > > > > > > > also removing unused compat code to demonstrate the benefit of
>> > > > > > > > > throwing away the old code that nobody uses.
>> > > > > > > > 
>> > > > > > > > The same thing I said applies - we don't know that nobody uses old q35
>> > > > > > > > machine types.
>> > > > > > > > We do know we don't need to migrate to/from them,
>> > > > > > > > so we can drop compat code.
>> > > > > > > > But please add aliases so people can still start these machines.
>> > > > > > > 
>> > > > > > > If people use them, they can easily update their configurations.
>> > > > > > > I will copy and paste the reply Markus sent 4 months ago below.
>> > > > > > > 
>> > > > > > > On Mon, Sep 14, 2015 at 09:18:47AM +0200, Markus Armbruster wrote:
>> > > > > > > > We've been through this before, but we can go through it once more.
>> > > > > > > > Choices:
>> > > > > > > > 
>> > > > > > > > A. Remove old machine type
>> > > > > > > > 
>> > > > > > > >    A guest using it can't be started.  Easy to understand on the host.
>> > > > > > > >    An error message advising to switch to a newer machine type would be
>> > > > > > > >    a nice touch.
>> > > > > > > > 
>> > > > > > > >    This is a clean break in backward compatibility.  To be mentioned in
>> > > > > > > >    release notes, obviously.
>> > > > > > > > 
>> > > > > > > > B. Change old machine type in a guest-visible way
>> > > > > > > > 
>> > > > > > > >    Depending on the nature of the change and the guest, a guest using it
>> > > > > > > >    either doesn't notice, copes with it successfully, or fails in
>> > > > > > > >    guest-specific ways.  If the latter, the failure can be "guest
>> > > > > > > >    hangs", which is much harder to figure out than A.
>> > > > > > > > 
>> > > > > > > >    Unless we can *demonstrate* that nothing bad happens for all the
>> > > > > > > >    guests people actually use with the old machine types, this is a
>> > > > > > > >    different kind of backward compatibility break.
>> > > > > > > > 
>> > > > > > > >    Demonstrating this is feels infeasible to me, but you're welcome to
>> > > > > > > >    try.
>> > > > > > > > 
>> > > > > > > > I could call the difference between the two a tradeoff, but since we've
>> > > > > > > > been through this before, I'll be more blunt: choosing B robs Peter (the
>> > > > > > > > guy with guests where badness happens) to pay Paul (the guy with guests
>> > > > > > > > that cope).  Paul is saved the inconvenience of having to read release
>> > > > > > > > notes or his logs, and change machine types.  Peter pays for that with
>> > > > > > > > figuring out WTF his guests are doing now.
>> > > > > > > > 
>> > > > > > > > As a user, I'd pick a clean break in backward compatibility over a hack
>> > > > > > > > that preserves effective compatibility when it works, but breaks it
>> > > > > > > > uncleanly when it doesn't.
>> > > > > > > > 
>> > > > > > > > As a developer, I'm insisting on it.
>> > > > > > > > 
>> > > > > > > > So, if you want B, the onus is on *you* to show us why nothing bad will
>> > > > > > > > happen.
>> > > > > > > > 
>> > > > > > 
>> > > > > > I agree with the conclusion for option B.  But I think the correct
>> > > > > > solution is not A, it is to analyse changes, maybe even test, and show
>> > > > > > that nothing bad can happen.
>> > > > > 
>> > > > > Do you volunteer for that work?
>> > > > 
>> > > > Nope, sorry. It's your idea, your patchset.
>> > > 
>> > > It's your idea. You are the one proposing to waste resources
>> > > keeping an old machine-type name "working" just because you don't
>> > > want users (who we don't even know if they actually exist) to
>> > > update their configurations on a QEMU upgrade.
>> > > 
>> > > I am proposing the opposite: dropping support to a feature that
>> > > people are unlikely to be using, in a very clear way.
>> > 
>> > What will happen with installed VMs? Will they break?
>> 
>> They won't start unless the QEMU command-line is changed, because
>> they are using a feature QEMU won't support anymore. Why is that
>> a problem?
>
> We don't support installing one machine type, then switching.
> So they won't start unless guest is re-installed.
> Not nice.

As Paolo said, this isn't true.  You're overstating your case.

I'm going to sound like a broken record, but here goes again: switching
machine types means switching hardware.  You have to power off to do it.
The OS will cope if the new hardware sufficiently similar.

In many cases, you can't know whether it'll cope without testing it.  We
can test only a finite set of (OS, old-hardware, new-hardware) tuples.
Our strategy to deal with this is as follows:

1. We make an effort to ensure switching between old and new machine
   types works for the most common OSes.  This effort is somewhat
   haphazard upstream, because we lack the resources for more systematic
   testing.  There's a reason enterprise downstreams exist.

2. A user who wants maximum guest ABI stability should ask for a
   specific version of the machine type.

   When a user asks for a specific machine type, we better provide
   exactly that, no ifs no buts.  Any guest-visible change is a bug.

   Your proposal to silently provide a newer version of q35 would be
   such a bug.

   We very occasionally cheat and make changes that we think are
   extremely unlikely to be noticed.  This is actually the pragmatic
   backward compatibility strategy we employ across the board: an
   incompatibility can be accepted when we're convinced users won't
   notice, and avoiding it would be too costly.

   To make use of this exception for your proposal, *you* have convince
   us that users won't notice.

   We occasionally drop features that aren't worth their keep.  If a
   replacement exists, we point to it.  Experimental features we even
   drop without prior notice.

   I'll rehash why q35 was experimental until recently below.

3. Users who can accept small ABI variations can use the latest machine
   type of a versioned set, via the version-less alias.  Not asking for
   a machine type is even easier, and gets you the latest version of the
   default machine type, but works only when there is a default machine
   type, and exposes you to a big ABI change when the default machine
   type changes.

>> Why do you want to waste so much time keeping a feature that
>> people are not even supposed to be using?
>
> Why aren't people supposed to use this feature?
>
>
>> > 
>> > > 
>> > > > I am saying, look
>> > > > for some low-hanging fruit.  Find some compat options we can
>> > > > drop without breaking guests, drop just these.  Are there
>> > > > options we need for piix anyway? No point in dropping them at
>> > > > all.
>> > > > 
>> > > > For example, the builtin AML can be dropped since we always use
>> > > > a bios with acpi support now.  It is also trivial to test.
>> > > > 
>> > > > Memory layout is probably ok to change.
>> > > > 
>> > > > Maybe more.
>> > > > 
>> > > > > > 
>> > > > > > Because A suffers from exactly the same problem if people
>> > > > > > just blindly switch to a new machine type.
>> > > > > 
>> > > > > Anything can happen if people change their configurations
>> > > > > blindly.
>> > > > > 
>> > > > > Nobody should change configuration blindly, and that's also
>> > > > > why we shouldn't run a different machine when the user is
>> > > > > asking for an old one. We don't know why the user is asking
>> > > > > for an old machine and we can't make decisions for the user.
>> > > > > Management software might know why an old machine is being
>> > > > > used and might be able to help update the config, but QEMU
>> > > > > doesn't.
>> > > > 
>> > > > What guidance do we provide? Try it and see if it works?  What
>> > > > exactly do we ask user to test? If QEMU developers can't find
>> > > > out whether switching a machine type is safe, what hope is
>> > > > there that management developers can?
>> > > 
>> > > Exactly the same guidance vendors already provide for people that
>> > > want to change machine types today. It depends on who wrote the
>> > > config files and why, and we can't and shouldn't make any
>> > > guesses.
>> > 
>> > AFAIK that's basically "don't do it".
>> 
>> So, are you saying that changing a machine is a risky thing, but
>> at the same time you are proposing that QEMU does that silently
>> for the user?
>
> We do this all the time. Any change in qemu changes something.
> Compatibility is a question of testing.

You are the one proposing to change the old machine types.  The onus is
on *you* to show that your proposed changes won't be noticed and are
worthwhile.

I have no opinion to offer on "won't be noticed", simply because I'm
convinced changing them is an egregious waste of resources.

Eduardo and I don't want to change the old machine types, we want to
*drop* them.  The onus is on us to show that dropping them won't unduly
inconvenience users.  Our arguments:

1. There is no evidence of non-experimental use.

2. There is no sane reason for non-experimental use (see below).

3. Existing users (if any) are better off moving to newer machine types.
   The old machine types provide no stability anyway, so there's no
   extra risk in moving.

4. However, moving them to newer machine types *silently* is
   inappropriate.  Users should be given the chance to assess the risk
   and plan their move.  In the meantime, they can stay on the old QEMU
   version.  Failing attempts to use the old machine types with a
   suitable error message or documentation accomplishes that.

5. Maintaining old machine types that should not be used anymore is a
   pointless waste of resources.

>> Really, just because you believe somebody out there is using an
>> experimental machine-type and is too afraid to change to a newer
>> one, you want to make QEMU developers waste their time testing
>> and maintaing old compat code and old machine-types forever?
>
> What exactly made it experimental?

I've wasted too much time in this thread for today already, so I'll be
brief:

1. Q35 barely worked.  The -hda/-cdrom command line sugar didn't work
   until 2.2.  Chipset emulation was full of bugs until 2.4.  Migration
   didn't work at all until 2.4.  That's were Eduardo proposes to make
   the cut.

2. Until 2.4, we pretty much ignored backward compatibility when
   changing Q35.  For instance, something like commit c8b5b20 would be
   unacceptable for a "real" machine type.

3. The features Q35 provides over i440FX have become usable only
   recently.

Why would anyone go to the trouble to ask for a machine type that
provides only disadvantages for any reason but experimenting with it?

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

* Re: [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code
  2016-02-12 10:58                   ` Markus Armbruster
@ 2016-02-18 11:18                     ` Markus Armbruster
  2016-02-18 12:07                       ` Michael S. Tsirkin
  0 siblings, 1 reply; 39+ messages in thread
From: Markus Armbruster @ 2016-02-18 11:18 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Eduardo Habkost, Marcel Apfelbaum, John Snow, qemu-devel,
	Paolo Bonzini, Igor Mammedov, Laszlo Ersek

Markus Armbruster <armbru@redhat.com> writes:

> "Michael S. Tsirkin" <mst@redhat.com> writes:
>
>> On Thu, Feb 11, 2016 at 01:51:30PM -0200, Eduardo Habkost wrote:
>>> On Sat, Feb 06, 2016 at 08:34:07PM +0200, Michael S. Tsirkin wrote:
>>> > On Fri, Feb 05, 2016 at 12:46:11PM -0200, Eduardo Habkost wrote:
>>> > > On Fri, Feb 05, 2016 at 12:14:16AM +0200, Michael S. Tsirkin wrote:
>>> > > > On Thu, Feb 04, 2016 at 05:09:44PM -0200, Eduardo Habkost wrote:
>>> > > > > On Thu, Feb 04, 2016 at 08:02:30PM +0200, Michael S. Tsirkin wrote:
>>> > > > > > On Thu, Feb 04, 2016 at 03:16:17PM -0200, Eduardo Habkost wrote:
>>> > > > > > > On Thu, Feb 04, 2016 at 06:01:50PM +0200, Michael S. Tsirkin wrote:
>>> > > > > > > > On Sat, Jan 23, 2016 at 02:02:08PM -0200, Eduardo Habkost wrote:
>>> > > > > > > > > This is another attempt to remove old q35 machine code. Now I am
>>> > > > > > > > > also removing unused compat code to demonstrate the benefit of
>>> > > > > > > > > throwing away the old code that nobody uses.
>>> > > > > > > > 
>>> > > > > > > > The same thing I said applies - we don't know that nobody uses old q35
>>> > > > > > > > machine types.
>>> > > > > > > > We do know we don't need to migrate to/from them,
>>> > > > > > > > so we can drop compat code.
>>> > > > > > > > But please add aliases so people can still start these machines.
>>> > > > > > > 
>>> > > > > > > If people use them, they can easily update their configurations.
>>> > > > > > > I will copy and paste the reply Markus sent 4 months ago below.
>>> > > > > > > 
>>> > > > > > > On Mon, Sep 14, 2015 at 09:18:47AM +0200, Markus Armbruster wrote:
>>> > > > > > > > We've been through this before, but we can go through it once more.
>>> > > > > > > > Choices:
>>> > > > > > > > 
>>> > > > > > > > A. Remove old machine type
>>> > > > > > > > 
>>> > > > > > > >    A guest using it can't be started.  Easy to understand on the host.
>>> > > > > > > >    An error message advising to switch to a newer machine type would be
>>> > > > > > > >    a nice touch.
>>> > > > > > > > 
>>> > > > > > > >    This is a clean break in backward compatibility.  To be mentioned in
>>> > > > > > > >    release notes, obviously.
>>> > > > > > > > 
>>> > > > > > > > B. Change old machine type in a guest-visible way
>>> > > > > > > > 
>>> > > > > > > >    Depending on the nature of the change and the guest, a guest using it
>>> > > > > > > >    either doesn't notice, copes with it successfully, or fails in
>>> > > > > > > >    guest-specific ways.  If the latter, the failure can be "guest
>>> > > > > > > >    hangs", which is much harder to figure out than A.
>>> > > > > > > > 
>>> > > > > > > >    Unless we can *demonstrate* that nothing bad happens for all the
>>> > > > > > > >    guests people actually use with the old machine types, this is a
>>> > > > > > > >    different kind of backward compatibility break.
>>> > > > > > > > 
>>> > > > > > > >    Demonstrating this is feels infeasible to me, but you're welcome to
>>> > > > > > > >    try.
>>> > > > > > > > 
>>> > > > > > > > I could call the difference between the two a tradeoff, but since we've
>>> > > > > > > > been through this before, I'll be more blunt: choosing B robs Peter (the
>>> > > > > > > > guy with guests where badness happens) to pay Paul (the guy with guests
>>> > > > > > > > that cope).  Paul is saved the inconvenience of having to read release
>>> > > > > > > > notes or his logs, and change machine types.  Peter pays for that with
>>> > > > > > > > figuring out WTF his guests are doing now.
>>> > > > > > > > 
>>> > > > > > > > As a user, I'd pick a clean break in backward compatibility over a hack
>>> > > > > > > > that preserves effective compatibility when it works, but breaks it
>>> > > > > > > > uncleanly when it doesn't.
>>> > > > > > > > 
>>> > > > > > > > As a developer, I'm insisting on it.
>>> > > > > > > > 
>>> > > > > > > > So, if you want B, the onus is on *you* to show us why nothing bad will
>>> > > > > > > > happen.
>>> > > > > > > > 
>>> > > > > > 
>>> > > > > > I agree with the conclusion for option B.  But I think the correct
>>> > > > > > solution is not A, it is to analyse changes, maybe even test, and show
>>> > > > > > that nothing bad can happen.
>>> > > > > 
>>> > > > > Do you volunteer for that work?
>>> > > > 
>>> > > > Nope, sorry. It's your idea, your patchset.
>>> > > 
>>> > > It's your idea. You are the one proposing to waste resources
>>> > > keeping an old machine-type name "working" just because you don't
>>> > > want users (who we don't even know if they actually exist) to
>>> > > update their configurations on a QEMU upgrade.
>>> > > 
>>> > > I am proposing the opposite: dropping support to a feature that
>>> > > people are unlikely to be using, in a very clear way.
>>> > 
>>> > What will happen with installed VMs? Will they break?
>>> 
>>> They won't start unless the QEMU command-line is changed, because
>>> they are using a feature QEMU won't support anymore. Why is that
>>> a problem?
>>
>> We don't support installing one machine type, then switching.
>> So they won't start unless guest is re-installed.
>> Not nice.
>
> As Paolo said, this isn't true.  You're overstating your case.
>
> I'm going to sound like a broken record, but here goes again: switching
> machine types means switching hardware.  You have to power off to do it.
> The OS will cope if the new hardware sufficiently similar.
>
> In many cases, you can't know whether it'll cope without testing it.  We
> can test only a finite set of (OS, old-hardware, new-hardware) tuples.
> Our strategy to deal with this is as follows:
>
> 1. We make an effort to ensure switching between old and new machine
>    types works for the most common OSes.  This effort is somewhat
>    haphazard upstream, because we lack the resources for more systematic
>    testing.  There's a reason enterprise downstreams exist.
>
> 2. A user who wants maximum guest ABI stability should ask for a
>    specific version of the machine type.
>
>    When a user asks for a specific machine type, we better provide
>    exactly that, no ifs no buts.  Any guest-visible change is a bug.
>
>    Your proposal to silently provide a newer version of q35 would be
>    such a bug.
>
>    We very occasionally cheat and make changes that we think are
>    extremely unlikely to be noticed.  This is actually the pragmatic
>    backward compatibility strategy we employ across the board: an
>    incompatibility can be accepted when we're convinced users won't
>    notice, and avoiding it would be too costly.
>
>    To make use of this exception for your proposal, *you* have convince
>    us that users won't notice.
>
>    We occasionally drop features that aren't worth their keep.  If a
>    replacement exists, we point to it.  Experimental features we even
>    drop without prior notice.
>
>    I'll rehash why q35 was experimental until recently below.
>
> 3. Users who can accept small ABI variations can use the latest machine
>    type of a versioned set, via the version-less alias.  Not asking for
>    a machine type is even easier, and gets you the latest version of the
>    default machine type, but works only when there is a default machine
>    type, and exposes you to a big ABI change when the default machine
>    type changes.
>
>>> Why do you want to waste so much time keeping a feature that
>>> people are not even supposed to be using?
>>
>> Why aren't people supposed to use this feature?
>>
>>
>>> > 
>>> > > 
>>> > > > I am saying, look
>>> > > > for some low-hanging fruit.  Find some compat options we can
>>> > > > drop without breaking guests, drop just these.  Are there
>>> > > > options we need for piix anyway? No point in dropping them at
>>> > > > all.
>>> > > > 
>>> > > > For example, the builtin AML can be dropped since we always use
>>> > > > a bios with acpi support now.  It is also trivial to test.
>>> > > > 
>>> > > > Memory layout is probably ok to change.
>>> > > > 
>>> > > > Maybe more.
>>> > > > 
>>> > > > > > 
>>> > > > > > Because A suffers from exactly the same problem if people
>>> > > > > > just blindly switch to a new machine type.
>>> > > > > 
>>> > > > > Anything can happen if people change their configurations
>>> > > > > blindly.
>>> > > > > 
>>> > > > > Nobody should change configuration blindly, and that's also
>>> > > > > why we shouldn't run a different machine when the user is
>>> > > > > asking for an old one. We don't know why the user is asking
>>> > > > > for an old machine and we can't make decisions for the user.
>>> > > > > Management software might know why an old machine is being
>>> > > > > used and might be able to help update the config, but QEMU
>>> > > > > doesn't.
>>> > > > 
>>> > > > What guidance do we provide? Try it and see if it works?  What
>>> > > > exactly do we ask user to test? If QEMU developers can't find
>>> > > > out whether switching a machine type is safe, what hope is
>>> > > > there that management developers can?
>>> > > 
>>> > > Exactly the same guidance vendors already provide for people that
>>> > > want to change machine types today. It depends on who wrote the
>>> > > config files and why, and we can't and shouldn't make any
>>> > > guesses.
>>> > 
>>> > AFAIK that's basically "don't do it".
>>> 
>>> So, are you saying that changing a machine is a risky thing, but
>>> at the same time you are proposing that QEMU does that silently
>>> for the user?
>>
>> We do this all the time. Any change in qemu changes something.
>> Compatibility is a question of testing.
>
> You are the one proposing to change the old machine types.  The onus is
> on *you* to show that your proposed changes won't be noticed and are
> worthwhile.
>
> I have no opinion to offer on "won't be noticed", simply because I'm
> convinced changing them is an egregious waste of resources.
>
> Eduardo and I don't want to change the old machine types, we want to
> *drop* them.  The onus is on us to show that dropping them won't unduly
> inconvenience users.  Our arguments:
>
> 1. There is no evidence of non-experimental use.
>
> 2. There is no sane reason for non-experimental use (see below).
>
> 3. Existing users (if any) are better off moving to newer machine types.
>    The old machine types provide no stability anyway, so there's no
>    extra risk in moving.
>
> 4. However, moving them to newer machine types *silently* is
>    inappropriate.  Users should be given the chance to assess the risk
>    and plan their move.  In the meantime, they can stay on the old QEMU
>    version.  Failing attempts to use the old machine types with a
>    suitable error message or documentation accomplishes that.
>
> 5. Maintaining old machine types that should not be used anymore is a
>    pointless waste of resources.
>
>>> Really, just because you believe somebody out there is using an
>>> experimental machine-type and is too afraid to change to a newer
>>> one, you want to make QEMU developers waste their time testing
>>> and maintaing old compat code and old machine-types forever?
>>
>> What exactly made it experimental?
>
> I've wasted too much time in this thread for today already, so I'll be
> brief:
>
> 1. Q35 barely worked.  The -hda/-cdrom command line sugar didn't work
>    until 2.2.  Chipset emulation was full of bugs until 2.4.  Migration
>    didn't work at all until 2.4.  That's were Eduardo proposes to make
>    the cut.
>
> 2. Until 2.4, we pretty much ignored backward compatibility when
>    changing Q35.  For instance, something like commit c8b5b20 would be
>    unacceptable for a "real" machine type.
>
> 3. The features Q35 provides over i440FX have become usable only
>    recently.
>
> Why would anyone go to the trouble to ask for a machine type that
> provides only disadvantages for any reason but experimenting with it?

No reply from you since Monday.

You can

(a) Apply Eduardo's series.

(b) Debate its merits.

(c) Declare the debate stuck, and put it on the agenda for the next KVM
    call.

(d) Other (do tell).

What you can't do make the problem go away by ignoring it.  It hasn't
gone away for six months and two releases.

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

* Re: [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code
  2016-02-18 11:18                     ` Markus Armbruster
@ 2016-02-18 12:07                       ` Michael S. Tsirkin
  2016-02-18 12:46                         ` Markus Armbruster
  0 siblings, 1 reply; 39+ messages in thread
From: Michael S. Tsirkin @ 2016-02-18 12:07 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Eduardo Habkost, Marcel Apfelbaum, John Snow, qemu-devel,
	Paolo Bonzini, Igor Mammedov, Laszlo Ersek

On Thu, Feb 18, 2016 at 12:18:04PM +0100, Markus Armbruster wrote:
> No reply from you since Monday.

Yes but I'm flooded with other stuff to review. I'll get back to it,
3 days isn't such a long time.

> You can
> 
> (a) Apply Eduardo's series.
> 
> (b) Debate its merits.
> 
> (c) Declare the debate stuck, and put it on the agenda for the next KVM
>     call.
> 
> (d) Other (do tell).

Thinking it over on the weekend can't hurt, so I intend to do this.
Meanwhile, you could help by reviewing the patchset and sending
acks on list (maybe you did, but I can't find it).

> What you can't do make the problem go away by ignoring it.  It hasn't
> gone away for six months and two releases.

I don't think there's a problem as such - the patchset is just supposed
to simplify maintainance - but I don't intend to ignore the patchset.

-- 
MST

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

* Re: [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code
  2016-01-23 16:02 [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code Eduardo Habkost
                   ` (9 preceding siblings ...)
  2016-02-04 16:01 ` [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code Michael S. Tsirkin
@ 2016-02-18 12:39 ` Markus Armbruster
  2016-02-18 23:17   ` Eduardo Habkost
  2016-02-23 13:00 ` Michael S. Tsirkin
  11 siblings, 1 reply; 39+ messages in thread
From: Markus Armbruster @ 2016-02-18 12:39 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Marcel Apfelbaum, John Snow, Michael S. Tsirkin, qemu-devel,
	Paolo Bonzini, Igor Mammedov, Laszlo Ersek

Eduardo Habkost <ehabkost@redhat.com> writes:

> This is another attempt to remove old q35 machine code. Now I am
> also removing unused compat code to demonstrate the benefit of
> throwing away the old code that nobody uses.
>
> Eduardo Habkost (5):
>   q35: Remove old machine versions
>   machine: Remove no_tco field
>   ich9: Remove enable_tco arguments from init functions
>   q35: Remove unused q35-acpi-dsdt.aml file
>   q35: No need to check gigabyte_align

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Followup questions:

* Can pcmc->pci_enabled be false in pc_q35_init()?

* Likewise, pcmc->smbios_defaults?

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

* Re: [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code
  2016-02-18 12:07                       ` Michael S. Tsirkin
@ 2016-02-18 12:46                         ` Markus Armbruster
  0 siblings, 0 replies; 39+ messages in thread
From: Markus Armbruster @ 2016-02-18 12:46 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Eduardo Habkost, Marcel Apfelbaum, Laszlo Ersek, qemu-devel,
	Igor Mammedov, Paolo Bonzini, John Snow

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Thu, Feb 18, 2016 at 12:18:04PM +0100, Markus Armbruster wrote:
>> No reply from you since Monday.
>
> Yes but I'm flooded with other stuff to review. I'll get back to it,
> 3 days isn't such a long time.
>
>> You can
>> 
>> (a) Apply Eduardo's series.
>> 
>> (b) Debate its merits.
>> 
>> (c) Declare the debate stuck, and put it on the agenda for the next KVM
>>     call.
>> 
>> (d) Other (do tell).
>
> Thinking it over on the weekend can't hurt, so I intend to do this.

We've debated this for six months, so taking a few more days to mull it
over some more won't make a difference.

> Meanwhile, you could help by reviewing the patchset and sending
> acks on list (maybe you did, but I can't find it).

I thought I had sent out my R-by long ago.  Looks like I haven't (my
bad), so I did so just now, after re-checking the patches.

>> What you can't do make the problem go away by ignoring it.  It hasn't
>> gone away for six months and two releases.
>
> I don't think there's a problem as such - the patchset is just supposed
> to simplify maintainance - but I don't intend to ignore the patchset.

Thanks!

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

* Re: [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code
  2016-02-18 12:39 ` Markus Armbruster
@ 2016-02-18 23:17   ` Eduardo Habkost
  2016-02-19  8:29     ` Markus Armbruster
  0 siblings, 1 reply; 39+ messages in thread
From: Eduardo Habkost @ 2016-02-18 23:17 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Marcel Apfelbaum, John Snow, Michael S. Tsirkin, qemu-devel,
	Paolo Bonzini, Igor Mammedov, Laszlo Ersek

On Thu, Feb 18, 2016 at 01:39:39PM +0100, Markus Armbruster wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > This is another attempt to remove old q35 machine code. Now I am
> > also removing unused compat code to demonstrate the benefit of
> > throwing away the old code that nobody uses.
> >
> > Eduardo Habkost (5):
> >   q35: Remove old machine versions
> >   machine: Remove no_tco field
> >   ich9: Remove enable_tco arguments from init functions
> >   q35: Remove unused q35-acpi-dsdt.aml file
> >   q35: No need to check gigabyte_align
> 
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> 
> Followup questions:
> 
> * Can pcmc->pci_enabled be false in pc_q35_init()?

No, but I am trying to keep pc_q35_init() code closer to
pc_piix.c:pc_init1() to make future code unification easier.

> 
> * Likewise, pcmc->smbios_defaults?

Ditto.

However, if other developers think otherwise and would like to
make q35_init() simpler in the meantime, I won't object to
patches removing the "if" lines.

-- 
Eduardo

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

* Re: [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code
  2016-02-18 23:17   ` Eduardo Habkost
@ 2016-02-19  8:29     ` Markus Armbruster
  0 siblings, 0 replies; 39+ messages in thread
From: Markus Armbruster @ 2016-02-19  8:29 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Michael S. Tsirkin, Laszlo Ersek, Marcel Apfelbaum, qemu-devel,
	Igor Mammedov, Paolo Bonzini, John Snow

Eduardo Habkost <ehabkost@redhat.com> writes:

> On Thu, Feb 18, 2016 at 01:39:39PM +0100, Markus Armbruster wrote:
>> Eduardo Habkost <ehabkost@redhat.com> writes:
>> 
>> > This is another attempt to remove old q35 machine code. Now I am
>> > also removing unused compat code to demonstrate the benefit of
>> > throwing away the old code that nobody uses.
>> >
>> > Eduardo Habkost (5):
>> >   q35: Remove old machine versions
>> >   machine: Remove no_tco field
>> >   ich9: Remove enable_tco arguments from init functions
>> >   q35: Remove unused q35-acpi-dsdt.aml file
>> >   q35: No need to check gigabyte_align
>> 
>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>> 
>> Followup questions:
>> 
>> * Can pcmc->pci_enabled be false in pc_q35_init()?
>
> No, but I am trying to keep pc_q35_init() code closer to
> pc_piix.c:pc_init1() to make future code unification easier.
>
>> 
>> * Likewise, pcmc->smbios_defaults?
>
> Ditto.
>
> However, if other developers think otherwise and would like to
> make q35_init() simpler in the meantime, I won't object to
> patches removing the "if" lines.

That's for the people working on this code to decide.

Thanks!

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

* Re: [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code
  2016-01-23 16:02 [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code Eduardo Habkost
                   ` (10 preceding siblings ...)
  2016-02-18 12:39 ` Markus Armbruster
@ 2016-02-23 13:00 ` Michael S. Tsirkin
  11 siblings, 0 replies; 39+ messages in thread
From: Michael S. Tsirkin @ 2016-02-23 13:00 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: Marcel Apfelbaum, Laszlo Ersek, qemu-devel, Markus Armbruster,
	Igor Mammedov, Paolo Bonzini, John Snow

On Sat, Jan 23, 2016 at 02:02:08PM -0200, Eduardo Habkost wrote:
> This is another attempt to remove old q35 machine code. Now I am
> also removing unused compat code to demonstrate the benefit of
> throwing away the old code that nobody uses.
> 
> Eduardo Habkost (5):
>   q35: Remove old machine versions
>   machine: Remove no_tco field
>   ich9: Remove enable_tco arguments from init functions
>   q35: Remove unused q35-acpi-dsdt.aml file
>   q35: No need to check gigabyte_align

Looks like everyone wants this applied, so I'll go along.
Thanks everyone!

>  Makefile                  |   2 +-
>  hw/acpi/ich9.c            |   8 +--
>  hw/i386/pc_q35.c          | 176 +---------------------------------------------
>  hw/isa/lpc_ich9.c         |   4 +-
>  include/hw/acpi/ich9.h    |   1 -
>  include/hw/boards.h       |   1 -
>  include/hw/i386/ich9.h    |   2 +-
>  pc-bios/q35-acpi-dsdt.aml | Bin 7344 -> 0 bytes
>  8 files changed, 9 insertions(+), 185 deletions(-)
>  delete mode 100644 pc-bios/q35-acpi-dsdt.aml
> 
> -- 
> 2.1.0

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

end of thread, other threads:[~2016-02-23 13:00 UTC | newest]

Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-23 16:02 [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code Eduardo Habkost
2016-01-23 16:02 ` [Qemu-devel] [PATCH v2 1/5] q35: Remove old machine versions Eduardo Habkost
2016-01-23 16:02 ` [Qemu-devel] [PATCH v2 2/5] machine: Remove no_tco field Eduardo Habkost
2016-01-23 16:02 ` [Qemu-devel] [PATCH v2 3/5] ich9: Remove enable_tco arguments from init functions Eduardo Habkost
2016-01-23 16:02 ` [Qemu-devel] [PATCH v2 4/5] q35: Remove unused q35-acpi-dsdt.aml file Eduardo Habkost
2016-01-23 16:02 ` [Qemu-devel] [PATCH v2 5/5] q35: No need to check gigabyte_align Eduardo Habkost
2016-01-25 11:27 ` [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code Laszlo Ersek
2016-01-26 10:15   ` Igor Mammedov
2016-01-26 14:49     ` Laszlo Ersek
2016-01-26 10:17 ` Igor Mammedov
2016-01-26 14:59 ` [Qemu-devel] [PATCH 6/5] pc: Remove unused pc_acpi_init() function Eduardo Habkost
2016-01-26 17:27   ` Laszlo Ersek
2016-01-26 14:59 ` [Qemu-devel] [PATCH 7/5] acpi: Remove unused acpi_table_add_builtin() function Eduardo Habkost
2016-01-26 17:27   ` Laszlo Ersek
2016-01-27 17:08   ` Igor Mammedov
2016-02-04 16:01 ` [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code Michael S. Tsirkin
2016-02-04 17:16   ` Eduardo Habkost
2016-02-04 18:02     ` Michael S. Tsirkin
2016-02-04 19:09       ` Eduardo Habkost
2016-02-04 22:14         ` Michael S. Tsirkin
2016-02-05  9:09           ` Markus Armbruster
2016-02-05 14:46           ` Eduardo Habkost
2016-02-06 18:34             ` Michael S. Tsirkin
2016-02-11 15:51               ` Eduardo Habkost
2016-02-11 16:33                 ` Michael S. Tsirkin
2016-02-11 17:24                   ` Eduardo Habkost
2016-02-11 20:49                   ` Paolo Bonzini
2016-02-11 20:49                   ` Paolo Bonzini
2016-02-12 10:58                   ` Markus Armbruster
2016-02-18 11:18                     ` Markus Armbruster
2016-02-18 12:07                       ` Michael S. Tsirkin
2016-02-18 12:46                         ` Markus Armbruster
2016-02-05  8:55       ` Markus Armbruster
2016-02-07 10:17         ` Michael S. Tsirkin
2016-02-08 11:59           ` Markus Armbruster
2016-02-18 12:39 ` Markus Armbruster
2016-02-18 23:17   ` Eduardo Habkost
2016-02-19  8:29     ` Markus Armbruster
2016-02-23 13:00 ` Michael S. Tsirkin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).