Re: [PATCH 1/2] Introduce (x86) CPU model deprecation API

2020-06-05 Thread Robert Hoo
On Fri, 2020-06-05 at 08:47 -0500, Eric Blake wrote:
> On 6/4/20 9:47 PM, Robert Hoo wrote:
> > On Thu, 2020-06-04 at 06:59 -0500, Eric Blake wrote:
> > > On 6/4/20 3:07 AM, Robert Hoo wrote:
> > > 
> > > > > > +++ b/qapi/machine-target.json
> > > > > > @@ -309,7 +309,8 @@
> > > > > > 'static': 'bool',
> > > > > > '*unavailable-features': [ 'str' ],
> > > > > > 'typename': 'str',
> > > > > > -'*alias-of' : 'str' },
> > > > > > +'*alias-of' : 'str',
> > > > > > +'deprecated' : 'bool' },
> > > > > 
> > > > > Missing documentation of the new member.  Should it be
> > > > > optional
> > > > > (present
> > > > > only when true)?
> > > > 
> > > > Which document do you mean?
> > 
> > Thanks Eric:)
> > 
> > > 
> > > A few lines earlier is '@alias-of: ...'; you'll need to add a
> > > similar
> > > line for '@deprecated', mentioning it is '(since 5.1)'.
> > > 
> > > > How to make it optional?
> > 
> > How about not making it optional? refer to Machineinfo::deprecated.
> 
> Always providing it doesn't hurt.  If there is precedence for not
> making 
> it optional, mentioning that precedence in the commit message can't
> hurt.

No specific precedence. Just feel a little weird that adding an
additional boolean, just for judging another boolean should present or
not. esp. given that Machineinfo::deprecated is not optional.
> 




Re: [PATCH 1/2] Introduce (x86) CPU model deprecation API

2020-06-05 Thread Eric Blake

On 6/4/20 9:47 PM, Robert Hoo wrote:

On Thu, 2020-06-04 at 06:59 -0500, Eric Blake wrote:

On 6/4/20 3:07 AM, Robert Hoo wrote:


+++ b/qapi/machine-target.json
@@ -309,7 +309,8 @@
'static': 'bool',
'*unavailable-features': [ 'str' ],
'typename': 'str',
-'*alias-of' : 'str' },
+'*alias-of' : 'str',
+'deprecated' : 'bool' },


Missing documentation of the new member.  Should it be optional
(present
only when true)?


Which document do you mean?


Thanks Eric:)



A few lines earlier is '@alias-of: ...'; you'll need to add a
similar
line for '@deprecated', mentioning it is '(since 5.1)'.


How to make it optional?


How about not making it optional? refer to Machineinfo::deprecated.


Always providing it doesn't hurt.  If there is precedence for not making 
it optional, mentioning that precedence in the commit message can't hurt.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org




Re: [PATCH 1/2] Introduce (x86) CPU model deprecation API

2020-06-04 Thread Robert Hoo
On Thu, 2020-06-04 at 06:59 -0500, Eric Blake wrote:
> On 6/4/20 3:07 AM, Robert Hoo wrote:
> 
> > > > +++ b/qapi/machine-target.json
> > > > @@ -309,7 +309,8 @@
> > > >'static': 'bool',
> > > >'*unavailable-features': [ 'str' ],
> > > >'typename': 'str',
> > > > -'*alias-of' : 'str' },
> > > > +'*alias-of' : 'str',
> > > > +'deprecated' : 'bool' },
> > > 
> > > Missing documentation of the new member.  Should it be optional
> > > (present
> > > only when true)?
> > 
> > Which document do you mean?

Thanks Eric:)

> 
> A few lines earlier is '@alias-of: ...'; you'll need to add a
> similar 
> line for '@deprecated', mentioning it is '(since 5.1)'.
> 
> > How to make it optional?

How about not making it optional? refer to Machineinfo::deprecated.
> 
> Name it '*deprecated', then deal with 'has_deprecated' in the C code
> for 
> the cases where the member should be output.
> 




Re: [PATCH 1/2] Introduce (x86) CPU model deprecation API

2020-06-04 Thread Eric Blake

On 6/4/20 3:07 AM, Robert Hoo wrote:


+++ b/qapi/machine-target.json
@@ -309,7 +309,8 @@
   'static': 'bool',
   '*unavailable-features': [ 'str' ],
   'typename': 'str',
