Re: [PATCH v3 2/2] Mark Icelake-Client CPU models deprecated

[Eduardo Habkost]

On Mon, Sep 21, 2020 at 03:45:12PM +0800, Robert Hoo wrote:
> On Fri, 2020-09-18 at 00:20 -0400, Eduardo Habkost wrote:
> > On Fri, Sep 18, 2020 at 10:18:56AM +0800, Robert Hoo wrote:
> > > On Thu, 2020-09-17 at 14:01 -0400, Eduardo Habkost wrote:
> > > > On Wed, Sep 16, 2020 at 04:37:14PM +0800, Robert Hoo wrote:
> > > > > Going to obsolete Icelake-Client CPU models in the future.
> > > > > 
> > > > > Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
> > > > > ---
> > > > > Change log
> > > > > v3:
> > > > > Obsolete in v5.2 --> v5.3.
> > > > > 
> > > > >  target/i386/cpu.c | 10 +++++++++-
> > > > >  1 file changed, 9 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > > > > index 9cb82b7..15c1c00 100644
> > > > > --- a/target/i386/cpu.c
> > > > > +++ b/target/i386/cpu.c
> > > > > @@ -3467,7 +3467,12 @@ static X86CPUDefinition
> > > > > builtin_x86_defs[] =
> > > > > {
> > > > >          .xlevel = 0x80000008,
> > > > >          .model_id = "Intel Core Processor (Icelake)",
> > > > >          .versions = (X86CPUVersionDefinition[]) {
> > > > > -            { .version = 1 },
> > > > > +            {
> > > > > +                .version = 1,
> > > > > +                .deprecated = true,
> > > > > +                .note = "Deprecated. Will be obsoleted in
> > > > > v5.3.
> > > > > Please use "
> > > > > +                        "'Icelake-Server-v1' CPU model",
> > > > 
> > > > What's the difference between "deprecated" and "obsoleted"?
> > > > 
> > > 
> > > Forgive my non-native understanding on English word:-D
> > 
> > No problem!  I'm not a native speaker either.  :-)
> > 
> > > Here is my understanding:
> > > 'Deprecate' is to express strong disapproval on the usage; but, can
> > > still be used if user insists.
> > > 'Obsolete' means not usable anymore.
> > > 
> > > You can feel free to reword the note words.
> > > Perhaps substitute 'removed' for 'obsolete' will be better.
> > 
> > "Removed" would be clearer, yes.  It's probably better to not
> > mention the exact version, and just say it will be removed in
> > the future.
> 
> Then I would tend to agree with your suggestion of no specific
> 'deprecation_note' at all; instead, a general warning message "will be
> removed in the future" in machine_run_board_init().

That would work too, but having "please use Icelake-Server
instead" would reduce user confusion.  It would also be
consistent with how MachineClass does it, and make it easier to
combine those two APIs together in the future.

It's not a hard requirement, though.  I'm OK if you want to keep
it as simple as possible.

> > 
> > Or maybe just make the message shorter and set deprecation_note
> > to "Please use Icelake-Server instead".  The details can be
> > documented in docs/system/deprecated.rst.
> > 
> Prefer documenting detail and model specific deprecation plan in
> docs/system/deprecated.rst.
> 

-- 
Eduardo