[Qemu-devel] [PATCH v2 0/6] Introducing QemuSupportState

[Qemu-devel] [PATCH v2 0/6] Introducing QemuSupportState

[Gerd Hoffmann]

Trying to fill the need to be more finegrained on support status.

v2:
 - reduce the number of support states, add documentation for them.
 - move QemuSupportState to ObjectClass, to simplify introspection
   integration (not done yet).
 - add UsageHints.

Gerd Hoffmann (6):
  move ObjectClass to typedefs.h
  add QemuSupportState
  Use QemuSupportState for machine types.
  Warn on obsolete and deprecated devices.
  tag cirrus as obsolete
  add UsageHints to QemuSupportState

 include/hw/boards.h          |  3 ---
 include/qemu/support-state.h | 18 +++++++++++++
 include/qemu/typedefs.h      |  1 +
 include/qom/object.h         |  4 ++-
 hw/core/qdev.c               |  8 +++++-
 hw/display/cirrus_vga.c      |  3 +++
 hw/display/cirrus_vga_isa.c  |  3 +++
 hw/i386/pc_piix.c            |  5 +++-
 hw/ppc/prep.c                |  5 +++-
 qdev-monitor.c               |  9 +++++++
 util/support-state.c         | 26 +++++++++++++++++++
 vl.c                         |  7 ++---
 qapi/common.json             | 61 ++++++++++++++++++++++++++++++++++++++++++++
 util/Makefile.objs           |  1 +
 14 files changed, 144 insertions(+), 10 deletions(-)
 create mode 100644 include/qemu/support-state.h
 create mode 100644 util/support-state.c

-- 
2.9.3

[Qemu-devel] [PATCH v2 1/6] move ObjectClass to typedefs.h

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/qemu/typedefs.h | 1 +
 include/qom/object.h    | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 3ec0e13a96..fed53f6de2 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -62,6 +62,7 @@ typedef struct NetClientState NetClientState;
 typedef struct NetFilterState NetFilterState;
 typedef struct NICInfo NICInfo;
 typedef struct NumaNodeMem NumaNodeMem;
+typedef struct ObjectClass ObjectClass;
 typedef struct PCIBridge PCIBridge;
 typedef struct PCIBus PCIBus;
 typedef struct PCIDevice PCIDevice;
diff --git a/include/qom/object.h b/include/qom/object.h
index f0b0bf39cc..499e1fd8b7 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -20,7 +20,6 @@
 struct TypeImpl;
 typedef struct TypeImpl *Type;
 
-typedef struct ObjectClass ObjectClass;
 typedef struct Object Object;
 
 typedef struct TypeInfo TypeInfo;
-- 
2.9.3

Re: [Qemu-devel] [PATCH v2 1/6] move ObjectClass to typedefs.h

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1391 bytes --]

On Tue, Nov 06, 2018 at 11:23:30AM +0100, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  include/qemu/typedefs.h | 1 +
>  include/qom/object.h    | 1 -
>  2 files changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
> index 3ec0e13a96..fed53f6de2 100644
> --- a/include/qemu/typedefs.h
> +++ b/include/qemu/typedefs.h
> @@ -62,6 +62,7 @@ typedef struct NetClientState NetClientState;
>  typedef struct NetFilterState NetFilterState;
>  typedef struct NICInfo NICInfo;
>  typedef struct NumaNodeMem NumaNodeMem;
> +typedef struct ObjectClass ObjectClass;
>  typedef struct PCIBridge PCIBridge;
>  typedef struct PCIBus PCIBus;
>  typedef struct PCIDevice PCIDevice;
> diff --git a/include/qom/object.h b/include/qom/object.h
> index f0b0bf39cc..499e1fd8b7 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -20,7 +20,6 @@
>  struct TypeImpl;
>  typedef struct TypeImpl *Type;
>  
> -typedef struct ObjectClass ObjectClass;
>  typedef struct Object Object;
>  
>  typedef struct TypeInfo TypeInfo;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] [PATCH v2 1/6] move ObjectClass to typedefs.h

[Philippe Mathieu-Daudé]

