* [Qemu-devel] [RESEND PATCH] q35: Remove old machine versions
@ 2015-12-17 17:27 Eduardo Habkost
2015-12-17 17:36 ` John Snow
2015-12-18 9:41 ` Gerd Hoffmann
0 siblings, 2 replies; 5+ messages in thread
From: Eduardo Habkost @ 2015-12-17 17:27 UTC (permalink / raw)
To: qemu-devel
Cc: Victor Kaplansky, Michael S. Tsirkin, Laszlo Ersek,
Marcel Apfelbaum, Markus Armbruster, Dr. David Alan Gilbert,
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.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
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);
--
2.1.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [RESEND PATCH] q35: Remove old machine versions
2015-12-17 17:27 [Qemu-devel] [RESEND PATCH] q35: Remove old machine versions Eduardo Habkost
@ 2015-12-17 17:36 ` John Snow
2015-12-17 17:42 ` Eduardo Habkost
2015-12-18 9:41 ` Gerd Hoffmann
1 sibling, 1 reply; 5+ messages in thread
From: John Snow @ 2015-12-17 17:36 UTC (permalink / raw)
To: Eduardo Habkost, qemu-devel
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 <ehabkost@redhat.com>
> ---
> 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?
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [RESEND PATCH] q35: Remove old machine versions
2015-12-17 17:36 ` John Snow
@ 2015-12-17 17:42 ` Eduardo Habkost
0 siblings, 0 replies; 5+ messages in thread
From: Eduardo Habkost @ 2015-12-17 17:42 UTC (permalink / raw)
To: John Snow
Cc: Victor Kaplansky, Michael S. Tsirkin, Marcel Apfelbaum,
qemu-devel, Markus Armbruster, Gerd Hoffmann, Paolo Bonzini,
Laszlo Ersek, Dr. David Alan Gilbert
On Thu, Dec 17, 2015 at 12:36:11PM -0500, John Snow wrote:
>
>
> 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 <ehabkost@redhat.com>
> > ---
> > 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(-)
> >
[...]
>
> 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?
Developer time. We don't waste time reviewing, refactoring, and
fixing compat code that nobody should ever run. See, for example,
the patch sent by Gerd today:
[PATCH 6/6] q35: skip q35-acpi-dsdt.aml load if not needed
--
Eduardo
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [RESEND PATCH] q35: Remove old machine versions
2015-12-17 17:27 [Qemu-devel] [RESEND PATCH] q35: Remove old machine versions Eduardo Habkost
2015-12-17 17:36 ` John Snow
@ 2015-12-18 9:41 ` Gerd Hoffmann
2015-12-18 17:53 ` Eduardo Habkost
1 sibling, 1 reply; 5+ messages in thread
From: Gerd Hoffmann @ 2015-12-18 9:41 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Victor Kaplansky, Marcel Apfelbaum, John Snow,
Michael S. Tsirkin, Markus Armbruster, qemu-devel, Paolo Bonzini,
Laszlo Ersek, Dr. David Alan Gilbert
On Do, 2015-12-17 at 15:27 -0200, 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.
> -static void pc_compat_1_6(MachineState *machine)
> -{
> - pc_compat_1_7(machine);
> - rom_file_has_mr = false;
> - has_acpi_build = false;
After applying this patch has_acpi_build is always true and can be
dropped, together with some other code elsewhere which depends on
has_acpi_build. The same is probably true for other variables as well
(gigabyte_align?). Guess it's best to do this as one-per-variable
incremental patches.
Reviewed-by: Gerd Hoffmann <kraxel@gmail.com>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [RESEND PATCH] q35: Remove old machine versions
2015-12-18 9:41 ` Gerd Hoffmann
@ 2015-12-18 17:53 ` Eduardo Habkost
0 siblings, 0 replies; 5+ messages in thread
From: Eduardo Habkost @ 2015-12-18 17:53 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Victor Kaplansky, Marcel Apfelbaum, John Snow,
Michael S. Tsirkin, Markus Armbruster, qemu-devel, Paolo Bonzini,
Laszlo Ersek, Dr. David Alan Gilbert
On Fri, Dec 18, 2015 at 10:41:45AM +0100, Gerd Hoffmann wrote:
> On Do, 2015-12-17 at 15:27 -0200, 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.
>
> > -static void pc_compat_1_6(MachineState *machine)
> > -{
> > - pc_compat_1_7(machine);
> > - rom_file_has_mr = false;
> > - has_acpi_build = false;
>
> After applying this patch has_acpi_build is always true and can be
> dropped, together with some other code elsewhere which depends on
> has_acpi_build. The same is probably true for other variables as well
> (gigabyte_align?). Guess it's best to do this as one-per-variable
> incremental patches.
The static variables will be replaced by PCMachineClass fields. See:
Subject: [Qemu-devel] [PATCH v3 0/6] pc: Initialization and compat function cleanup
Date: Tue, 1 Dec 2015 20:58:02 -0200
Message-Id: <1449010688-19205-1-git-send-email-ehabkost@redhat.com>
>
> Reviewed-by: Gerd Hoffmann <kraxel@gmail.com>
Thanks!
--
Eduardo
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-12-18 17:53 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-17 17:27 [Qemu-devel] [RESEND PATCH] q35: Remove old machine versions Eduardo Habkost
2015-12-17 17:36 ` John Snow
2015-12-17 17:42 ` Eduardo Habkost
2015-12-18 9:41 ` Gerd Hoffmann
2015-12-18 17:53 ` Eduardo Habkost
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.