Re: [Qemu-devel] [PATCH 1/3] qapi: SupportStatusInfo struct

[Eduardo Habkost]

On Tue, Apr 30, 2019 at 11:10:06AM +0100, Daniel P. Berrangé wrote:
> On Wed, Apr 24, 2019 at 03:20:36PM -0300, Eduardo Habkost wrote:
> > On Wed, Apr 24, 2019 at 09:26:52AM +0100, Daniel P. Berrangé wrote:
> > > On Tue, Apr 23, 2019 at 06:22:44PM -0300, Eduardo Habkost wrote:
> > > > This struct will be used to represent support and deprecation
> > > > status of QEMU features.
> > > > 
> > > > Signed-off-by: Eduardo Habkost <address@hidden>
> > > > ---
> > > >  qapi/common.json | 24 ++++++++++++++++++++++++
> > > >  1 file changed, 24 insertions(+)
> > > > 
> > > > diff --git a/qapi/common.json b/qapi/common.json
> > > > index 99d313ef3b..b59d0dc66b 100644
> > > > --- a/qapi/common.json
> > > > +++ b/qapi/common.json
> > > > @@ -193,3 +193,27 @@
> > > >               'ppc64', 'riscv32', 'riscv64', 's390x', 'sh4',
> > > >               'sh4eb', 'sparc', 'sparc64', 'tricore', 'unicore32',
> > > >               'x86_64', 'xtensa', 'xtensaeb' ] }
> > > > +
> > > > +##
> > > > +# @SupportStatusInfo:
> > > > +#
> > > > +# Information on support status of a given feature
> > > > +# (e.g. machine type)
> > > > +#
> > > > +# @deprecated: if true, the given feature is deprecated and may be 
> > > > removed
> > > > +#              in future versions of QEMU according to the QEMU 
> > > > deprecation
> > > > +#              policy.
> > > > +#
> > > > +# @status-message: Human readable message describing support status
> > > > +#                  of the feature.
> > > > +#
> > > > +# @suggested-alternative: Optional.  Suggested alternative for a 
> > > > deprecated
> > > > +#                         feature.  For machine types, this should be 
> > > > the name
> > > > +#                         of an available machine-type.
> > > > +#
> > > > +# Since: 4.1
> > > > +##
> > > > +{ 'struct': 'SupportStatusInfo',
> > > > +  'data': { 'deprecated': 'bool',
> > > > +            '*status-message': 'str',
> > > > +            '*suggested-alternative': 'str' } }
> > > 
> > > I see status-message has to be optional, since you're embedding the
> > > struct into another struct and want deprecated==false by default.
> > > 
> > > I'd be inclined to change it to embed a pointer to the struct and
> > > drop the deprecated field, and make both status-message and
> > > suggested-alternative be mandatory. ie a struct  "DeprecationInfo" 
> > > the pointer to which is NULL if not deprecated.
> > 
> > That could be a simple solution if we were sure we would only
> > track deprecation info.  But I would like us to track additional
> > support status on that struct eventually.
> 
> I've no objection to expanding it to capture other info - we'll
> want to change the 'deprecated' field to an enum though. Also,
> in some cases there won't be any real alternative so lets just
> call it 'hint' instead. How about
> 
>     { 'enum':  "SupportStatus",
>       'data': { 'production',  # Secure for use in hostile production env
>                 'development', # No security guarentee, friendly dev only
>               'deprecated',  # To be removed in the future
>     } }

"secure for production" and "deprecated" are independent
variables.  There are deprecated features that are still secure,
and deprecated features that were never supposed to be used in
production in the first place.

> 
>     { 'struct': "SupportStatusInfo",
>        'data': { 'status': 'SupportStatus',
>                  '*info': 'str',
>                '*hint': 'str' } }

"hint" seems nicer than "suggested" or "recommended".  I like it.

-- 
Eduardo