On 6/11/18 11:23, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>   include/qemu/typedefs.h | 1 +
>   include/qom/object.h    | 1 -
>   2 files changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
> index 3ec0e13a96..fed53f6de2 100644
> --- a/include/qemu/typedefs.h
> +++ b/include/qemu/typedefs.h
> @@ -62,6 +62,7 @@ typedef struct NetClientState NetClientState;
>   typedef struct NetFilterState NetFilterState;
>   typedef struct NICInfo NICInfo;
>   typedef struct NumaNodeMem NumaNodeMem;
> +typedef struct ObjectClass ObjectClass;
>   typedef struct PCIBridge PCIBridge;
>   typedef struct PCIBus PCIBus;
>   typedef struct PCIDevice PCIDevice;
> diff --git a/include/qom/object.h b/include/qom/object.h
> index f0b0bf39cc..499e1fd8b7 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -20,7 +20,6 @@
>   struct TypeImpl;
>   typedef struct TypeImpl *Type;
>   
> -typedef struct ObjectClass ObjectClass;
>   typedef struct Object Object;
>   
>   typedef struct TypeInfo TypeInfo;
> 

[Qemu-devel] [PATCH v2 2/6] add QemuSupportState

[Gerd Hoffmann]

Indicates support state for something (device, backend, subsystem, ...)
in qemu.  Add QemuSupportState field to ObjectClass.  Add some support
code.

TODO: wire up to qom-list-types

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/qemu/support-state.h | 17 +++++++++++++++++
 include/qom/object.h         |  3 +++
 util/support-state.c         | 26 ++++++++++++++++++++++++++
 qapi/common.json             | 32 ++++++++++++++++++++++++++++++++
 util/Makefile.objs           |  1 +
 5 files changed, 79 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..6567d8702b
--- /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   *help;
+} QemuSupportState;
+
+void qemu_warn_support_state(const char *type, const char *name,
+                             ObjectClass *oc);
+
+bool qemu_is_deprecated(ObjectClass *oc);
+bool qemu_is_obsolete(ObjectClass *oc);
+
+#endif /* QEMU_SUPPORT_STATE_H */
diff --git a/include/qom/object.h b/include/qom/object.h
index 499e1fd8b7..2b5f27bf85 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -15,6 +15,7 @@
 #define QEMU_OBJECT_H
 
 #include "qapi/qapi-builtin-types.h"
+#include "qemu/support-state.h"
 #include "qemu/queue.h"
 
 struct TypeImpl;
@@ -399,6 +400,8 @@ struct ObjectClass
     ObjectUnparent *unparent;
 
     GHashTable *properties;
