Re: [Qemu-ppc] [Qemu-devel] [PATCH 1/4] add QemuSupportState

[Gerd Hoffmann]

On Wed, Oct 31, 2018 at 03:06:04PM -0300, Eduardo Habkost wrote:
> On Wed, Oct 31, 2018 at 12:04:16PM -0400, John Snow wrote:
> > 
> > 
> > On 10/30/2018 09:32 AM, Philippe Mathieu-Daudé wrote:
> > > Hi Gerd,
> > > 
> > > On 30/10/18 12:13, Gerd Hoffmann wrote:
> > >> Indicates support state for somerhing (device, backend, subsystem, ...)
> > > 
> > > "something"
> > > 
> > >> in qemu.  Modeled roughly after the "S:" states we have in MAINTANERS.
> > >>
> > >> Signed-off-by: Gerd Hoffmann <address@hidden>
> > >> ---
> > >>   include/qemu/support-state.h | 17 +++++++++++++++++
> > >>   util/support-state.c         | 23 +++++++++++++++++++++++
> > >>   qapi/common.json             | 16 ++++++++++++++++
> > >>   util/Makefile.objs           |  1 +
> > >>   4 files changed, 57 insertions(+)
> > >>   create mode 100644 include/qemu/support-state.h
> > >>   create mode 100644 util/support-state.c
> > >>
> > >> diff --git a/include/qemu/support-state.h b/include/qemu/support-state.h
> > >> new file mode 100644
> > >> index 0000000000..5fd3c83eee
> > >> --- /dev/null
> > >> +++ b/include/qemu/support-state.h
> > >> @@ -0,0 +1,17 @@
> > >> +#ifndef QEMU_SUPPORT_STATE_H
> > >> +#define QEMU_SUPPORT_STATE_H
> > >> +
> > >> +#include "qapi/qapi-types-common.h"
> > >> +
> > >> +typedef struct QemuSupportState {
> > >> +    SupportState state;
> > >> +    const char *reason;
> > >> +} QemuSupportState;
> > >> +
> > >> +void qemu_warn_support_state(const char *type, const char *name,
> > >> +                             QemuSupportState *state);
> > >> +
> > >> +bool qemu_is_deprecated(QemuSupportState *state);
> > >> +bool qemu_is_obsolete(QemuSupportState *state);
> > >> +
> > >> +#endif /* QEMU_SUPPORT_STATE_H */
> > >> diff --git a/util/support-state.c b/util/support-state.c
> > >> new file mode 100644
> > >> index 0000000000..7966fa0fc7
> > >> --- /dev/null
> > >> +++ b/util/support-state.c
> > >> @@ -0,0 +1,23 @@
> > >> +#include "qemu/osdep.h"
> > >> +#include "qemu/error-report.h"
> > >> +#include "qemu/support-state.h"
> > >> +
> > >> +void qemu_warn_support_state(const char *type, const char *name,
> > >> +                             QemuSupportState *state)
> > >> +{
> > >> +    warn_report("%s %s is %s%s%s%s", type, name,
> > >> +                SupportState_str(state->state),
> > >> +                state->reason ? " ("          : "",
> > >> +                state->reason ? state->reason : "",
> > >> +                state->reason ? ")"           : "");
> > >> +}
> > >> +
> > >> +bool qemu_is_deprecated(QemuSupportState *state)
> > >> +{
> > >> +    return state->state == SUPPORT_STATE_DEPRECATED;
> > >> +}
> > >> +
> > >> +bool qemu_is_obsolete(QemuSupportState *state)
> > >> +{
> > >> +    return state->state == SUPPORT_STATE_OBSOLETE;
> > >> +}
> > >> diff --git a/qapi/common.json b/qapi/common.json
> > >> index 021174f04e..78176151af 100644
> > >> --- a/qapi/common.json
> > >> +++ b/qapi/common.json
> > >> @@ -151,3 +151,19 @@
> > >>                'ppc64', 'riscv32', 'riscv64', 's390x', 'sh4',
> > >>                'sh4eb', 'sparc', 'sparc64', 'tricore', 'unicore32',
> > >>                'x86_64', 'xtensa', 'xtensaeb' ] }
> > >> +
> > >> +##
> > >> +# @SupportState:
> > >> +#
> > >> +# Indicate Support level of qemu devices, backends, subsystems, ...
> > >> +#
> > >> +# Since: 3.2
> > >> +##
> > >> +{ 'enum': 'SupportState',
> > >> +  'data': [ 'unknown',
> > > 
> > > 'unknown' is scary and should be fixed.
> > > 
> > >> +            'supported',
> > >> +            'maintained',
> > >> +            'odd-fixes',
> > > 
> > > All those fit in 'supported'
> > > 
> > >> +            'orphan',
> > >> +            'obsolete',
> > >> +            'deprecated' ] }
> > > 
> > > And all those should appear as 'deprecated' IMHO.
> > > 
> > 
> > You've covered it a bit below, but I think we need more than "supported"
> > and "deprecated" -- at *least* a third state "unsupported" is required, IMO:
> > 
> > - Supported: We expect this to work. We test this component. We will
> > ship CVE fixes in a timely manner for this component. You are, as a
> > user, using something that is cared for and you may rely on us to care
> > for it.

supported + maintained.  Yes, merging them makes sense probably.

> > - Unsupported: We expect this to work, but nobody is tending to it
> > actively. It might be perfectly OK, but the tests and support may be
> > lacking. Try not to rely on this in an enterprise environment unless you
> > have resources to devote to caring for it personally. I'd count things
> > like Raspberry Pi boards in this category: they work, usually, but not
> > fully. Some people are working on them, but they're not ready for prime
> > time usage.
> 
> I wonder: do we need a distinction between code that's
> unsupported and expected to be removed in the next versions of
> QEMU, and code that's unsupported but not planned to be removed
> yet?

odd-fixes + orphan would be the "not planned to be removed" bucket.
Maybe merge those too, from a user point of view there isn't much of
a difference.

obsolete is "likely to be put on deprecation schedule" ...

> > - Deprecated: This used to work, or used to be maintained, but has been
> > superseded or is otherwise scheduled to be removed -- the expectation is
> > that this device will get worse, not better. The device model may be
> > known to be incorrect, there may be major bugs, and it receives little
> > to no care or maintenance. Actively avoid using in an enterprise context.

... for one of these reasons.

> Deprecated code, on the other hand, is expected to be enabled at
> build time even on enterprise distributions, and will definitely
> require a mechanism to generate a warning at runtime.

That sounds more like "obsolete".  "deprecated" would be "is on
deprecation schedule already".

cheers,
  Gerd