| Message ID | 20181030111348.14713-2-kraxel@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introducing QemuSupportState | expand |
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 <kraxel@redhat.com> > --- > 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. > diff --git a/util/Makefile.objs b/util/Makefile.objs > index 0820923c18..6e5f8faf82 100644 > --- a/util/Makefile.objs > +++ b/util/Makefile.objs > @@ -50,5 +50,6 @@ util-obj-y += range.o > util-obj-y += stats64.o > util-obj-y += systemd.o > util-obj-y += iova-tree.o > +util-obj-y += support-state.o > util-obj-$(CONFIG_LINUX) += vfio-helpers.o > util-obj-$(CONFIG_OPENGL) += drm.o >
On Tue, Oct 30, 2018 at 02:32:40PM +0100, Philippe Mathieu-Daudé wrote: > Hi Gerd, > > On 30/10/18 12:13, Gerd Hoffmann wrote: > > Indicates support state for somerhing (device, backend, subsystem, ...) > > "something" Oops, I'll fix. > > +## > > +# @SupportState: > > +# > > +# Indicate Support level of qemu devices, backends, subsystems, ... > > +# > > +# Since: 3.2 > > +## > > +{ 'enum': 'SupportState', > > + 'data': [ 'unknown', > > 'unknown' is scary and should be fixed. 'unknown' maps to "0" due to being first in list, so this is what you get when it isn't explicitly set to something else. Which make sense IMHO. > > + 'supported', > > + 'maintained', > > + 'odd-fixes', > > All those fit in 'supported' > > > + 'orphan', > > + 'obsolete', > > + 'deprecated' ] } > > And all those should appear as 'deprecated' IMHO. See minutes on deprecation discussion. Seems there is agreement we need something more finegrained than "supported" and "deprecated". cheers, Gerd
On 30/10/18 15:00, Gerd Hoffmann wrote: > On Tue, Oct 30, 2018 at 02:32:40PM +0100, Philippe Mathieu-Daudé wrote: >> Hi Gerd, >> >> On 30/10/18 12:13, Gerd Hoffmann wrote: >>> Indicates support state for somerhing (device, backend, subsystem, ...) >> >> "something" > > Oops, I'll fix. > >>> +## >>> +# @SupportState: >>> +# >>> +# Indicate Support level of qemu devices, backends, subsystems, ... >>> +# >>> +# Since: 3.2 >>> +## >>> +{ 'enum': 'SupportState', >>> + 'data': [ 'unknown', >> >> 'unknown' is scary and should be fixed. > > 'unknown' maps to "0" due to being first in list, so this is what you > get when it isn't explicitly set to something else. Which make sense > IMHO. Yes, I understand in your next patch, this case won't display warning to the user. I wanted to say "we should fix those entries in the MAINTAINERS file". > >>> + 'supported', >>> + 'maintained', >>> + 'odd-fixes', >> >> All those fit in 'supported' >> >>> + 'orphan', >>> + 'obsolete', >>> + 'deprecated' ] } >> >> And all those should appear as 'deprecated' IMHO. > > See minutes on deprecation discussion. Seems there is agreement we > need something more finegrained than "supported" and "deprecated". I read again the "Minutes of KVM Forum BoF on deprecating stuff" thread and don't find details on finegrains, can you point it to me? I think these are fine in the MAINTAINERS entries, but don't give useful information to a QEMU user that is not custom to MAINTAINERS. As a user I'd expect anything not "supported" to be eventually "deprecated". Should we continue this discussion on the "Minutes of KVM Forum BoF on deprecating stuff" thread? Thanks, Phil. > > cheers, > Gerd > >
On Tue, Oct 30, 2018 at 12:13:45PM +0100, Gerd Hoffmann wrote: > Indicates support state for somerhing (device, backend, subsystem, ...) > in qemu. Modeled roughly after the "S:" states we have in MAINTANERS. > Personally, I would prefer to start with a very simple data model (e.g. a 'deprecated' boolean flag), instead of trying to model every possibility in the first version. With a detailed maintenance status flag, the meaning of the values would be confusing for somebody using a downstream distribution of QEMU. Would it reflect upstream maintenance status, or downstream vendor guarantees about support? Would downstream vendors be required to patch QEMU to update DeviceClass::supported on every device they ship? I think this would cause confusion and require extra work for every downstream vendor, and not solve any real world problems. A 'deprecated' flag could still require extra work, but only in a few specific cases: deprecated devices are the exception, not the rule, and downstream vendors would only need to touch device code if their deprecation status is different from upstream. > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- [...] > + > +## > +# @SupportState: > +# > +# Indicate Support level of qemu devices, backends, subsystems, ... If we're following that path, I would like to get each possible value for this enum to be clearly documented. Especially considering that this don't match the list on MAINTAINERS exactly. For example, I don't understand the difference between "obsolete" and "deprecated". > +# > +# Since: 3.2 > +## > +{ 'enum': 'SupportState', > + 'data': [ 'unknown', > + 'supported', > + 'maintained', > + 'odd-fixes', > + 'orphan', > + 'obsolete', > + 'deprecated' ] } > diff --git a/util/Makefile.objs b/util/Makefile.objs > index 0820923c18..6e5f8faf82 100644 > --- a/util/Makefile.objs > +++ b/util/Makefile.objs > @@ -50,5 +50,6 @@ util-obj-y += range.o > util-obj-y += stats64.o > util-obj-y += systemd.o > util-obj-y += iova-tree.o > +util-obj-y += support-state.o > util-obj-$(CONFIG_LINUX) += vfio-helpers.o > util-obj-$(CONFIG_OPENGL) += drm.o > -- > 2.9.3 >
On Tue, Oct 30, 2018 at 12:13:45PM +0100, Gerd Hoffmann wrote: > Indicates support state for somerhing (device, backend, subsystem, ...) > in qemu. Modeled roughly after the "S:" states we have in MAINTANERS. > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> [...] > +typedef struct QemuSupportState { > + SupportState state; > + const char *reason; > +} QemuSupportState; I find it weird that we're calling this field "reason", while this is how the field is being actually used: * "use a newer machine type instead" * "use 40p machine type instead" * "use '-vga std' instead" These are not reasons for deprecation, they are just suggested alternatives.
On Tue, 30 Oct 2018 15:13:45 +0100 Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > On 30/10/18 15:00, Gerd Hoffmann wrote: > > On Tue, Oct 30, 2018 at 02:32:40PM +0100, Philippe Mathieu-Daudé wrote: > >>> +## > >>> +# @SupportState: > >>> +# > >>> +# Indicate Support level of qemu devices, backends, subsystems, ... > >>> +# > >>> +# Since: 3.2 > >>> +## > >>> +{ 'enum': 'SupportState', > >>> + 'data': [ 'unknown', > >> > >> 'unknown' is scary and should be fixed. > > > > 'unknown' maps to "0" due to being first in list, so this is what you > > get when it isn't explicitly set to something else. Which make sense > > IMHO. > > Yes, I understand in your next patch, this case won't display warning to > the user. > > I wanted to say "we should fix those entries in the MAINTAINERS file". I think that has been an ongoing quest for years :) > > > > >>> + 'supported', > >>> + 'maintained', > >>> + 'odd-fixes', > >> > >> All those fit in 'supported' > >> > >>> + 'orphan', > >>> + 'obsolete', > >>> + 'deprecated' ] } > >> > >> And all those should appear as 'deprecated' IMHO. > > > > See minutes on deprecation discussion. Seems there is agreement we > > need something more finegrained than "supported" and "deprecated". > > I read again the "Minutes of KVM Forum BoF on deprecating stuff" thread > and don't find details on finegrains, can you point it to me? > > I think these are fine in the MAINTAINERS entries, but don't give useful > information to a QEMU user that is not custom to MAINTAINERS. We might squash 'supported' and 'maintained' together (as their only real difference is whether someone gets paid for it), but 'odd fixes' is different IMO (you have someone to talk to, but they don't dedicate much of their time to it.) > > As a user I'd expect anything not "supported" to be eventually "deprecated". But there are differences: - 'orphan' - nobody is looking after it; should become 'deprecated' if nobody steps up, but may move to one of the 'someone looks after it' states - 'obsolete' - don't use this; should move to 'deprecated' once a replacement is ready (or it is not needed anymore) - 'deprecated' - on the removal schedule; has not necessarily been in 'orphan' or 'obsolete' before > > Should we continue this discussion on the "Minutes of KVM Forum BoF on > deprecating stuff" thread? > > Thanks, > > Phil. > > > > > cheers, > > Gerd > > > > >
Hi, Gerd. On Tue, Oct 30, 2018 at 12:13:45PM +0100, Gerd Hoffmann wrote: > Indicates support state for somerhing (device, backend, subsystem, ...) > in qemu. Modeled roughly after the "S:" states we have in MAINTANERS. > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > 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', > + 'supported', > + 'maintained', > + 'odd-fixes', > + 'orphan', > + 'obsolete', > + 'deprecated' ] } Regardless how fine-grained we decide to be here, is it possible that you describe in the documentation comment where each state shall be used?
On 30/10/18 16:46, Cornelia Huck wrote: > On Tue, 30 Oct 2018 15:13:45 +0100 > Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > >> On 30/10/18 15:00, Gerd Hoffmann wrote: >>> On Tue, Oct 30, 2018 at 02:32:40PM +0100, Philippe Mathieu-Daudé wrote: >>>>> +## >>>>> +# @SupportState: >>>>> +# >>>>> +# Indicate Support level of qemu devices, backends, subsystems, ... >>>>> +# >>>>> +# Since: 3.2 >>>>> +## >>>>> +{ 'enum': 'SupportState', >>>>> + 'data': [ 'unknown', >>>> >>>> 'unknown' is scary and should be fixed. >>> >>> 'unknown' maps to "0" due to being first in list, so this is what you >>> get when it isn't explicitly set to something else. Which make sense >>> IMHO. >> >> Yes, I understand in your next patch, this case won't display warning to >> the user. >> >> I wanted to say "we should fix those entries in the MAINTAINERS file". > > I think that has been an ongoing quest for years :) > >> >>> >>>>> + 'supported', >>>>> + 'maintained', >>>>> + 'odd-fixes', >>>> >>>> All those fit in 'supported' >>>> >>>>> + 'orphan', >>>>> + 'obsolete', >>>>> + 'deprecated' ] } >>>> >>>> And all those should appear as 'deprecated' IMHO. >>> >>> See minutes on deprecation discussion. Seems there is agreement we >>> need something more finegrained than "supported" and "deprecated". >> >> I read again the "Minutes of KVM Forum BoF on deprecating stuff" thread >> and don't find details on finegrains, can you point it to me? >> >> I think these are fine in the MAINTAINERS entries, but don't give useful >> information to a QEMU user that is not custom to MAINTAINERS. > > We might squash 'supported' and 'maintained' together (as their only > real difference is whether someone gets paid for it), but 'odd fixes' > is different IMO (you have someone to talk to, but they don't dedicate > much of their time to it.) > >> >> As a user I'd expect anything not "supported" to be eventually "deprecated". > > But there are differences: > - 'orphan' - nobody is looking after it; should become 'deprecated' if > nobody steps up, but may move to one of the 'someone looks after it' > states > - 'obsolete' - don't use this; should move to 'deprecated' once a > replacement is ready (or it is not needed anymore) > - 'deprecated' - on the removal schedule; has not necessarily been in > 'orphan' or 'obsolete' before OK! If I understand correctly, we have this 'state machine': Supported Unsupported Deprecated (someone to talk) (nobody to talk) (scheduled for removal) +--------------+ +------------+ +--------------+ | S:Maintained | | S:Unknown | | | | | | | | | | S:Supported + <---> | S:Orphan +----> | S:Deprecated +----> Removed | | | | | | | S:Odd Fixes | | S:Obsolete | | | +------+-------+ +------------+ +--------------+ | ^ | | +-------------------------------------------+ So we have 3 distinct states. Note: - Maintainers can deprecate stuffs - Orphan code can become Supported - Once scheduled for removal, there is no way back - 'Unknown' seems pretty similar to 'Orphan'. > >> >> Should we continue this discussion on the "Minutes of KVM Forum BoF on >> deprecating stuff" thread? >> >> Thanks, >> >> Phil. >> >>> >>> cheers, >>> Gerd >>> >>> >> >
On Tue, Oct 30, 2018 at 06:37:42PM +0100, Philippe Mathieu-Daudé wrote: > On 30/10/18 16:46, Cornelia Huck wrote: > > On Tue, 30 Oct 2018 15:13:45 +0100 > > Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > > > > > On 30/10/18 15:00, Gerd Hoffmann wrote: > > > > On Tue, Oct 30, 2018 at 02:32:40PM +0100, Philippe Mathieu-Daudé wrote: > > > > > > +## > > > > > > +# @SupportState: > > > > > > +# > > > > > > +# Indicate Support level of qemu devices, backends, subsystems, ... > > > > > > +# > > > > > > +# Since: 3.2 > > > > > > +## > > > > > > +{ 'enum': 'SupportState', > > > > > > + 'data': [ 'unknown', > > > > > > > > > > 'unknown' is scary and should be fixed. > > > > > > > > 'unknown' maps to "0" due to being first in list, so this is what you > > > > get when it isn't explicitly set to something else. Which make sense > > > > IMHO. > > > > > > Yes, I understand in your next patch, this case won't display warning to > > > the user. > > > > > > I wanted to say "we should fix those entries in the MAINTAINERS file". > > > > I think that has been an ongoing quest for years :) > > > > > > > > > > > + 'supported', > > > > > > + 'maintained', > > > > > > + 'odd-fixes', > > > > > > > > > > All those fit in 'supported' > > > > > > + 'orphan', > > > > > > + 'obsolete', > > > > > > + 'deprecated' ] } > > > > > > > > > > And all those should appear as 'deprecated' IMHO. > > > > > > > > See minutes on deprecation discussion. Seems there is agreement we > > > > need something more finegrained than "supported" and "deprecated". > > > > > > I read again the "Minutes of KVM Forum BoF on deprecating stuff" thread > > > and don't find details on finegrains, can you point it to me? > > > > > > I think these are fine in the MAINTAINERS entries, but don't give useful > > > information to a QEMU user that is not custom to MAINTAINERS. > > > > We might squash 'supported' and 'maintained' together (as their only > > real difference is whether someone gets paid for it), but 'odd fixes' > > is different IMO (you have someone to talk to, but they don't dedicate > > much of their time to it.) > > > > > > > > As a user I'd expect anything not "supported" to be eventually "deprecated". > > > > But there are differences: > > - 'orphan' - nobody is looking after it; should become 'deprecated' if > > nobody steps up, but may move to one of the 'someone looks after it' > > states > > - 'obsolete' - don't use this; should move to 'deprecated' once a > > replacement is ready (or it is not needed anymore) > > - 'deprecated' - on the removal schedule; has not necessarily been in > > 'orphan' or 'obsolete' before > > OK! > > If I understand correctly, we have this 'state machine': > > Supported Unsupported Deprecated > > (someone to talk) (nobody to talk) (scheduled for removal) > +--------------+ +------------+ +--------------+ > | S:Maintained | | S:Unknown | | | > | | | | | | > | S:Supported + <---> | S:Orphan +----> | S:Deprecated +----> Removed > | | | | | | > | S:Odd Fixes | | S:Obsolete | | | > +------+-------+ +------------+ +--------------+ > | ^ > | | > +-------------------------------------------+ > > So we have 3 distinct states. > > Note: > - Maintainers can deprecate stuffs > - Orphan code can become Supported > - Once scheduled for removal, there is no way back > - 'Unknown' seems pretty similar to 'Orphan'. I'm still worried that the supported/unsupported distinction may cause unnecessary hassle for every downstream distributor of QEMU. Do we really have a need to differentiate supported vs unsupported device types at runtime? I'd prefer to make support/unsupported differentiation to be a build system feature (e.g. a CONFIG_UNSUPPORTED build-time option) instead of a QMP/runtime feature.
On Tue, 30 Oct 2018 20:15:45 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: > On Tue, Oct 30, 2018 at 06:37:42PM +0100, Philippe Mathieu-Daudé wrote: > > If I understand correctly, we have this 'state machine': > > > > Supported Unsupported Deprecated > > > > (someone to talk) (nobody to talk) (scheduled for removal) > > +--------------+ +------------+ +--------------+ > > | S:Maintained | | S:Unknown | | | > > | | | | | | > > | S:Supported + <---> | S:Orphan +----> | S:Deprecated +----> Removed > > | | | | | | > > | S:Odd Fixes | | S:Obsolete | | | > > +------+-------+ +------------+ +--------------+ > > | ^ > > | | > > +-------------------------------------------+ I believe we can also take things out of the deprecated state again, but I would expect that to be a very rare event. 'Obsolete' is probably 'unsupported' in the 'do not use that' sense, but there still might be someone to talk to. Similarly, 'unknown' devices etc. may have someone we can talk to, it's just the problem of finding out who that is and getting them to move it into the 'supported' state :) > > > > So we have 3 distinct states. > > > > Note: > > - Maintainers can deprecate stuffs It's not only maintainers that can do that, but it has to get their ack, of course. > > - Orphan code can become Supported > > - Once scheduled for removal, there is no way back We should not block the way back, or else we cannot adapt to things we did not know when we first deprecated it. > > - 'Unknown' seems pretty similar to 'Orphan'. Not quite. If something is 'orphan', it was explicitly moved to that state when a maintainer gave it up. 'Unknown' may also be stuff that simply fell through the cracks. > > I'm still worried that the supported/unsupported distinction may > cause unnecessary hassle for every downstream distributor of > QEMU. Do we really have a need to differentiate supported vs > unsupported device types at runtime? > > I'd prefer to make support/unsupported differentiation to be a > build system feature (e.g. a CONFIG_UNSUPPORTED build-time > option) instead of a QMP/runtime feature. I think that makes sense -- we really want to single out devices/machines that are deprecated and will go away unless something magic happens.
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 <kraxel@redhat.com> >> --- >> 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. - 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. - 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. The idea being that there is definitely a difference between obviously and wholly broken components that we're working on replacing or getting rid of, and components that are "in beta" or partially functional, and those that are full, "tier 1" supported subsystems. Adding any more nuanced states than this, though, risks making it too difficult to make any informed decisions as a user, I think. This patch as-is maybe adds too many? --js >> diff --git a/util/Makefile.objs b/util/Makefile.objs >> index 0820923c18..6e5f8faf82 100644 >> --- a/util/Makefile.objs >> +++ b/util/Makefile.objs >> @@ -50,5 +50,6 @@ util-obj-y += range.o >> util-obj-y += stats64.o >> util-obj-y += systemd.o >> util-obj-y += iova-tree.o >> +util-obj-y += support-state.o >> util-obj-$(CONFIG_LINUX) += vfio-helpers.o >> util-obj-$(CONFIG_OPENGL) += drm.o >> >
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 <kraxel@redhat.com> > >> --- > >> 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. > > - 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? > > - 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. > > The idea being that there is definitely a difference between obviously > and wholly broken components that we're working on replacing or getting > rid of, and components that are "in beta" or partially functional, and > those that are full, "tier 1" supported subsystems. I agree there's a difference between unsupported and supported code. But I'd say that making this a build time option is a must (as many distributions would wish to disable unsupported code at build time). Making the information available at runtime is just nice to have. 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. > > Adding any more nuanced states than this, though, risks making it too > difficult to make any informed decisions as a user, I think. This patch > as-is maybe adds too many? Agreed. > > --js > > >> diff --git a/util/Makefile.objs b/util/Makefile.objs > >> index 0820923c18..6e5f8faf82 100644 > >> --- a/util/Makefile.objs > >> +++ b/util/Makefile.objs > >> @@ -50,5 +50,6 @@ util-obj-y += range.o > >> util-obj-y += stats64.o > >> util-obj-y += systemd.o > >> util-obj-y += iova-tree.o > >> +util-obj-y += support-state.o > >> util-obj-$(CONFIG_LINUX) += vfio-helpers.o > >> util-obj-$(CONFIG_OPENGL) += drm.o > >> > >
On 10/31/2018 02:06 PM, 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 <kraxel@redhat.com> >>>> --- >>>> 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. >> >> - 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? > I maybe should not have used the word deprecated, which I think obfuscates the code quality metric we might be trying to convey: even top-notch, first-tier support code can be deprecated in favor of some newer, better model. So let's not use it for these purposes: as you suggest, even deprecated code should be compiled because it hasn't been removed *yet*, and it serves explicitly as a transitional stage. I just really want to paint a difference between... say, "stuff that maybe works, but isn't tier 1" and "stuff that probably doesn't work without some elbow grease." I'm just having a hard time coming up with a hard metric to differentiate the two, but I can't shake the feeling that "SUPPORTED" vs "UNSUPPORTED" is just simply too strict of a binary. In my mental model, it's three tiers: A: First-class support. B: Proceed with caution. C: Experimental/Development ONLY. As a stoplight, it'd be: GREEN: Expected to work. Well cared for and tested. YELLOW: Some problems, but expected either to transition to green (maintainer willing), OR be deprecated/removed. At a code quality level where non-developers should try using it and report bugs. These devices might work well for certain use cases but aren't fully implemented yet and could break. RED: Broken. Kept in the code base for development reasons; e.g. new ARM/SoC models that are in development, but are known to not work yet. People who are not developers should not waste their time trying to use them to accomplish real work yet. That's kind of the granularity I have in mind, but I don't know if it's practical to grade everything on so fuzzy a scale to begin with. I'd suggest as a first pass marking everything either as YELLOW or RED to suggest "SUPPORTED" vs "UNSUPPORTED", and then individually making the case for specific components to be promoted to GREEN. --js >> >> - 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. >> >> The idea being that there is definitely a difference between obviously >> and wholly broken components that we're working on replacing or getting >> rid of, and components that are "in beta" or partially functional, and >> those that are full, "tier 1" supported subsystems. > > I agree there's a difference between unsupported and supported > code. > > But I'd say that making this a build time option is a must (as > many distributions would wish to disable unsupported code at > build time). Making the information available at runtime is just > nice to have. > Oh, yes, it must absolutely be build-time. In my three-tier example, I would expect downstream distributions like Fedora to compile out the RED/C-TIER immediately. For something like RHEL, they might compile out the YELLOW/B-TIER as well. Some use cases might want to compile out RED but leave YELLOW as some run-time warning or run-time error ("please use --use-shoddy-parts if you wish to use %%ramshackle_device%% ...") I think what this buys us is confidence that despite QEMU's very wide scope, we can quickly see both in code and at run-time that we're not using something we shouldn't be, or misunderstanding the relative quality of the parts we're building our virtual machine from. Having conservative green/yellow lists might help reduce the "search space" for documentation and help more users arrive at "good" configurations by default. > 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. > > >> >> Adding any more nuanced states than this, though, risks making it too >> difficult to make any informed decisions as a user, I think. This patch >> as-is maybe adds too many? > > Agreed. > >> >> --js >> >>>> diff --git a/util/Makefile.objs b/util/Makefile.objs >>>> index 0820923c18..6e5f8faf82 100644 >>>> --- a/util/Makefile.objs >>>> +++ b/util/Makefile.objs >>>> @@ -50,5 +50,6 @@ util-obj-y += range.o >>>> util-obj-y += stats64.o >>>> util-obj-y += systemd.o >>>> util-obj-y += iova-tree.o >>>> +util-obj-y += support-state.o >>>> util-obj-$(CONFIG_LINUX) += vfio-helpers.o >>>> util-obj-$(CONFIG_OPENGL) += drm.o >>>> >>> >
On Wed, Oct 31, 2018 at 02:37:36PM -0400, John Snow wrote: > > > On 10/31/2018 02:06 PM, 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 <kraxel@redhat.com> > >>>> --- > >>>> 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. > >> > >> - 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? > > > > I maybe should not have used the word deprecated, which I think > obfuscates the code quality metric we might be trying to convey: even > top-notch, first-tier support code can be deprecated in favor of some > newer, better model. > > So let's not use it for these purposes: as you suggest, even deprecated > code should be compiled because it hasn't been removed *yet*, and it > serves explicitly as a transitional stage. > > > I just really want to paint a difference between... say, > > "stuff that maybe works, but isn't tier 1" and > "stuff that probably doesn't work without some elbow grease." > > I'm just having a hard time coming up with a hard metric to > differentiate the two, but I can't shake the feeling that "SUPPORTED" vs > "UNSUPPORTED" is just simply too strict of a binary. > > In my mental model, it's three tiers: > > A: First-class support. > B: Proceed with caution. > C: Experimental/Development ONLY. > > As a stoplight, it'd be: > > GREEN: Expected to work. Well cared for and tested. > > YELLOW: Some problems, but expected either to transition to green > (maintainer willing), OR be deprecated/removed. At a code quality level > where non-developers should try using it and report bugs. These devices > might work well for certain use cases but aren't fully implemented yet > and could break. > > RED: Broken. Kept in the code base for development reasons; e.g. new > ARM/SoC models that are in development, but are known to not work yet. > People who are not developers should not waste their time trying to use > them to accomplish real work yet. > > > That's kind of the granularity I have in mind, but I don't know if it's > practical to grade everything on so fuzzy a scale to begin with. I'd > suggest as a first pass marking everything either as YELLOW or RED to > suggest "SUPPORTED" vs "UNSUPPORTED", and then individually making the > case for specific components to be promoted to GREEN. > > --js > > >> > >> - 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. > >> > >> The idea being that there is definitely a difference between obviously > >> and wholly broken components that we're working on replacing or getting > >> rid of, and components that are "in beta" or partially functional, and > >> those that are full, "tier 1" supported subsystems. > > > > I agree there's a difference between unsupported and supported > > code. > > > > But I'd say that making this a build time option is a must (as > > many distributions would wish to disable unsupported code at > > build time). Making the information available at runtime is just > > nice to have. > > > > Oh, yes, it must absolutely be build-time. In my three-tier example, I > would expect downstream distributions like Fedora to compile out the > RED/C-TIER immediately. For something like RHEL, they might compile out > the YELLOW/B-TIER as well. > > Some use cases might want to compile out RED but leave YELLOW as some > run-time warning or run-time error ("please use --use-shoddy-parts if > you wish to use %%ramshackle_device%% ...") > > I think what this buys us is confidence that despite QEMU's very wide > scope, we can quickly see both in code and at run-time that we're not > using something we shouldn't be, or misunderstanding the relative > quality of the parts we're building our virtual machine from. > > Having conservative green/yellow lists might help reduce the "search > space" for documentation and help more users arrive at "good" > configurations by default. Right, I think we are on the same page here. Now, support/stability status and deprecation/removal status seem to be independent variables. It's perfectly valid to say "this device is expected to be removed in the future, you need to update your configuration" and "this device is expected to be stable and we support it" at the same time. Let's not mix up both. This also means keeping in mind the different stories we want to address. The ones I would like to address are: 1) As a virtualization app user, I want to be warned at runtime in case my VM configuration relies on features that will be removed in future versions of the stack, so I have a chance to update my VM configuration before updating the software. 2) As a QEMU distributor, downstream vendor, or developer, I want to: * know which parts of the code are really supported and expected to work without bugs; * disable unsupported/unstable features at build time; * disable test cases for known-broken parts of QEMU; * know if an user is relying on an unsupported or known-broken feature of QEMU. Are there any additional use cases I forgot, here? > > > 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. > > > > > >> > >> Adding any more nuanced states than this, though, risks making it too > >> difficult to make any informed decisions as a user, I think. This patch > >> as-is maybe adds too many? > > > > Agreed. > > > >> > >> --js > >> > >>>> diff --git a/util/Makefile.objs b/util/Makefile.objs > >>>> index 0820923c18..6e5f8faf82 100644 > >>>> --- a/util/Makefile.objs > >>>> +++ b/util/Makefile.objs > >>>> @@ -50,5 +50,6 @@ util-obj-y += range.o > >>>> util-obj-y += stats64.o > >>>> util-obj-y += systemd.o > >>>> util-obj-y += iova-tree.o > >>>> +util-obj-y += support-state.o > >>>> util-obj-$(CONFIG_LINUX) += vfio-helpers.o > >>>> util-obj-$(CONFIG_OPENGL) += drm.o > >>>> > >>> > >
On 10/31/2018 02:58 PM, Eduardo Habkost wrote: > On Wed, Oct 31, 2018 at 02:37:36PM -0400, John Snow wrote: >> >> >> On 10/31/2018 02:06 PM, 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 <kraxel@redhat.com> >>>>>> --- >>>>>> 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. >>>> >>>> - 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? >>> >> >> I maybe should not have used the word deprecated, which I think >> obfuscates the code quality metric we might be trying to convey: even >> top-notch, first-tier support code can be deprecated in favor of some >> newer, better model. >> >> So let's not use it for these purposes: as you suggest, even deprecated >> code should be compiled because it hasn't been removed *yet*, and it >> serves explicitly as a transitional stage. >> >> >> I just really want to paint a difference between... say, >> >> "stuff that maybe works, but isn't tier 1" and >> "stuff that probably doesn't work without some elbow grease." >> >> I'm just having a hard time coming up with a hard metric to >> differentiate the two, but I can't shake the feeling that "SUPPORTED" vs >> "UNSUPPORTED" is just simply too strict of a binary. >> >> In my mental model, it's three tiers: >> >> A: First-class support. >> B: Proceed with caution. >> C: Experimental/Development ONLY. >> >> As a stoplight, it'd be: >> >> GREEN: Expected to work. Well cared for and tested. >> >> YELLOW: Some problems, but expected either to transition to green >> (maintainer willing), OR be deprecated/removed. At a code quality level >> where non-developers should try using it and report bugs. These devices >> might work well for certain use cases but aren't fully implemented yet >> and could break. >> >> RED: Broken. Kept in the code base for development reasons; e.g. new >> ARM/SoC models that are in development, but are known to not work yet. >> People who are not developers should not waste their time trying to use >> them to accomplish real work yet. >> >> >> That's kind of the granularity I have in mind, but I don't know if it's >> practical to grade everything on so fuzzy a scale to begin with. I'd >> suggest as a first pass marking everything either as YELLOW or RED to >> suggest "SUPPORTED" vs "UNSUPPORTED", and then individually making the >> case for specific components to be promoted to GREEN. >> >> --js >> >>>> >>>> - 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. >>>> >>>> The idea being that there is definitely a difference between obviously >>>> and wholly broken components that we're working on replacing or getting >>>> rid of, and components that are "in beta" or partially functional, and >>>> those that are full, "tier 1" supported subsystems. >>> >>> I agree there's a difference between unsupported and supported >>> code. >>> >>> But I'd say that making this a build time option is a must (as >>> many distributions would wish to disable unsupported code at >>> build time). Making the information available at runtime is just >>> nice to have. >>> >> >> Oh, yes, it must absolutely be build-time. In my three-tier example, I >> would expect downstream distributions like Fedora to compile out the >> RED/C-TIER immediately. For something like RHEL, they might compile out >> the YELLOW/B-TIER as well. >> >> Some use cases might want to compile out RED but leave YELLOW as some >> run-time warning or run-time error ("please use --use-shoddy-parts if >> you wish to use %%ramshackle_device%% ...") >> >> I think what this buys us is confidence that despite QEMU's very wide >> scope, we can quickly see both in code and at run-time that we're not >> using something we shouldn't be, or misunderstanding the relative >> quality of the parts we're building our virtual machine from. >> >> Having conservative green/yellow lists might help reduce the "search >> space" for documentation and help more users arrive at "good" >> configurations by default. > > Right, I think we are on the same page here. > > Now, support/stability status and deprecation/removal status seem > to be independent variables. It's perfectly valid to say "this > device is expected to be removed in the future, you need to > update your configuration" and "this device is expected to be > stable and we support it" at the same time. Let's not mix up > both. > Agree > This also means keeping in mind the different stories we want to > address. The ones I would like to address are: > > 1) As a virtualization app user, I want to be warned at runtime > in case my VM configuration relies on features that will be > removed in future versions of the stack, so I have a chance to > update my VM configuration before updating the software. > Yes, or as a user, get warned if my configuration is less than optimal. Use of a "yellow" or "red" component that I thought was perfectly safe. Many people don't realize they're using legacy components until we actually nominate them for deprecation. They could have found out much sooner. Part of Nemu's value now is knowing that whatever is left has a much higher chance of being something you want, because there's less ways to do things. > 2) As a QEMU distributor, downstream vendor, or developer, I want > to: > * know which parts of the code are really supported and > expected to work without bugs; > * disable unsupported/unstable features at build time; > * disable test cases for known-broken parts of QEMU; > * know if an user is relying on an unsupported or known-broken > feature of QEMU. > Tying components to test cases is compelling. > Are there any additional use cases I forgot, here? > Those are the big ones. > >> >>> 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. >>> >>> >>>> >>>> Adding any more nuanced states than this, though, risks making it too >>>> difficult to make any informed decisions as a user, I think. This patch >>>> as-is maybe adds too many? >>> >>> Agreed. >>> >>>> >>>> --js >>>> >>>>>> diff --git a/util/Makefile.objs b/util/Makefile.objs >>>>>> index 0820923c18..6e5f8faf82 100644 >>>>>> --- a/util/Makefile.objs >>>>>> +++ b/util/Makefile.objs >>>>>> @@ -50,5 +50,6 @@ util-obj-y += range.o >>>>>> util-obj-y += stats64.o >>>>>> util-obj-y += systemd.o >>>>>> util-obj-y += iova-tree.o >>>>>> +util-obj-y += support-state.o >>>>>> util-obj-$(CONFIG_LINUX) += vfio-helpers.o >>>>>> util-obj-$(CONFIG_OPENGL) += drm.o >>>>>> >>>>> >>> >
Hi, > > - Maintainers can deprecate stuffs > > - Orphan code can become Supported > > - Once scheduled for removal, there is no way back > > - 'Unknown' seems pretty similar to 'Orphan'. > > I'm still worried that the supported/unsupported distinction may > cause unnecessary hassle for every downstream distributor of > QEMU. Do we really have a need to differentiate supported vs > unsupported device types at runtime? How do you suggest to handle cirrus then? Trying to deprecate it outright didn't work very well, kind of understandable given that it has been the default for a long time. So I think we have to mark cirrus as "obsolete", printing a message for the users that they should switch to another display device (stdvga for example), but keep it working for a while. There are also a bunch of devices where I suspect they are not used much if at all. All those isa sound cards for example. Playing old DOS games is pretty much the only use case I can think of. But I'm wondering whenever people actually use qemu for that. There are alternatives which probably handle that use case better, i.e. dosbox. Simliar case is bluetooth emulation. I can't remember having seen any message or patch for years. Tagging such devices as "obsolete", with a message asking people to report their use cases, might help figuring if and why those devices are used in the wild. > I'd prefer to make support/unsupported differentiation to be a > build system feature (e.g. a CONFIG_UNSUPPORTED build-time > option) instead of a QMP/runtime feature. That would be nice too, but I think we need kbuild first, otherwise it'll be pretty messy. cheers, Gerd
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 <kraxel@redhat.com> > > >> --- > > >> 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
On Mon, Nov 05, 2018 at 08:30:28AM +0100, Gerd Hoffmann wrote: > Hi, > > > > - Maintainers can deprecate stuffs > > > - Orphan code can become Supported > > > - Once scheduled for removal, there is no way back > > > - 'Unknown' seems pretty similar to 'Orphan'. > > > > I'm still worried that the supported/unsupported distinction may > > cause unnecessary hassle for every downstream distributor of > > QEMU. Do we really have a need to differentiate supported vs > > unsupported device types at runtime? > > How do you suggest to handle cirrus then? > > Trying to deprecate it outright didn't work very well, kind of > understandable given that it has been the default for a long time. > > So I think we have to mark cirrus as "obsolete", printing a message for > the users that they should switch to another display device (stdvga for > example), but keep it working for a while. > > There are also a bunch of devices where I suspect they are not used much > if at all. All those isa sound cards for example. Playing old DOS > games is pretty much the only use case I can think of. But I'm > wondering whenever people actually use qemu for that. There are > alternatives which probably handle that use case better, i.e. dosbox. > > Simliar case is bluetooth emulation. I can't remember having seen any > message or patch for years. > > Tagging such devices as "obsolete", with a message asking people to > report their use cases, might help figuring if and why those devices are > used in the wild. Thanks for the more detailed description of the use case you have in mind. It makes sense to me. Now, I have two questions: 1) What's more important: telling the user they are relying on an obsolete feature, or that they are relying on an unsupported feature? What exactly is the difference? 2) Do we really need to differentiate between "obsolete" and "deprecated" features? What exactly is the difference? In either case, I believe a simple supported/obsolete (or supported/unsupported) distinction is probably going to be more useful than a detailed supported/maintained/odd-fixes/orphan/obsolete model. Reviewing a list of obsolete devices downstream (to decide if they should be still compiled in downstream) sounds doable. Fixing up detailed supported/maintained/odd-fixes/orphan data downstream sounds like unnecessary extra work. > > > I'd prefer to make support/unsupported differentiation to be a > > build system feature (e.g. a CONFIG_UNSUPPORTED build-time > > option) instead of a QMP/runtime feature. > > That would be nice too, but I think we need kbuild first, otherwise > it'll be pretty messy. Agreed.
On Mon, Nov 05, 2018 at 11:49:40AM -0200, Eduardo Habkost wrote: > On Mon, Nov 05, 2018 at 08:30:28AM +0100, Gerd Hoffmann wrote: > > Hi, > > > > > > - Maintainers can deprecate stuffs > > > > - Orphan code can become Supported > > > > - Once scheduled for removal, there is no way back > > > > - 'Unknown' seems pretty similar to 'Orphan'. > > > > > > I'm still worried that the supported/unsupported distinction may > > > cause unnecessary hassle for every downstream distributor of > > > QEMU. Do we really have a need to differentiate supported vs > > > unsupported device types at runtime? > > > > How do you suggest to handle cirrus then? > > > > Trying to deprecate it outright didn't work very well, kind of > > understandable given that it has been the default for a long time. > > > > So I think we have to mark cirrus as "obsolete", printing a message for > > the users that they should switch to another display device (stdvga for > > example), but keep it working for a while. > > > > There are also a bunch of devices where I suspect they are not used much > > if at all. All those isa sound cards for example. Playing old DOS > > games is pretty much the only use case I can think of. But I'm > > wondering whenever people actually use qemu for that. There are > > alternatives which probably handle that use case better, i.e. dosbox. > > > > Simliar case is bluetooth emulation. I can't remember having seen any > > message or patch for years. > > > > Tagging such devices as "obsolete", with a message asking people to > > report their use cases, might help figuring if and why those devices are > > used in the wild. > > Thanks for the more detailed description of the use case you have > in mind. It makes sense to me. > > Now, I have two questions: > > 1) What's more important: telling the user they are relying on an > obsolete feature, or that they are relying on an unsupported > feature? What exactly is the difference? Maybe we should pick up the suggestion (by danp I think) to have two states, one describing the support level, and one for usage hints (i.e "you should not use this for performance reasons, unless you run a guest older than a decade"). > 2) Do we really need to differentiate between "obsolete" and > "deprecated" features? What exactly is the difference? "deprecated" - is on deprecation schedule according to qemu deprecation policy (remove after two releases). "obsolete" - not (yet) on deprecation schedule. > In either case, I believe a simple supported/obsolete (or > supported/unsupported) distinction is probably going to be more > useful than a detailed > supported/maintained/odd-fixes/orphan/obsolete model. > > Reviewing a list of obsolete devices downstream (to decide if > they should be still compiled in downstream) sounds doable. > Fixing up detailed supported/maintained/odd-fixes/orphan data > downstream sounds like unnecessary extra work. > > > > > > > I'd prefer to make support/unsupported differentiation to be a > > > build system feature (e.g. a CONFIG_UNSUPPORTED build-time > > > option) instead of a QMP/runtime feature. > > > > That would be nice too, but I think we need kbuild first, otherwise > > it'll be pretty messy. > > Agreed. > > -- > Eduardo
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', + 'supported', + 'maintained', + 'odd-fixes', + 'orphan', + 'obsolete', + 'deprecated' ] } diff --git a/util/Makefile.objs b/util/Makefile.objs index 0820923c18..6e5f8faf82 100644 --- a/util/Makefile.objs +++ b/util/Makefile.objs @@ -50,5 +50,6 @@ util-obj-y += range.o util-obj-y += stats64.o util-obj-y += systemd.o util-obj-y += iova-tree.o +util-obj-y += support-state.o util-obj-$(CONFIG_LINUX) += vfio-helpers.o util-obj-$(CONFIG_OPENGL) += drm.o
Indicates support state for somerhing (device, backend, subsystem, ...) in qemu. Modeled roughly after the "S:" states we have in MAINTANERS. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- 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