+
+    QemuSupportState supported;
 };
 
 /**
diff --git a/util/support-state.c b/util/support-state.c
new file mode 100644
index 0000000000..3170dbd15d
--- /dev/null
+++ b/util/support-state.c
@@ -0,0 +1,26 @@
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "qemu/support-state.h"
+#include "qom/object.h"
+
+void qemu_warn_support_state(const char *type, const char *name,
+                             ObjectClass *oc)
+{
+    const char *help = oc->supported.help;
+
+    warn_report("%s %s is %s%s%s%s", type, name,
+                SupportState_str(oc->supported.state),
+                help ? " (" : "",
+                help ? help : "",
+                help ? ")"  : "");
+}
+
+bool qemu_is_deprecated(ObjectClass *oc)
+{
+    return oc->supported.state == SUPPORT_STATE_DEPRECATED;
+}
+
+bool qemu_is_obsolete(ObjectClass *oc)
+{
+    return oc->supported.state == SUPPORT_STATE_OBSOLETE;
+}
diff --git a/qapi/common.json b/qapi/common.json
index 021174f04e..00374127b8 100644
--- a/qapi/common.json
+++ b/qapi/common.json
@@ -151,3 +151,35 @@
              'ppc64', 'riscv32', 'riscv64', 's390x', 'sh4',
              'sh4eb', 'sparc', 'sparc64', 'tricore', 'unicore32',
              'x86_64', 'xtensa', 'xtensaeb' ] }
+
+##
+# @SupportState:
+#
+# Indicate Support level of qemu devices, backends, subsystems, ...
+#
+# @unspecified: not specified (zero-initialized).
+#
+# @experimental: in development, can be unstable or incomplete.
+#
+# @supported: works stable and is fully supported.
+#             (supported + maintained in MAINTAINERS).
+#
+# @unsupported: should work, support is weak or not present.
+#               (odd-fixes + orphan in MAINTAINERS).
+#
+# @obsolete: is obsolete, still present for compatibility reasons,
+#            will likely be removed at some point in the future.
+#            Not deprecated (yet).
+#            (obsolete in MAINTAINERS).
+#
+# @deprecated: is deprecated, according to qemu deprecation policy.
+#
+# Since: 3.2
+##
+{ 'enum': 'SupportState',
+  'data': [ 'unspecified',
+            'experimental',
+            'supported',
+            'unsupported',
+            '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

Re: [Qemu-devel] [PATCH v2 2/6] add QemuSupportState

[Eduardo Habkost]

On Tue, Nov 06, 2018 at 11:23:31AM +0100, Gerd Hoffmann wrote:
> Indicates support state for something (device, backend, subsystem, ...)
> in qemu.  Add QemuSupportState field to ObjectClass.  Add some support
> code.
> 
> TODO: wire up to qom-list-types
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
[...]
> +##
> +# @SupportState:
> +#
> +# Indicate Support level of qemu devices, backends, subsystems, ...
> +#
> +# @unspecified: not specified (zero-initialized).
> +#
> +# @experimental: in development, can be unstable or incomplete.

People reading this document would ask: what would appear on
MAINTAINERS if SupportState is `experimental`?

> +#
> +# @supported: works stable and is fully supported.
> +#             (supported + maintained in MAINTAINERS).
> +#
> +# @unsupported: should work, support is weak or not present.
> +#               (odd-fixes + orphan in MAINTAINERS).

What's the difference in practice between unsupported and
experimental?

> +#
> +# @obsolete: is obsolete, still present for compatibility reasons,
> +#            will likely be removed at some point in the future.
> +#            Not deprecated (yet).
> +#            (obsolete in MAINTAINERS).
> +#
> +# @deprecated: is deprecated, according to qemu deprecation policy.

I believe we want to differentiate "deprecated, but still safe to
use in production if you have a migration plan" from "deprecated,
and also unstable and unsafe for production".

I expect enterprise distributions to have a strict policy of not
allowing unsupported and experimental devices to be enabled, but
still allow deprecated devices to be enabled (but only if they
are stable/supported).

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 2/6] add QemuSupportState

[Paolo Bonzini]

On 06/11/2018 15:26, Eduardo Habkost wrote:
> On Tue, Nov 06, 2018 at 11:23:31AM +0100, Gerd Hoffmann wrote:
>> Indicates support state for something (device, backend, subsystem, ...)
>> in qemu.  Add QemuSupportState field to ObjectClass.  Add some support
>> code.
>>
>> TODO: wire up to qom-list-types
>>
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>> ---
> [...]
>> +##
>> +# @SupportState:
>> +#
>> +# Indicate Support level of qemu devices, backends, subsystems, ...
>> +#
>> +# @unspecified: not specified (zero-initialized).
>> +#
>> +# @experimental: in development, can be unstable or incomplete.
> 
> People reading this document would ask: what would appear on
> MAINTAINERS if SupportState is `experimental`?

Probably Maintained.  It's something that is on its way towards becoming
"supported", but still too immature ("unstable or incomplete").

>> +#
>> +# @supported: works stable and is fully supported.
>> +#             (supported + maintained in MAINTAINERS).
>> +#
>> +# @unsupported: should work, support is weak or not present.
>> +#               (odd-fixes + orphan in MAINTAINERS).
> 
> What's the difference in practice between unsupported and
> experimental?
> 
>> +#
>> +# @obsolete: is obsolete, still present for compatibility reasons,
>> +#            will likely be removed at some point in the future.

I am not sure this is necessarily true.  I don't see Cirrus or adlib or
pcnet disappearing anytime soon.

Paolo

>> +#            Not deprecated (yet).
>> +#            (obsolete in MAINTAINERS).
>> +#
>> +# @deprecated: is deprecated, according to qemu deprecation policy.
> 
> I believe we want to differentiate "deprecated, but still safe to
> use in production if you have a migration plan" from "deprecated,
> and also unstable and unsafe for production".
> 
> I expect enterprise distributions to have a strict policy of not
> allowing unsupported and experimental devices to be enabled, but
> still allow deprecated devices to be enabled (but only if they
> are stable/supported).
> 

Re: [Qemu-devel] [PATCH v2 2/6] add QemuSupportState

[Gerd Hoffmann]

  Hi,

> >> +# @experimental: in development, can be unstable or incomplete.
> > 
> > People reading this document would ask: what would appear on
> > MAINTAINERS if SupportState is `experimental`?
> 
> Probably Maintained.  It's something that is on its way towards becoming
> "supported", but still too immature ("unstable or incomplete").

Yep.  For the user there isn't much of a difference between
"experimental" and "unsupported" though, except that "experimental"
indicates that someone is working on it and the chances are higher that
it'll transition to "supported" at some point in the future.  So I'm not
fully sure it makes sense to list that as separate category.

> >> +# @obsolete: is obsolete, still present for compatibility reasons,
> >> +#            will likely be removed at some point in the future.
> 
> I am not sure this is necessarily true.  I don't see Cirrus or adlib or
> pcnet disappearing anytime soon.

So s/will likely/might/ ?

cheers,
  Gerd

[Qemu-devel] [PATCH v2 3/6] Use QemuSupportState for machine types.

[Gerd Hoffmann]

Switch over the current deprecation_reason users to use
the QemuSupportState field in ObjectClass instead.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/hw/boards.h | 3 ---
 hw/i386/pc_piix.c   | 5 ++++-
 hw/ppc/prep.c       | 5 ++++-
 vl.c                | 7 ++++---
 4 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index f82f28468b..b7bb181fc8 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -106,8 +106,6 @@ typedef struct {
 
 /**
  * MachineClass:
- * @deprecation_reason: If set, the machine is marked as deprecated. The
- *    string should provide some clear information about what to use instead.
  * @max_cpus: maximum number of CPUs supported. Default: 1
  * @min_cpus: minimum number of CPUs supported. Default: 1
  * @default_cpus: number of CPUs instantiated if none are specified. Default: 1
@@ -167,7 +165,6 @@ struct MachineClass {
     char *name;
     const char *alias;
     const char *desc;
-    const char *deprecation_reason;
 
     void (*init)(MachineState *state);
     void (*reset)(void);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index dc09466b3e..407f4503a3 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -955,9 +955,12 @@ DEFINE_I440FX_MACHINE(v0_12, "pc-0.12", pc_compat_0_13,
 
 static void pc_i440fx_0_11_machine_options(MachineClass *m)
 {
+    ObjectClass *oc = OBJECT_CLASS(m);
+
     pc_i440fx_0_12_machine_options(m);
     m->hw_version = "0.11";
-    m->deprecation_reason = "use a newer machine type instead";
+    oc->supported.state = SUPPORT_STATE_DEPRECATED;
+    oc->supported.help = "use a newer machine type instead";
     SET_MACHINE_COMPAT(m, PC_COMPAT_0_11);
 }
 
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 2afb7f437e..260700847b 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -588,7 +588,10 @@ static void ppc_prep_init(MachineState *machine)
 
 static void prep_machine_init(MachineClass *mc)
 {
-    mc->deprecation_reason = "use 40p machine type instead";
+    ObjectClass *oc = OBJECT_CLASS(mc);
+
+    oc->supported.state = SUPPORT_STATE_DEPRECATED;
+    oc->supported.help = "use 40p machine type instead";
     mc->desc = "PowerPC PREP platform";
     mc->init = ppc_prep_init;
     mc->block_default_type = IF_IDE;
diff --git a/vl.c b/vl.c
index 1fcacc5caa..ba81fefb66 100644
--- a/vl.c
+++ b/vl.c
@@ -2573,7 +2573,7 @@ static gint machine_class_cmp(gconstpointer a, gconstpointer b)
             }
             printf("%-20s %s%s%s\n", mc->name, mc->desc,
                    mc->is_default ? " (default)" : "",
-                   mc->deprecation_reason ? " (deprecated)" : "");
+                   qemu_is_deprecated(OBJECT_CLASS(mc)) ? " (deprecated)" : "");
         }
     }
 
@@ -4290,9 +4290,10 @@ int main(int argc, char **argv, char **envp)
 
     configure_accelerator(current_machine);
 
-    if (!qtest_enabled() && machine_class->deprecation_reason) {
+    if (!qtest_enabled() && qemu_is_deprecated(OBJECT_CLASS(machine_class))) {
         error_report("Machine type '%s' is deprecated: %s",
-                     machine_class->name, machine_class->deprecation_reason);
+                     machine_class->name,
+                     OBJECT_CLASS(machine_class)->supported.help);
     }
 
     /*
-- 
2.9.3

Re: [Qemu-devel] [PATCH v2 3/6] Use QemuSupportState for machine types.

[Markus Armbruster]

Gerd Hoffmann <kraxel@redhat.com> writes:

> Switch over the current deprecation_reason users to use
> the QemuSupportState field in ObjectClass instead.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  include/hw/boards.h | 3 ---
>  hw/i386/pc_piix.c   | 5 ++++-
>  hw/ppc/prep.c       | 5 ++++-
>  vl.c                | 7 ++++---
>  4 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index f82f28468b..b7bb181fc8 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -106,8 +106,6 @@ typedef struct {
>  
>  /**
>   * MachineClass:
> - * @deprecation_reason: If set, the machine is marked as deprecated. The
> - *    string should provide some clear information about what to use instead.
>   * @max_cpus: maximum number of CPUs supported. Default: 1
>   * @min_cpus: minimum number of CPUs supported. Default: 1
>   * @default_cpus: number of CPUs instantiated if none are specified. Default: 1
> @@ -167,7 +165,6 @@ struct MachineClass {
>      char *name;
>      const char *alias;
>      const char *desc;
> -    const char *deprecation_reason;
>  
>      void (*init)(MachineState *state);
>      void (*reset)(void);
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index dc09466b3e..407f4503a3 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -955,9 +955,12 @@ DEFINE_I440FX_MACHINE(v0_12, "pc-0.12", pc_compat_0_13,
>  
>  static void pc_i440fx_0_11_machine_options(MachineClass *m)
>  {
> +    ObjectClass *oc = OBJECT_CLASS(m);
> +
>      pc_i440fx_0_12_machine_options(m);
>      m->hw_version = "0.11";
> -    m->deprecation_reason = "use a newer machine type instead";
> +    oc->supported.state = SUPPORT_STATE_DEPRECATED;
> +    oc->supported.help = "use a newer machine type instead";
>      SET_MACHINE_COMPAT(m, PC_COMPAT_0_11);
>  }
>  
> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> index 2afb7f437e..260700847b 100644
> --- a/hw/ppc/prep.c
> +++ b/hw/ppc/prep.c
> @@ -588,7 +588,10 @@ static void ppc_prep_init(MachineState *machine)
>  
>  static void prep_machine_init(MachineClass *mc)
>  {
> -    mc->deprecation_reason = "use 40p machine type instead";
> +    ObjectClass *oc = OBJECT_CLASS(mc);
> +
> +    oc->supported.state = SUPPORT_STATE_DEPRECATED;
> +    oc->supported.help = "use 40p machine type instead";
>      mc->desc = "PowerPC PREP platform";
>      mc->init = ppc_prep_init;
>      mc->block_default_type = IF_IDE;
> diff --git a/vl.c b/vl.c
> index 1fcacc5caa..ba81fefb66 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2573,7 +2573,7 @@ static gint machine_class_cmp(gconstpointer a, gconstpointer b)
>              }
>              printf("%-20s %s%s%s\n", mc->name, mc->desc,
>                     mc->is_default ? " (default)" : "",
> -                   mc->deprecation_reason ? " (deprecated)" : "");
> +                   qemu_is_deprecated(OBJECT_CLASS(mc)) ? " (deprecated)" : "");
>          }
>      }
>  
> @@ -4290,9 +4290,10 @@ int main(int argc, char **argv, char **envp)
>  
>      configure_accelerator(current_machine);
>  
> -    if (!qtest_enabled() && machine_class->deprecation_reason) {
> +    if (!qtest_enabled() && qemu_is_deprecated(OBJECT_CLASS(machine_class))) {
>          error_report("Machine type '%s' is deprecated: %s",
> -                     machine_class->name, machine_class->deprecation_reason);
> +                     machine_class->name,
> +                     OBJECT_CLASS(machine_class)->supported.help);
>      }
>  
>      /*

Let's switch to warn_report() while we're touching it.

[Qemu-devel] [PATCH v2 4/6] Warn on obsolete and deprecated devices.

[Gerd Hoffmann]

Print a warning for deprecated and obsolete devices.
Also add support state to device listing.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/core/qdev.c | 8 +++++++-
 qdev-monitor.c | 9 +++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 6b3cc55b27..6205522c3e 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -133,11 +133,17 @@ DeviceState *qdev_create(BusState *bus, const char *name)
 
 DeviceState *qdev_try_create(BusState *bus, const char *type)
 {
+    ObjectClass *oc;
     DeviceState *dev;
 
-    if (object_class_by_name(type) == NULL) {
+    oc = object_class_by_name(type);
+    if (oc == NULL) {
         return NULL;
     }
+    if (qemu_is_deprecated(oc) ||
+        qemu_is_obsolete(oc)) {
+        qemu_warn_support_state("device", type, oc);
+    }
     dev = DEVICE(object_new(type));
     if (!dev) {
         return NULL;
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 802c18a74e..80370372f9 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -115,6 +115,8 @@ static void out_printf(const char *fmt, ...)
 
 static void qdev_print_devinfo(DeviceClass *dc)
 {
+    ObjectClass *oc = OBJECT_CLASS(dc);
+
     out_printf("name \"%s\"", object_class_get_name(OBJECT_CLASS(dc)));
     if (dc->bus_type) {
         out_printf(", bus %s", dc->bus_type);
@@ -128,6 +130,9 @@ static void qdev_print_devinfo(DeviceClass *dc)
     if (!dc->user_creatable) {
         out_printf(", no-user");
     }
+    if (oc->supported.state != SUPPORT_STATE_UNSPECIFIED) {
+        out_printf(", %s", SupportState_str(oc->supported.state));
+    }
     out_printf("\n");
 }
 
@@ -579,6 +584,10 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
     if (!dc) {
         return NULL;
     }
+    if (qemu_is_deprecated(OBJECT_CLASS(dc)) ||
+        qemu_is_obsolete(OBJECT_CLASS(dc))) {
+        qemu_warn_support_state("device", driver, OBJECT_CLASS(dc));
+    }
 
     /* find bus */
     path = qemu_opt_get(opts, "bus");
