Re: [PATCH 1/2] Introduce (x86) CPU model deprecation API
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
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
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
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
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
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