From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47740) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a9cTT-0007vb-Gf for qemu-devel@nongnu.org; Thu, 17 Dec 2015 12:36:20 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a9cTO-00044a-Ct for qemu-devel@nongnu.org; Thu, 17 Dec 2015 12:36:19 -0500 Received: from mx1.redhat.com ([209.132.183.28]:41889) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a9cTO-00044L-5P for qemu-devel@nongnu.org; Thu, 17 Dec 2015 12:36:14 -0500 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (Postfix) with ESMTPS id A951BC0B9338 for ; Thu, 17 Dec 2015 17:36:13 +0000 (UTC) References: <1450373244-27585-1-git-send-email-ehabkost@redhat.com> From: John Snow Message-ID: <5672F28B.5020308@redhat.com> Date: Thu, 17 Dec 2015 12:36:11 -0500 MIME-Version: 1.0 In-Reply-To: <1450373244-27585-1-git-send-email-ehabkost@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RESEND PATCH] q35: Remove old machine versions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost , qemu-devel@nongnu.org Cc: Victor Kaplansky , "Michael S. Tsirkin" , Marcel Apfelbaum , Markus Armbruster , "Dr. David Alan Gilbert" , Paolo Bonzini , Laszlo Ersek 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 > --- > 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?