-- 
2.9.3

Re: [Qemu-devel] [PATCH v2 4/6] Warn on obsolete and deprecated devices.

[Michael S. Tsirkin]

On Tue, Nov 06, 2018 at 11:23:33AM +0100, Gerd Hoffmann wrote:
> Print a warning for deprecated and obsolete devices.
> Also add support state to device listing.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Should we also add a runtime flag to block these?
E.g. I can see libvirt doing that and passing
the responsibility for what is reasonably
supported to qemu.

> ---
>  hw/core/qdev.c | 8 +++++++-
>  qdev-monitor.c | 9 +++++++++
>  2 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 6b3cc55b27..6205522c3e 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -133,11 +133,17 @@ DeviceState *qdev_create(BusState *bus, const char *name)
>  
>  DeviceState *qdev_try_create(BusState *bus, const char *type)
>  {
> +    ObjectClass *oc;
>      DeviceState *dev;
>  
> -    if (object_class_by_name(type) == NULL) {
> +    oc = object_class_by_name(type);
> +    if (oc == NULL) {
>          return NULL;
>      }
> +    if (qemu_is_deprecated(oc) ||
> +        qemu_is_obsolete(oc)) {
> +        qemu_warn_support_state("device", type, oc);
> +    }
>      dev = DEVICE(object_new(type));
>      if (!dev) {
>          return NULL;
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index 802c18a74e..80370372f9 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -115,6 +115,8 @@ static void out_printf(const char *fmt, ...)
>  
>  static void qdev_print_devinfo(DeviceClass *dc)
>  {
> +    ObjectClass *oc = OBJECT_CLASS(dc);
> +
>      out_printf("name \"%s\"", object_class_get_name(OBJECT_CLASS(dc)));
>      if (dc->bus_type) {
>          out_printf(", bus %s", dc->bus_type);
> @@ -128,6 +130,9 @@ static void qdev_print_devinfo(DeviceClass *dc)
>      if (!dc->user_creatable) {
>          out_printf(", no-user");
>      }
> +    if (oc->supported.state != SUPPORT_STATE_UNSPECIFIED) {
> +        out_printf(", %s", SupportState_str(oc->supported.state));
> +    }
>      out_printf("\n");
>  }
>  
> @@ -579,6 +584,10 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
>      if (!dc) {
>          return NULL;
>      }
> +    if (qemu_is_deprecated(OBJECT_CLASS(dc)) ||
> +        qemu_is_obsolete(OBJECT_CLASS(dc))) {
> +        qemu_warn_support_state("device", driver, OBJECT_CLASS(dc));
> +    }
>  
>      /* find bus */
>      path = qemu_opt_get(opts, "bus");
> -- 
> 2.9.3

Re: [Qemu-devel] [PATCH v2 4/6] Warn on obsolete and deprecated devices.

[Gerd Hoffmann]

On Tue, Nov 06, 2018 at 09:36:44AM -0500, Michael S. Tsirkin wrote:
> On Tue, Nov 06, 2018 at 11:23:33AM +0100, Gerd Hoffmann wrote:
> > Print a warning for deprecated and obsolete devices.
> > Also add support state to device listing.
> > 
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> 
> Should we also add a runtime flag to block these?

Yes, probably.  My priority is to hash out the classification
first though, so it isn't in this series yet.

On the cmd line switches:  How much flexibility do we want on this?  We
can go for a simple -future switch, as suggested by armbru, disallowing
obsolete and deprecated devices.  We could go for a full-blown ...

  -policy {experimental,unsupported,...}={enable,warn,disable}

... or something inbetween.

Comments?  Other ideas?

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v2 4/6] Warn on obsolete and deprecated devices.

[Markus Armbruster]

Gerd Hoffmann <kraxel@redhat.com> writes:

> Print a warning for deprecated and obsolete devices.
> Also add support state to device listing.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/core/qdev.c | 8 +++++++-
>  qdev-monitor.c | 9 +++++++++
>  2 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 6b3cc55b27..6205522c3e 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -133,11 +133,17 @@ DeviceState *qdev_create(BusState *bus, const char *name)
>  
>  DeviceState *qdev_try_create(BusState *bus, const char *type)
>  {
> +    ObjectClass *oc;
>      DeviceState *dev;
>  
> -    if (object_class_by_name(type) == NULL) {
> +    oc = object_class_by_name(type);
> +    if (oc == NULL) {
>          return NULL;
>      }
> +    if (qemu_is_deprecated(oc) ||
> +        qemu_is_obsolete(oc)) {
> +        qemu_warn_support_state("device", type, oc);

Looks like this:

    $ qemu-system-x86_64 -nodefaults -S -display none -device cirrus-vga
    qemu-system-x86_64: -device cirrus-vga: warning: device cirrus-vga is obsolete (use "-vga std" instead, see https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful/)

I'd prefer:

    $ qemu-system-x86_64 -nodefaults -S -display none -device cirrus-vga
    qemu-system-x86_64: -device cirrus-vga: warning: device cirrus-vga is obsolete
    Use "-vga std" instead, see
    https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful/

The obvious way to get it:

   void warn_support_state(const char *type, const char *name,
                           ObjectClass *oc)
   {
       const char *help = oc->supported.help;

       warn_report("%s %s is %s", type, name,
                   SupportState_str(oc->supported.state));
       if (help) {
           error_printf("%s\n", help);
       }
   }

with the ->help suitably capitalized and formatted.

That should make qemu_warn_support_state() usable for the previous
patch, too.

Note I scratched the qemu_ prefix.  Matter of taste, I guess.


> +    }
>      dev = DEVICE(object_new(type));
>      if (!dev) {
>          return NULL;
[...]

[Qemu-devel] [PATCH v2 5/6] tag cirrus as obsolete

[Gerd Hoffmann]

Standard VGA should be used instead.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/cirrus_vga.c     | 3 +++
 hw/display/cirrus_vga_isa.c | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index d9b854d74d..8474415687 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -3024,6 +3024,9 @@ static void cirrus_vga_class_init(ObjectClass *klass, void *data)
     dc->vmsd = &vmstate_pci_cirrus_vga;
     dc->props = pci_vga_cirrus_properties;
     dc->hotpluggable = false;
+    klass->supported.state = SUPPORT_STATE_OBSOLETE;
+    klass->supported.help = "use \"-vga std\" instead, see "
+        "https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful/";
 }
 
 static const TypeInfo cirrus_vga_info = {
diff --git a/hw/display/cirrus_vga_isa.c b/hw/display/cirrus_vga_isa.c
index fa10b74230..6bc83f7509 100644
--- a/hw/display/cirrus_vga_isa.c
+++ b/hw/display/cirrus_vga_isa.c
@@ -81,6 +81,9 @@ static void isa_cirrus_vga_class_init(ObjectClass *klass, void *data)
     dc->realize = isa_cirrus_vga_realizefn;
     dc->props = isa_cirrus_vga_properties;
     set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories);
+    klass->supported.state = SUPPORT_STATE_OBSOLETE;
+    klass->supported.help = "use \"-vga std\" instead, see "
+        "https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful/";
 }
 
 static const TypeInfo isa_cirrus_vga_info = {
-- 
2.9.3

[Qemu-devel] [PATCH v2 6/6] add UsageHints to QemuSupportState

[Gerd Hoffmann]

So we can add device usage recommendations to devices,
independant from support state.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/qemu/support-state.h |  1 +
 qapi/common.json             | 29 +++++++++++++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/include/qemu/support-state.h b/include/qemu/support-state.h
index 6567d8702b..6ea0d03bd5 100644
--- a/include/qemu/support-state.h
+++ b/include/qemu/support-state.h
@@ -5,6 +5,7 @@
 
 typedef struct QemuSupportState {
     SupportState state;
+    UsageHints   hints;
     const char   *help;
 } QemuSupportState;
 
diff --git a/qapi/common.json b/qapi/common.json
index 00374127b8..6952ea2074 100644
--- a/qapi/common.json
+++ b/qapi/common.json
@@ -183,3 +183,32 @@
             'unsupported',
             'obsolete',
             'deprecated' ] }
+
+##
+# @UsageHints:
+#
+# Usage recommendations.
+#
+# @unspecified: not specified (zero-initialized).
+#
+# @green: Best choice.  Typically paravirtual devices go into this
+#         category.
+#         Example (nic): virtio-net.
+#         Example (usb); xhci.
+#
+# @yellow: Reasonable choice.  Typically emulated devices with
+#          good performance go into this category.
+#          Example (nic): e1000, e1000e
+#
+# @red: Bad choice.  Avoid this unless you run an old guest which
+#       lacks support for something better.
+#       Example (nic): rtl8139, pcnet, ne2k, ...
+#       Example (usb): ehci, uhci, ohci
+#
+# Since: 3.2
+##
+{ 'enum': 'UsageHints',
+  'data': [ 'unspecified',
+            'green',
+            'yellow',
+            'red' ] }
-- 
2.9.3

Re: [Qemu-devel] [PATCH v2 6/6] add UsageHints to QemuSupportState

[Markus Armbruster]

Gerd Hoffmann <kraxel@redhat.com> writes:

> So we can add device usage recommendations to devices,
> independant from support state.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  include/qemu/support-state.h |  1 +
>  qapi/common.json             | 29 +++++++++++++++++++++++++++++
>  2 files changed, 30 insertions(+)
>
> diff --git a/include/qemu/support-state.h b/include/qemu/support-state.h
> index 6567d8702b..6ea0d03bd5 100644
> --- a/include/qemu/support-state.h
> +++ b/include/qemu/support-state.h
> @@ -5,6 +5,7 @@
>  
>  typedef struct QemuSupportState {
>      SupportState state;
> +    UsageHints   hints;
>      const char   *help;
>  } QemuSupportState;
>  
> diff --git a/qapi/common.json b/qapi/common.json
> index 00374127b8..6952ea2074 100644
> --- a/qapi/common.json
> +++ b/qapi/common.json
> @@ -183,3 +183,32 @@
>              'unsupported',
>              'obsolete',
>              'deprecated' ] }
> +
> +##
> +# @UsageHints:
> +#
> +# Usage recommendations.
> +#
> +# @unspecified: not specified (zero-initialized).

(zero-initialized) is an implementation detail, which in my opinion
doesn't belong here.  Same in PATCH 2.

> +#
> +# @green: Best choice.  Typically paravirtual devices go into this
> +#         category.
> +#         Example (nic): virtio-net.
> +#         Example (usb); xhci.
> +#
> +# @yellow: Reasonable choice.  Typically emulated devices with
> +#          good performance go into this category.
> +#          Example (nic): e1000, e1000e
> +#
> +# @red: Bad choice.  Avoid this unless you run an old guest which
> +#       lacks support for something better.
> +#       Example (nic): rtl8139, pcnet, ne2k, ...
> +#       Example (usb): ehci, uhci, ohci
> +#
> +# Since: 3.2
> +##
> +{ 'enum': 'UsageHints',
> +  'data': [ 'unspecified',
> +            'green',
> +            'yellow',
> +            'red' ] }

Simple.  I like simple, as long as it's good enough.  Hard to tell
without actual users.

Re: [Qemu-devel] [PATCH v2 0/6] Introducing QemuSupportState

[Markus Armbruster]

This series is a step from treating deprecation completely ad hoc to a
more structured approach.  I like it.  I guess we'll have to extend it.
For instance, we deprecate not only objects, but also members and even
values of members.  Members should be straightforward, values probably
not.  But that's not an argument against this series.