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

2018-11-05 Thread Gerd Hoffmann
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



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

2018-11-05 Thread Eduardo Habkost
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.

-- 
Eduardo



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

2018-11-04 Thread 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 
> > >> ---
> > >>   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 00..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 00..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 

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

2018-11-04 Thread Gerd Hoffmann
  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




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

2018-10-31 Thread John Snow



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 
>> ---
>>   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 00..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 00..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 

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

2018-10-31 Thread Eduardo Habkost
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 
>  ---
>    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 00..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 00..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
> 

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

2018-10-31 Thread John Snow



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 
 ---
   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 00..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 00..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 

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

2018-10-31 Thread Eduardo Habkost
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 
> >> ---
> >>   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 00..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 00..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 

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

2018-10-31 Thread John Snow



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 
>> ---
>>   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 00..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 00..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
>> @@ 

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

2018-10-31 Thread Cornelia Huck
On Tue, 30 Oct 2018 20:15:45 -0300
Eduardo Habkost  wrote:

> On Tue, Oct 30, 2018 at 06:37:42PM +0100, Philippe Mathieu-Daudé wrote:
> > If I understand correctly, we have this 'state machine':
> > 
> >SupportedUnsupported  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.



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

2018-10-30 Thread Eduardo Habkost
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é  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':
> 
>SupportedUnsupported  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.

-- 
Eduardo



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

2018-10-30 Thread Philippe Mathieu-Daudé

On 30/10/18 16:46, Cornelia Huck wrote:

On Tue, 30 Oct 2018 15:13:45 +0100
Philippe Mathieu-Daudé  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':

   SupportedUnsupported  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

   








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

2018-10-30 Thread Cornelia Huck
On Tue, 30 Oct 2018 15:13:45 +0100
Philippe Mathieu-Daudé  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
> > 
> >   
> 




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

2018-10-30 Thread Eduardo Habkost
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 
> ---
[...]
> +
> +##
> +# @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
> 

-- 
Eduardo



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

2018-10-30 Thread Eduardo Habkost
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 
[...]
> +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.

-- 
Eduardo



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

2018-10-30 Thread Philippe Mathieu-Daudé

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






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

2018-10-30 Thread Gerd Hoffmann
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




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

2018-10-30 Thread Philippe Mathieu-Daudé

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 
---
  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 00..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 00..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





[Qemu-devel] [PATCH 1/4] add QemuSupportState

2018-10-30 Thread Gerd Hoffmann
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 
---
 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 00..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 00..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
-- 
2.9.3