-'*alias-of' : 'str' },
+'*alias-of' : 'str',
+'deprecated' : 'bool' },


Missing documentation of the new member.  Should it be optional
(present
only when true)?

Which document do you mean?


A few lines earlier is '@alias-of: ...'; you'll need to add a similar 
line for '@deprecated', mentioning it is '(since 5.1)'.



How to make it optional?


Name it '*deprecated', then deal with 'has_deprecated' in the C code for 
the cases where the member should be output.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org




Re: [PATCH 1/2] Introduce (x86) CPU model deprecation API

2020-06-04 Thread Robert Hoo
On Wed, 2020-06-03 at 09:11 -0500, Eric Blake wrote:
> On 6/3/20 6:47 AM, Robert Hoo wrote:
> > Complement versioned CPU model framework with the ability of
> > marking some
> > versions deprecated. When that CPU model is chosen, get some
> > warning. The
> > warning message is customized, e.g. telling in which future QEMU
> > version will
> > it be obsoleted.
> > The deprecation message will also appear by x86_cpu_list_entry(),
> > e.g. '-cpu
> > help'.
> > QMP 'query-cpu-definitions' will also return a bool value
> > indicating the
> > deprecation status.
> > 
> > Signed-off-by: Robert Hoo 
> > ---
> >   exec.c   |  3 +++
> >   include/hw/core/cpu.h|  1 +
> >   qapi/machine-target.json |  3 ++-
> >   target/i386/cpu.c| 45
> > +++--
> >   4 files changed, 49 insertions(+), 3 deletions(-)
> > +++ b/qapi/machine-target.json
> > @@ -309,7 +309,8 @@
> >   'static': 'bool',
> >   '*unavailable-features': [ 'str' ],
> >   'typename': 'str',
> > -'*alias-of' : 'str' },
> > +'*alias-of' : 'str',
> > +'deprecated' : 'bool' },
> 
> Missing documentation of the new member.  Should it be optional
> (present 
> only when true)?
Which document do you mean?
How to make it optional?
(Sorry, new to QMP)
> 
> > @@ -1638,6 +1639,11 @@ struct X86CPUModel {
> >* This matters only for "-cpu help" and query-cpu-
> > definitions
> >*/
> >   bool is_alias;
> > +/*
> > + * If true, this is deprecated and obsoleted in the future.
> > + * Trying to use deprecated CPU model shall be warned.
> 
> If true, this model is deprecated, and may be removed in the future. 
> Trying to use it now will cause a warning.
Thanks Eric:)
> 
> > + */
> > +bool deprecated;
> >   };
> >   
> 
> 




Re: [PATCH 1/2] Introduce (x86) CPU model deprecation API

2020-06-03 Thread Eric Blake

On 6/3/20 6:47 AM, Robert Hoo wrote:

Complement versioned CPU model framework with the ability of marking some
versions deprecated. When that CPU model is chosen, get some warning. The
warning message is customized, e.g. telling in which future QEMU version will
it be obsoleted.
The deprecation message will also appear by x86_cpu_list_entry(), e.g. '-cpu
help'.
QMP 'query-cpu-definitions' will also return a bool value indicating the
deprecation status.

Signed-off-by: Robert Hoo 
---
  exec.c   |  3 +++
  include/hw/core/cpu.h|  1 +
  qapi/machine-target.json |  3 ++-
  target/i386/cpu.c| 45 +++--
  4 files changed, 49 insertions(+), 3 deletions(-)



+++ b/qapi/machine-target.json
@@ -309,7 +309,8 @@
  'static': 'bool',
  '*unavailable-features': [ 'str' ],
  'typename': 'str',
-'*alias-of' : 'str' },
+'*alias-of' : 'str',
+'deprecated' : 'bool' },


Missing documentation of the new member.  Should it be optional (present 
only when true)?



@@ -1638,6 +1639,11 @@ struct X86CPUModel {
   * This matters only for "-cpu help" and query-cpu-definitions
   */
  bool is_alias;
+/*
+ * If true, this is deprecated and obsoleted in the future.
+ * Trying to use deprecated CPU model shall be warned.


If true, this model is deprecated, and may be removed in the future. 
Trying to use it now will cause a warning.



+ */
+bool deprecated;
  };
  



--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org