| Message ID | 20190423212246.3542-2-ehabkost@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Export machine type deprecation info through QMP | expand |
Hi Eduardo, On 4/23/19 11:22 PM, Eduardo Habkost wrote: > This struct will be used to represent support and deprecation > status of QEMU features. Excellent idea! > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > 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) Hopefully it is generic, we can use it for devices too. > +# > +# @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', This one shouldn't be optional IMHO, we should enforce developpers to give a reason for their deprecation. With this argument not optional: Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > + '*suggested-alternative': 'str' } } >
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 <ehabkost@redhat.com> > --- > 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. Regards, Daniel
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 <ehabkost@redhat.com> > > --- > > 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. Also, I'd like to explicitly differentiate "information is not available because QEMU is old" from "information is available and machine type is not deprecated".
On Wed, Apr 24, 2019 at 12:23:33AM +0200, Philippe Mathieu-Daudé wrote: > Hi Eduardo, > > On 4/23/19 11:22 PM, Eduardo Habkost wrote: > > This struct will be used to represent support and deprecation > > status of QEMU features. > > Excellent idea! > > > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > --- > > 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) > > Hopefully it is generic, we can use it for devices too. Yes, I want to keep it generic. > > > +# > > +# @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', > > This one shouldn't be optional IMHO, we should enforce developpers to > give a reason for their deprecation. > > With this argument not optional: > > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> I might agree with you if you show me what would be a good "reason for deprecation" string for pc-0.15 and spapr. :) The only information contained in these machines today is "use another machine instead", and this is already encoded in the suggested-alternative field.
Hi Eduardo, On 04/23/2019 06:22 PM, Eduardo Habkost wrote: > This struct will be used to represent support and deprecation > status of QEMU features. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > 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. Eventually management software will need the know the QEMU version the feature is planed for removal. So makes sense to include a field to capture that information as well or do you expect it to be added (as a good practice) in the 'status-message'? - Wainer > +# > +# @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' } }
On Thu, Apr 25, 2019 at 11:20:58AM -0300, Wainer dos Santos Moschetta wrote: > Hi Eduardo, > > > On 04/23/2019 06:22 PM, Eduardo Habkost wrote: > > This struct will be used to represent support and deprecation > > status of QEMU features. > > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > --- > > 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. > > Eventually management software will need the know the QEMU version the > feature is planed for removal. So makes sense to include a field to capture > that information as well or do you expect it to be added (as a good > practice) in the 'status-message'? If we really want to provide extra information like version numbers, adding a separate field sounds better than using status-message. But I'm not sure we really want to include this amount of detail in the API. Mentioning explicit version numbers could make things more complex for downstream distributions of QEMU that include backports and/or have a different deprecation policy. I'd like to hear opinions from others.
On Thu, Apr 25, 2019 at 02:42:08PM -0300, Eduardo Habkost wrote: > On Thu, Apr 25, 2019 at 11:20:58AM -0300, Wainer dos Santos Moschetta wrote: > > Hi Eduardo, > > > > > > On 04/23/2019 06:22 PM, Eduardo Habkost wrote: > > > This struct will be used to represent support and deprecation > > > status of QEMU features. > > > > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > > --- > > > 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. > > > > Eventually management software will need the know the QEMU version the > > feature is planed for removal. So makes sense to include a field to capture > > that information as well or do you expect it to be added (as a good > > practice) in the 'status-message'? > > If we really want to provide extra information like version > numbers, adding a separate field sounds better than using > status-message. > > But I'm not sure we really want to include this amount of detail > in the API. Mentioning explicit version numbers could make > things more complex for downstream distributions of QEMU that > include backports and/or have a different deprecation policy. > > I'd like to hear opinions from others. Yeah, I'm *not* in favour of mentioning any version number in this. Our "2 cycle" deprecation rule is more of a guideline than a strict rule. It can be extended if we find some blocking problem that makes removal more painful than expected. Regards, Daniel
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 <ehabkost@redhat.com> > > > --- > > > 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 } } { 'struct': "SupportStatusInfo", 'data': { 'status': 'SupportStatus', '*info': 'str', '*hint': 'str' } } Regards, Daniel
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 <ehabkost@redhat.com> > > > > --- > > > > 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.
On Tue, Apr 30, 2019 at 09:42:36AM -0300, Eduardo Habkost wrote: > 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 <ehabkost@redhat.com> > > > > > --- > > > > > 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. Good point - we should track quality status separately. So still need a boolean deprecation flag too i guess. > > > > > { 'struct': "SupportStatusInfo", > > 'data': { 'status': 'SupportStatus', > > '*info': 'str', > > '*hint': 'str' } } > > "hint" seems nicer than "suggested" or "recommended". I like it. > > -- > Eduardo Regards, Daniel
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' } }
This struct will be used to represent support and deprecation status of QEMU features. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- qapi/common.json | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)