Re: [Qemu-devel] [RESEND PATCH] q35: Remove old machine versions

[John Snow]

On 12/17/2015 12:27 PM, Eduardo Habkost wrote:
> 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.
> 
> Signed-off-by: Eduardo Habkost <address@hidden>
> ---
> Resubmitting this series, as I didn't see any convincing argument
> to keep the old code around.
> ---
>  hw/i386/pc_q35.c | 163 
> -------------------------------------------------------
>  1 file changed, 163 deletions(-)
> 
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 9a12068..4156f5e 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -277,75 +277,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);
> -    rsdp_in_ram = false;
> -    machine->suppress_vmdesc = true;
> -}
> -
> -static void pc_compat_2_1(MachineState *machine)
> -{
> -    PCMachineState *pcms = PC_MACHINE(machine);
> -
> -    pc_compat_2_2(machine);
> -    pcms->enforce_aligned_dimm = false;
> -    smbios_uuid_encoded = false;
> -    x86_cpu_change_kvm_default("svm", NULL);
> -}
> -
> -static void pc_compat_2_0(MachineState *machine)
> -{
> -    pc_compat_2_1(machine);
> -    smbios_legacy_mode = true;
> -    has_reserved_memory = false;
> -    pc_set_legacy_acpi_data_size();
> -}
> -
> -static void pc_compat_1_7(MachineState *machine)
> -{
> -    pc_compat_2_0(machine);
> -    smbios_defaults = false;
> -    gigabyte_align = false;
> -    option_rom_has_mr = true;
> -    x86_cpu_change_kvm_default("x2apic", NULL);
> -}
> -
> -static void pc_compat_1_6(MachineState *machine)
> -{
> -    pc_compat_1_7(machine);
> -    rom_file_has_mr = false;
> -    has_acpi_build = false;
> -}
> -
> -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) \
>      { \
> @@ -391,97 +322,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;
> -    m->alias = NULL;
> -    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)
> -{
> -    pc_q35_2_3_machine_options(m);
> -    m->hw_version = "2.2.0";
> -    SET_MACHINE_COMPAT(m, PC_COMPAT_2_2);
> -}
> -
> -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)
> -{
> -    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);
> -}
> -
> -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)
> -{
> -    pc_q35_2_1_machine_options(m);
> -    m->hw_version = "2.0.0";
> -    SET_MACHINE_COMPAT(m, PC_COMPAT_2_0);
> -}
> -
> -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)
> -{
> -    pc_q35_2_0_machine_options(m);
> -    m->hw_version = "1.7.0";
> -    m->default_machine_opts = NULL;
> -    SET_MACHINE_COMPAT(m, PC_COMPAT_1_7);
> -}
> -
> -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)
> -{
> -    pc_q35_machine_options(m);
> -    m->hw_version = "1.6.0";
> -    SET_MACHINE_COMPAT(m, PC_COMPAT_1_6);
> -}
> -
> -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);
> 

To recap, the main objection last time was that it might upset existing
libvirt configurations because libvirt saves the machine types in the
XML, and this will cause existing configurations to be unable to boot.

There was some discussion over "fudging" it so that we silently forced
users onto 2.4+, but this was rejected as a bad idea in favor of just
simply breaking anybody relying on pre-2.4 machine types.

So the question becomes:

What do we gain by deleting this? Is cleaning up this code worth
breaking some configurations?