[Qemu-devel] [PATCH v2] hw/i386: Deprecate the machines pc-0.10 to pc-1.2

[Qemu-devel] [PATCH v2] hw/i386: Deprecate the machines pc-0.10 to pc-1.2

[Thomas Huth]

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

Re: [Qemu-devel] [PATCH v2] hw/i386: Deprecate the machines pc-0.10 to pc-1.2

[Thomas Huth]

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

Re: [Qemu-devel] [PATCH v2] hw/i386: Deprecate the machines pc-0.10 to pc-1.2

[Gerd Hoffmann]

  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

Re: [Qemu-devel] [PATCH v2] hw/i386: Deprecate the machines pc-0.10 to pc-1.2

[Igor Mammedov]

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
> 

Re: [Qemu-devel] [PATCH v2] hw/i386: Deprecate the machines pc-0.10 to pc-1.2

[Thomas Huth]

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

Re: [Qemu-devel] [PATCH v2] hw/i386: Deprecate the machines pc-0.10 to pc-1.2

[Thomas Huth]

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

Re: [Qemu-devel] [PATCH v2] hw/i386: Deprecate the machines pc-0.10 to pc-1.2

[Gerd Hoffmann]

  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

Re: [Qemu-devel] [PATCH v2] hw/i386: Deprecate the machines pc-0.10 to pc-1.2

[Michael S. Tsirkin]

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

Re: [Qemu-devel] [PATCH v2] hw/i386: Deprecate the machines pc-0.10 to pc-1.2

[Eduardo Habkost]

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

Re: [Qemu-devel] [PATCH v2] hw/i386: Deprecate the machines pc-0.10 to pc-1.2

[Eduardo Habkost]

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