hw/i386/pc_piix.c | 9 +++++++++ 1 file changed, 9 insertions(+)
We don't want to carry along old machine types forever. If we are able to
remove the pc machines up to 0.13 one day for example, this would allow
us to eventually kill the code for rombar=0 (i.e. where QEMU copies ROM
BARs directly to low memory). Everything up to pc-1.2 is also known to
have issues with migration. So let's start with a deprecation message
for the old machine types so that the (hopefully) few users of these old
systems start switching over to newer machine types instead.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
Note: Even if we mark all these old machines as deprecated, this ofcourse
doesn't mean that we also have to remove them all at once later when we
decide to finally really remove some. We could then also start by removing
0.10 and 0.11 only, for example (since there should really be no users left
for these), or only up to 0.13 (to be able to kill rombar=0).
v2:
- Deprecate machines up to pc-1.2
hw/i386/pc_piix.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 9f102aa..aace378 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -38,6 +38,7 @@
#include "sysemu/kvm.h"
#include "hw/kvm/clock.h"
#include "sysemu/sysemu.h"
+#include "sysemu/qtest.h"
#include "hw/sysbus.h"
#include "sysemu/arch_init.h"
#include "sysemu/block-backend.h"
@@ -84,6 +85,14 @@ static void pc_init1(MachineState *machine,
MemoryRegion *pci_memory;
MemoryRegion *rom_memory;
ram_addr_t lowmem;
+ char *mc_name = MACHINE_CLASS(pcmc)->name;
+
+ /* Machines pc-0.10 up to pc-1.2 are considered as deprecated */
+ if (!qtest_enabled() && (!strncmp(mc_name, "pc-0.", 5)
+ || (!strncmp(mc_name, "pc-1.", 5) && mc_name[5] < '3'))) {
+ error_report("Machine type '%s' is deprecated, "
+ "please use a newer type instead", mc_name);
+ }
/*
* Calculate ram split, for memory below and above 4G. It's a bit
--
1.8.3.1
On 10.05.2017 17:34, Thomas Huth wrote: > We don't want to carry along old machine types forever. If we are able to > remove the pc machines up to 0.13 one day for example, this would allow > us to eventually kill the code for rombar=0 (i.e. where QEMU copies ROM > BARs directly to low memory). Everything up to pc-1.2 is also known to > have issues with migration. So let's start with a deprecation message > for the old machine types so that the (hopefully) few users of these old > systems start switching over to newer machine types instead. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > Note: Even if we mark all these old machines as deprecated, this ofcourse > doesn't mean that we also have to remove them all at once later when we > decide to finally really remove some. We could then also start by removing > 0.10 and 0.11 only, for example (since there should really be no users left > for these), or only up to 0.13 (to be able to kill rombar=0). > > v2: > - Deprecate machines up to pc-1.2 > > hw/i386/pc_piix.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > index 9f102aa..aace378 100644 > --- a/hw/i386/pc_piix.c > +++ b/hw/i386/pc_piix.c > @@ -38,6 +38,7 @@ > #include "sysemu/kvm.h" > #include "hw/kvm/clock.h" > #include "sysemu/sysemu.h" > +#include "sysemu/qtest.h" > #include "hw/sysbus.h" > #include "sysemu/arch_init.h" > #include "sysemu/block-backend.h" > @@ -84,6 +85,14 @@ static void pc_init1(MachineState *machine, > MemoryRegion *pci_memory; > MemoryRegion *rom_memory; > ram_addr_t lowmem; > + char *mc_name = MACHINE_CLASS(pcmc)->name; > + > + /* Machines pc-0.10 up to pc-1.2 are considered as deprecated */ > + if (!qtest_enabled() && (!strncmp(mc_name, "pc-0.", 5) > + || (!strncmp(mc_name, "pc-1.", 5) && mc_name[5] < '3'))) { > + error_report("Machine type '%s' is deprecated, " > + "please use a newer type instead", mc_name); > + } > > /* > * Calculate ram split, for memory below and above 4G. It's a bit > Ping! Any comments on this version of the patch? Thomas
Hi, > > v2: > > - Deprecate machines up to pc-1.2 > > > > hw/i386/pc_piix.c | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > > index 9f102aa..aace378 100644 > > --- a/hw/i386/pc_piix.c > > +++ b/hw/i386/pc_piix.c > > @@ -38,6 +38,7 @@ > > #include "sysemu/kvm.h" > > #include "hw/kvm/clock.h" > > #include "sysemu/sysemu.h" > > +#include "sysemu/qtest.h" > > #include "hw/sysbus.h" > > #include "sysemu/arch_init.h" > > #include "sysemu/block-backend.h" > > @@ -84,6 +85,14 @@ static void pc_init1(MachineState *machine, > > MemoryRegion *pci_memory; > > MemoryRegion *rom_memory; > > ram_addr_t lowmem; > > + char *mc_name = MACHINE_CLASS(pcmc)->name; > > + > > + /* Machines pc-0.10 up to pc-1.2 are considered as deprecated > > */ > > + if (!qtest_enabled() && (!strncmp(mc_name, "pc-0.", 5) > > + || (!strncmp(mc_name, "pc-1.", 5) && mc_name[5] < '3'))) { > > + error_report("Machine type '%s' is deprecated, " > > + "please use a newer type instead", mc_name); > > + } > > > > /* > > * Calculate ram split, for memory below and above 4G. It's a > > bit > > > > Ping! Any comments on this version of the patch? Does it make sense to build some infrastructure for this, so we don't have ad-hoc code to print deprecation warnings everywhere? Something like adding a "bool deprecated" or "char *deprecated_msg" field to MachineClass (and possibly elsewhere too, for example DeviceClass). cheers, Gerd
On Tue, 30 May 2017 12:35:01 +0200 Gerd Hoffmann <kraxel@redhat.com> wrote: > Hi, > > > > v2: > > > - Deprecate machines up to pc-1.2 > > > > > > hw/i386/pc_piix.c | 9 +++++++++ > > > 1 file changed, 9 insertions(+) > > > > > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > > > index 9f102aa..aace378 100644 > > > --- a/hw/i386/pc_piix.c > > > +++ b/hw/i386/pc_piix.c > > > @@ -38,6 +38,7 @@ > > > #include "sysemu/kvm.h" > > > #include "hw/kvm/clock.h" > > > #include "sysemu/sysemu.h" > > > +#include "sysemu/qtest.h" > > > #include "hw/sysbus.h" > > > #include "sysemu/arch_init.h" > > > #include "sysemu/block-backend.h" > > > @@ -84,6 +85,14 @@ static void pc_init1(MachineState *machine, > > > MemoryRegion *pci_memory; > > > MemoryRegion *rom_memory; > > > ram_addr_t lowmem; > > > + char *mc_name = MACHINE_CLASS(pcmc)->name; > > > + > > > + /* Machines pc-0.10 up to pc-1.2 are considered as deprecated > > > */ > > > + if (!qtest_enabled() && (!strncmp(mc_name, "pc-0.", 5) > > > + || (!strncmp(mc_name, "pc-1.", 5) && mc_name[5] < '3'))) { > > > + error_report("Machine type '%s' is deprecated, " > > > + "please use a newer type instead", mc_name); > > > + } > > > > > > /* > > > * Calculate ram split, for memory below and above 4G. It's a > > > bit > > > > > > > Ping! Any comments on this version of the patch? > > Does it make sense to build some infrastructure for this, so we don't > have ad-hoc code to print deprecation warnings everywhere? we are deprecating incomplete numa mapping but it uses 'obsoleted' word somewhere in warning message (in machine_numa_finish_init). it could be better if if we had error_report_depricated() which would add prefix deprecated prefix consistently something like: "DEPRECATED: " > Something like adding a "bool deprecated" or "char *deprecated_msg" > field to MachineClass (and possibly elsewhere too, for example > DeviceClass). > > cheers, > Gerd >
On 30.05.2017 13:02, Igor Mammedov wrote: > On Tue, 30 May 2017 12:35:01 +0200 > Gerd Hoffmann <kraxel@redhat.com> wrote: > >> Hi, >> >>>> v2: >>>> - Deprecate machines up to pc-1.2 >>>> >>>> hw/i386/pc_piix.c | 9 +++++++++ >>>> 1 file changed, 9 insertions(+) >>>> >>>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c >>>> index 9f102aa..aace378 100644 >>>> --- a/hw/i386/pc_piix.c >>>> +++ b/hw/i386/pc_piix.c >>>> @@ -38,6 +38,7 @@ >>>> #include "sysemu/kvm.h" >>>> #include "hw/kvm/clock.h" >>>> #include "sysemu/sysemu.h" >>>> +#include "sysemu/qtest.h" >>>> #include "hw/sysbus.h" >>>> #include "sysemu/arch_init.h" >>>> #include "sysemu/block-backend.h" >>>> @@ -84,6 +85,14 @@ static void pc_init1(MachineState *machine, >>>> MemoryRegion *pci_memory; >>>> MemoryRegion *rom_memory; >>>> ram_addr_t lowmem; >>>> + char *mc_name = MACHINE_CLASS(pcmc)->name; >>>> + >>>> + /* Machines pc-0.10 up to pc-1.2 are considered as deprecated >>>> */ >>>> + if (!qtest_enabled() && (!strncmp(mc_name, "pc-0.", 5) >>>> + || (!strncmp(mc_name, "pc-1.", 5) && mc_name[5] < '3'))) { >>>> + error_report("Machine type '%s' is deprecated, " >>>> + "please use a newer type instead", mc_name); >>>> + } >>>> >>>> /* >>>> * Calculate ram split, for memory below and above 4G. It's a >>>> bit >>>> >>> >>> Ping! Any comments on this version of the patch? >> >> Does it make sense to build some infrastructure for this, so we don't >> have ad-hoc code to print deprecation warnings everywhere? > we are deprecating incomplete numa mapping but it uses 'obsoleted' word > somewhere in warning message (in machine_numa_finish_init). > > it could be better if if we had error_report_depricated() > which would add prefix deprecated prefix consistently > something like: "DEPRECATED: " I don't mind the exact wording as long as we present a message to the users to inform them that the interface might get removed soon... But if you feel that we need to unify all these spots with an error_report_deprecated() function, feel free to suggest a patch on the list! Thomas
On 30.05.2017 12:35, Gerd Hoffmann wrote: > Hi, > >>> v2: >>> - Deprecate machines up to pc-1.2 >>> >>> hw/i386/pc_piix.c | 9 +++++++++ >>> 1 file changed, 9 insertions(+) >>> >>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c >>> index 9f102aa..aace378 100644 >>> --- a/hw/i386/pc_piix.c >>> +++ b/hw/i386/pc_piix.c >>> @@ -38,6 +38,7 @@ >>> #include "sysemu/kvm.h" >>> #include "hw/kvm/clock.h" >>> #include "sysemu/sysemu.h" >>> +#include "sysemu/qtest.h" >>> #include "hw/sysbus.h" >>> #include "sysemu/arch_init.h" >>> #include "sysemu/block-backend.h" >>> @@ -84,6 +85,14 @@ static void pc_init1(MachineState *machine, >>> MemoryRegion *pci_memory; >>> MemoryRegion *rom_memory; >>> ram_addr_t lowmem; >>> + char *mc_name = MACHINE_CLASS(pcmc)->name; >>> + >>> + /* Machines pc-0.10 up to pc-1.2 are considered as deprecated >>> */ >>> + if (!qtest_enabled() && (!strncmp(mc_name, "pc-0.", 5) >>> + || (!strncmp(mc_name, "pc-1.", 5) && mc_name[5] < '3'))) { >>> + error_report("Machine type '%s' is deprecated, " >>> + "please use a newer type instead", mc_name); >>> + } >>> >>> /* >>> * Calculate ram split, for memory below and above 4G. It's a >>> bit >>> >> >> Ping! Any comments on this version of the patch? > > Does it make sense to build some infrastructure for this, so we don't > have ad-hoc code to print deprecation warnings everywhere? > > Something like adding a "bool deprecated" or "char *deprecated_msg" > field to MachineClass (and possibly elsewhere too, for example > DeviceClass). So far we've got deprecation warnings at very different places in the code - for command line parameters, for HMP commands, for some few devices, ... and this time it's for the first time for old machine types. I currently don't see a pattern yet where a "deprecated" flag in MachineClass or DeviceClass would really help to simplify the code right now ... maybe later if we deprecate multiple machines at the same time? Thomas
Hi, > So far we've got deprecation warnings at very different places in the > code - for command line parameters, for HMP commands, for some few > devices, ... and this time it's for the first time for old machine > types. I currently don't see a pattern yet where a "deprecated" flag > in > MachineClass or DeviceClass would really help to simplify the code > right > now ... maybe later if we deprecate multiple machines at the same > time? Advantage would not only be a simpler code, but also to formalize the deprecation process a bit, especially for cases where we most likely will continue deprecate stuff (like old, versioned machine types) in the future. It would make it easier to grep for deprecated code. And optionally we could put some meta info into the source code, such as planned removal date and removal reason, which could be printed together with the deprecation message. cheers, Gerd
On Tue, May 30, 2017 at 12:35:01PM +0200, Gerd Hoffmann wrote: > Hi, > > > > v2: > > > - Deprecate machines up to pc-1.2 > > > > > > hw/i386/pc_piix.c | 9 +++++++++ > > > 1 file changed, 9 insertions(+) > > > > > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > > > index 9f102aa..aace378 100644 > > > --- a/hw/i386/pc_piix.c > > > +++ b/hw/i386/pc_piix.c > > > @@ -38,6 +38,7 @@ > > > #include "sysemu/kvm.h" > > > #include "hw/kvm/clock.h" > > > #include "sysemu/sysemu.h" > > > +#include "sysemu/qtest.h" > > > #include "hw/sysbus.h" > > > #include "sysemu/arch_init.h" > > > #include "sysemu/block-backend.h" > > > @@ -84,6 +85,14 @@ static void pc_init1(MachineState *machine, > > > MemoryRegion *pci_memory; > > > MemoryRegion *rom_memory; > > > ram_addr_t lowmem; > > > + char *mc_name = MACHINE_CLASS(pcmc)->name; > > > + > > > + /* Machines pc-0.10 up to pc-1.2 are considered as deprecated > > > */ > > > + if (!qtest_enabled() && (!strncmp(mc_name, "pc-0.", 5) > > > + || (!strncmp(mc_name, "pc-1.", 5) && mc_name[5] < '3'))) { > > > + error_report("Machine type '%s' is deprecated, " > > > + "please use a newer type instead", mc_name); > > > + } > > > > > > /* > > > * Calculate ram split, for memory below and above 4G. It's a > > > bit > > > > > > > Ping! Any comments on this version of the patch? > > Does it make sense to build some infrastructure for this, so we don't > have ad-hoc code to print deprecation warnings everywhere? > > Something like adding a "bool deprecated" or "char *deprecated_msg" > field to MachineClass (and possibly elsewhere too, for example > DeviceClass). I would prefer that. And as our class_init (pc_*_machine_options()) functions already call their successors, we would need only one deprecated=true line in the code (at pc_i440fx_1_3_machine_options()). It's probably a good idea to report the deprecated flag on the 'query-machines' QMP command too. -- Eduardo
On Wed, May 10, 2017 at 05:34:53PM +0200, Thomas Huth wrote: > We don't want to carry along old machine types forever. If we are able to > remove the pc machines up to 0.13 one day for example, this would allow > us to eventually kill the code for rombar=0 (i.e. where QEMU copies ROM > BARs directly to low memory). Everything up to pc-1.2 is also known to > have issues with migration. So let's start with a deprecation message > for the old machine types so that the (hopefully) few users of these old > systems start switching over to newer machine types instead. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > Note: Even if we mark all these old machines as deprecated, this ofcourse > doesn't mean that we also have to remove them all at once later when we > decide to finally really remove some. We could then also start by removing > 0.10 and 0.11 only, for example (since there should really be no users left > for these), or only up to 0.13 (to be able to kill rombar=0). > > v2: > - Deprecate machines up to pc-1.2 > > hw/i386/pc_piix.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > index 9f102aa..aace378 100644 > --- a/hw/i386/pc_piix.c > +++ b/hw/i386/pc_piix.c > @@ -38,6 +38,7 @@ > #include "sysemu/kvm.h" > #include "hw/kvm/clock.h" > #include "sysemu/sysemu.h" > +#include "sysemu/qtest.h" > #include "hw/sysbus.h" > #include "sysemu/arch_init.h" > #include "sysemu/block-backend.h" > @@ -84,6 +85,14 @@ static void pc_init1(MachineState *machine, > MemoryRegion *pci_memory; > MemoryRegion *rom_memory; > ram_addr_t lowmem; > + char *mc_name = MACHINE_CLASS(pcmc)->name; > + > + /* Machines pc-0.10 up to pc-1.2 are considered as deprecated */ > + if (!qtest_enabled() && (!strncmp(mc_name, "pc-0.", 5) > + || (!strncmp(mc_name, "pc-1.", 5) && mc_name[5] < '3'))) { > + error_report("Machine type '%s' is deprecated, " > + "please use a newer type instead", mc_name); > + } > > /* > * Calculate ram split, for memory below and above 4G. It's a bit Can't we use a standard compat property machinery for this? Set a "deprecated" property, then query it in a central place. We also want a way for management to get this info. > -- > 1.8.3.1
On Tue, May 30, 2017 at 05:24:55PM +0300, Michael S. Tsirkin wrote: > On Wed, May 10, 2017 at 05:34:53PM +0200, Thomas Huth wrote: > > We don't want to carry along old machine types forever. If we are able to > > remove the pc machines up to 0.13 one day for example, this would allow > > us to eventually kill the code for rombar=0 (i.e. where QEMU copies ROM > > BARs directly to low memory). Everything up to pc-1.2 is also known to > > have issues with migration. So let's start with a deprecation message > > for the old machine types so that the (hopefully) few users of these old > > systems start switching over to newer machine types instead. > > > > Signed-off-by: Thomas Huth <thuth@redhat.com> > > --- > > Note: Even if we mark all these old machines as deprecated, this ofcourse > > doesn't mean that we also have to remove them all at once later when we > > decide to finally really remove some. We could then also start by removing > > 0.10 and 0.11 only, for example (since there should really be no users left > > for these), or only up to 0.13 (to be able to kill rombar=0). > > > > v2: > > - Deprecate machines up to pc-1.2 > > > > hw/i386/pc_piix.c | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > > index 9f102aa..aace378 100644 > > --- a/hw/i386/pc_piix.c > > +++ b/hw/i386/pc_piix.c > > @@ -38,6 +38,7 @@ > > #include "sysemu/kvm.h" > > #include "hw/kvm/clock.h" > > #include "sysemu/sysemu.h" > > +#include "sysemu/qtest.h" > > #include "hw/sysbus.h" > > #include "sysemu/arch_init.h" > > #include "sysemu/block-backend.h" > > @@ -84,6 +85,14 @@ static void pc_init1(MachineState *machine, > > MemoryRegion *pci_memory; > > MemoryRegion *rom_memory; > > ram_addr_t lowmem; > > + char *mc_name = MACHINE_CLASS(pcmc)->name; > > + > > + /* Machines pc-0.10 up to pc-1.2 are considered as deprecated */ > > + if (!qtest_enabled() && (!strncmp(mc_name, "pc-0.", 5) > > + || (!strncmp(mc_name, "pc-1.", 5) && mc_name[5] < '3'))) { > > + error_report("Machine type '%s' is deprecated, " > > + "please use a newer type instead", mc_name); > > + } > > > > /* > > * Calculate ram split, for memory below and above 4G. It's a bit > > Can't we use a standard compat property machinery for this? > Set a "deprecated" property, then query it in a central place. > > We also want a way for management to get this info. There's probably no need for QOM-based compat properties, but a simple MachineClass struct field (and a new MachineInfo field on QMP) would work. -- Eduardo
© 2016 - 2024 Red Hat, Inc.