Libguestfs - Oct 2021 - [PATCH 5/9] qapi: Generalize struct member policy checking

The generated visitor functions call visit_deprecated_accept() and
visit_deprecated() when visiting a struct member with special feature
flag 'deprecated'.  This makes the feature flag visible to the actual
visitors.  I want to make feature flag 'unstable' visible there as
well, so I can add policy for it.

To let me make it visible, replace these functions by
visit_policy_reject() and visit_policy_skip(), which take the member's
special features as an argument.  Note that the new functions have the
opposite sense, i.e. the return value flips.

Signed-off-by: Markus Armbruster <armbru at redhat.com>
---
 include/qapi/visitor-impl.h   |  6 ++++--
 include/qapi/visitor.h        | 17 +++++++++++++----
 qapi/qapi-forward-visitor.c   | 16 +++++++++-------
 qapi/qapi-visit-core.c        | 22 ++++++++++++----------
 qapi/qobject-input-visitor.c  | 15 ++++++++++-----
 qapi/qobject-output-visitor.c |  9 ++++++---
 qapi/trace-events             |  4 ++--
 scripts/qapi/visit.py         | 14 +++++++-------
 8 files changed, 63 insertions(+), 40 deletions(-)

diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 72b6537bef..2badec5ba4 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -114,10 +114,12 @@ struct Visitor
     void (*optional)(Visitor *v, const char *name, bool *present);
 
     /* Optional */
-    bool (*deprecated_accept)(Visitor *v, const char *name, Error **errp);
+    bool (*policy_reject)(Visitor *v, const char *name,
+                          unsigned special_features, Error **errp);
 
     /* Optional */
-    bool (*deprecated)(Visitor *v, const char *name);
+    bool (*policy_skip)(Visitor *v, const char *name,
+                        unsigned special_features);
 
     /* Must be set */
     VisitorType type;
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index dcb96018a9..d53a84c9ba 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -461,22 +461,31 @@ void visit_end_alternate(Visitor *v, void **obj);
 bool visit_optional(Visitor *v, const char *name, bool *present);
 
 /*
- * Should we reject deprecated member @name?
+ * Should we reject member @name due to policy?
+ *
+ * @special_features is the member's special features encoded as a
+ * bitset of QapiSpecialFeature.
  *
  * @name must not be NULL.  This function is only useful between
  * visit_start_struct() and visit_end_struct(), since only objects
  * have deprecated members.
  */
-bool visit_deprecated_accept(Visitor *v, const char *name, Error **errp);
+bool visit_policy_reject(Visitor *v, const char *name,
+                         unsigned special_features, Error **errp);
 
 /*
- * Should we visit deprecated member @name?
+ *
+ * Should we skip member @name due to policy?
+ *
+ * @special_features is the member's special features encoded as a
+ * bitset of QapiSpecialFeature.
  *
  * @name must not be NULL.  This function is only useful between
  * visit_start_struct() and visit_end_struct(), since only objects
  * have deprecated members.
  */
-bool visit_deprecated(Visitor *v, const char *name);
+bool visit_policy_skip(Visitor *v, const char *name,
+                       unsigned special_features);
 
 /*
  * Set policy for handling deprecated management interfaces.
diff --git a/qapi/qapi-forward-visitor.c b/qapi/qapi-forward-visitor.c
index a4b111e22a..25d098aa8a 100644
--- a/qapi/qapi-forward-visitor.c
+++ b/qapi/qapi-forward-visitor.c
@@ -246,25 +246,27 @@ static void forward_field_optional(Visitor *v, const char
*name, bool *present)
     visit_optional(ffv->target, name, present);
 }
 
-static bool forward_field_deprecated_accept(Visitor *v, const char *name,
-                                            Error **errp)
+static bool forward_field_policy_reject(Visitor *v, const char *name,
+                                        unsigned special_features,
+                                        Error **errp)
 {
     ForwardFieldVisitor *ffv = to_ffv(v);
 
     if (!forward_field_translate_name(ffv, &name, errp)) {
         return false;
     }
-    return visit_deprecated_accept(ffv->target, name, errp);
+    return visit_policy_reject(ffv->target, name, special_features, errp);
 }
 
-static bool forward_field_deprecated(Visitor *v, const char *name)
+static bool forward_field_policy_skip(Visitor *v, const char *name,
+                                      unsigned special_features)
 {
     ForwardFieldVisitor *ffv = to_ffv(v);
 
     if (!forward_field_translate_name(ffv, &name, NULL)) {
         return false;
     }
-    return visit_deprecated(ffv->target, name);
+    return visit_policy_skip(ffv->target, name, special_features);
 }
 
 static void forward_field_complete(Visitor *v, void *opaque)
@@ -313,8 +315,8 @@ Visitor *visitor_forward_field(Visitor *target, const char
*from, const char *to
     v->visitor.type_any = forward_field_type_any;
     v->visitor.type_null = forward_field_type_null;
     v->visitor.optional = forward_field_optional;
-    v->visitor.deprecated_accept = forward_field_deprecated_accept;
-    v->visitor.deprecated = forward_field_deprecated;
+    v->visitor.policy_reject = forward_field_policy_reject;
+    v->visitor.policy_skip = forward_field_policy_skip;
     v->visitor.complete = forward_field_complete;
     v->visitor.free = forward_field_free;
 
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 49136ae88e..b4a81f1757 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -139,22 +139,24 @@ bool visit_optional(Visitor *v, const char *name, bool
*present)
     return *present;
 }
 
-bool visit_deprecated_accept(Visitor *v, const char *name, Error **errp)
+bool visit_policy_reject(Visitor *v, const char *name,
+                         unsigned special_features, Error **errp)
 {
-    trace_visit_deprecated_accept(v, name);
-    if (v->deprecated_accept) {
-        return v->deprecated_accept(v, name, errp);
+    trace_visit_policy_reject(v, name);
+    if (v->policy_reject) {
+        return v->policy_reject(v, name, special_features, errp);
     }
-    return true;
+    return false;
 }
 
-bool visit_deprecated(Visitor *v, const char *name)
+bool visit_policy_skip(Visitor *v, const char *name,
+                       unsigned special_features)
 {
-    trace_visit_deprecated(v, name);
-    if (v->deprecated) {
-        return v->deprecated(v, name);
+    trace_visit_policy_skip(v, name);
+    if (v->policy_skip) {
+        return v->policy_skip(v, name, special_features);
     }
-    return true;
+    return false;
 }
 
 void visit_set_policy(Visitor *v, CompatPolicy *policy)
diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index 71b24a4429..fda485614b 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -662,16 +662,21 @@ static void qobject_input_optional(Visitor *v, const char
*name, bool *present)
     *present = true;
 }
 
-static bool qobject_input_deprecated_accept(Visitor *v, const char *name,
-                                            Error **errp)
+static bool qobject_input_policy_reject(Visitor *v, const char *name,
+                                        unsigned special_features,
+                                        Error **errp)
 {
+    if (!(special_features && 1u << QAPI_DEPRECATED)) {
+        return false;
+    }
+
     switch (v->compat_policy.deprecated_input) {
     case COMPAT_POLICY_INPUT_ACCEPT:
-        return true;
+        return false;
     case COMPAT_POLICY_INPUT_REJECT:
         error_setg(errp, "Deprecated parameter '%s' disabled by
policy",
                    name);
-        return false;
+        return true;
     case COMPAT_POLICY_INPUT_CRASH:
     default:
         abort();
@@ -712,7 +717,7 @@ static QObjectInputVisitor
*qobject_input_visitor_base_new(QObject *obj)
     v->visitor.end_list = qobject_input_end_list;
     v->visitor.start_alternate = qobject_input_start_alternate;
     v->visitor.optional = qobject_input_optional;
-    v->visitor.deprecated_accept = qobject_input_deprecated_accept;
+    v->visitor.policy_reject = qobject_input_policy_reject;
     v->visitor.free = qobject_input_free;
 
     v->root = qobject_ref(obj);
diff --git a/qapi/qobject-output-visitor.c b/qapi/qobject-output-visitor.c
index 9b7f510036..b5c6564cbb 100644
--- a/qapi/qobject-output-visitor.c
+++ b/qapi/qobject-output-visitor.c
@@ -13,6 +13,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qapi/compat-policy.h"
 #include "qapi/qobject-output-visitor.h"
 #include "qapi/visitor-impl.h"
 #include "qemu/queue.h"
@@ -208,9 +209,11 @@ static bool qobject_output_type_null(Visitor *v, const char
*name,
     return true;
 }
 
-static bool qobject_output_deprecated(Visitor *v, const char *name)
+static bool qobject_output_policy_skip(Visitor *v, const char *name,
+                                       unsigned special_features)
 {
-    return v->compat_policy.deprecated_output != COMPAT_POLICY_OUTPUT_HIDE;
+    return !(special_features && 1u << QAPI_DEPRECATED)
+        || v->compat_policy.deprecated_output == COMPAT_POLICY_OUTPUT_HIDE;
 }
 
 /* Finish building, and return the root object.
@@ -262,7 +265,7 @@ Visitor *qobject_output_visitor_new(QObject **result)
     v->visitor.type_number = qobject_output_type_number;
     v->visitor.type_any = qobject_output_type_any;
     v->visitor.type_null = qobject_output_type_null;
-    v->visitor.deprecated = qobject_output_deprecated;
+    v->visitor.policy_skip = qobject_output_policy_skip;
     v->visitor.complete = qobject_output_complete;
     v->visitor.free = qobject_output_free;
 
diff --git a/qapi/trace-events b/qapi/trace-events
index cccafc07e5..ab108c4f0e 100644
--- a/qapi/trace-events
+++ b/qapi/trace-events
@@ -17,8 +17,8 @@ visit_start_alternate(void *v, const char *name, void *obj,
size_t size) "v=%p n
 visit_end_alternate(void *v, void *obj) "v=%p obj=%p"
 
 visit_optional(void *v, const char *name, bool *present) "v=%p name=%s
present=%p"
-visit_deprecated_accept(void *v, const char *name) "v=%p name=%s"
-visit_deprecated(void *v, const char *name) "v=%p name=%s"
+visit_policy_reject(void *v, const char *name) "v=%p name=%s"
+visit_policy_skip(void *v, const char *name) "v=%p name=%s"
 
 visit_type_enum(void *v, const char *name, int *obj) "v=%p name=%s
obj=%p"
 visit_type_int(void *v, const char *name, int64_t *obj) "v=%p name=%s
obj=%p"
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index 9d9196a143..e13bbe4292 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -21,7 +21,7 @@
     indent,
     mcgen,
 )
-from .gen import QAPISchemaModularCVisitor, ifcontext
+from .gen import QAPISchemaModularCVisitor, gen_special_features, ifcontext
 from .schema import (
     QAPISchema,
     QAPISchemaEnumMember,
@@ -76,7 +76,6 @@ def gen_visit_object_members(name: str,
                      c_type=base.c_name())
 
     for memb in members:
-        deprecated = 'deprecated' in [f.name for f in memb.features]
         ret += memb.ifcond.gen_if()
         if memb.optional:
             ret += mcgen('''
@@ -84,14 +83,15 @@ def gen_visit_object_members(name: str,
 ''',
                          name=memb.name, c_name=c_name(memb.name))
             indent.increase()
-        if deprecated:
+        special_features = gen_special_features(memb.features)
+        if special_features != '0':
             ret += mcgen('''
-    if (!visit_deprecated_accept(v, "%(name)s", errp)) {
+    if (visit_policy_reject(v, "%(name)s", %(special_features)s,
errp)) {
         return false;
     }
-    if (visit_deprecated(v, "%(name)s")) {
+    if (!visit_policy_skip(v, "%(name)s", %(special_features)s)) {
 ''',
-                         name=memb.name)
+                         name=memb.name, special_features=special_features)
             indent.increase()
         ret += mcgen('''
     if (!visit_type_%(c_type)s(v, "%(name)s",
&obj->%(c_name)s, errp)) {
@@ -100,7 +100,7 @@ def gen_visit_object_members(name: str,
 ''',
                      c_type=memb.type.c_name(), name=memb.name,
                      c_name=c_name(memb.name))
-        if deprecated:
+        if special_features != '0':
             indent.decrease()
             ret += mcgen('''
     }
-- 
2.31.1

On Mon, Oct 25, 2021 at 1:25 AM Markus Armbruster <armbru at redhat.com>
wrote:
> The generated visitor functions call visit_deprecated_accept() and
> visit_deprecated() when visiting a struct member with special feature
> flag 'deprecated'.  This makes the feature flag visible to the
actual
> visitors.  I want to make feature flag 'unstable' visible there as
> well, so I can add policy for it.
>
> To let me make it visible, replace these functions by
> visit_policy_reject() and visit_policy_skip(), which take the member's
> special features as an argument.  Note that the new functions have the
> opposite sense, i.e. the return value flips.
>
> Signed-off-by: Markus Armbruster <armbru at redhat.com>
> ---
>  include/qapi/visitor-impl.h   |  6 ++++--
>  include/qapi/visitor.h        | 17 +++++++++++++----
>  qapi/qapi-forward-visitor.c   | 16 +++++++++-------
>  qapi/qapi-visit-core.c        | 22 ++++++++++++----------
>  qapi/qobject-input-visitor.c  | 15 ++++++++++-----
>  qapi/qobject-output-visitor.c |  9 ++++++---
>  qapi/trace-events             |  4 ++--
>  scripts/qapi/visit.py         | 14 +++++++-------
>  8 files changed, 63 insertions(+), 40 deletions(-)
>
> diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
> index 72b6537bef..2badec5ba4 100644
> --- a/include/qapi/visitor-impl.h
> +++ b/include/qapi/visitor-impl.h
> @@ -114,10 +114,12 @@ struct Visitor
>      void (*optional)(Visitor *v, const char *name, bool *present);
>
>      /* Optional */
> -    bool (*deprecated_accept)(Visitor *v, const char *name, Error **errp);
> +    bool (*policy_reject)(Visitor *v, const char *name,
> +                          unsigned special_features, Error **errp);
>
>      /* Optional */
> -    bool (*deprecated)(Visitor *v, const char *name);
> +    bool (*policy_skip)(Visitor *v, const char *name,
> +                        unsigned special_features);
>
>      /* Must be set */
>      VisitorType type;
> diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
> index dcb96018a9..d53a84c9ba 100644
> --- a/include/qapi/visitor.h
> +++ b/include/qapi/visitor.h
> @@ -461,22 +461,31 @@ void visit_end_alternate(Visitor *v, void **obj);
>  bool visit_optional(Visitor *v, const char *name, bool *present);
>
>  /*
> - * Should we reject deprecated member @name?
> + * Should we reject member @name due to policy?
> + *
> + * @special_features is the member's special features encoded as a
> + * bitset of QapiSpecialFeature.
>   *
>   * @name must not be NULL.  This function is only useful between
>   * visit_start_struct() and visit_end_struct(), since only objects
>   * have deprecated members.
>   */
> -bool visit_deprecated_accept(Visitor *v, const char *name, Error **errp);
> +bool visit_policy_reject(Visitor *v, const char *name,
> +                         unsigned special_features, Error **errp);
>
>  /*
> - * Should we visit deprecated member @name?
> + *
> + * Should we skip member @name due to policy?
> + *
> + * @special_features is the member's special features encoded as a
> + * bitset of QapiSpecialFeature.
>   *
>   * @name must not be NULL.  This function is only useful between
>   * visit_start_struct() and visit_end_struct(), since only objects
>   * have deprecated members.
>   */
> -bool visit_deprecated(Visitor *v, const char *name);
> +bool visit_policy_skip(Visitor *v, const char *name,
> +                       unsigned special_features);
>
>  /*
>   * Set policy for handling deprecated management interfaces.
> diff --git a/qapi/qapi-forward-visitor.c b/qapi/qapi-forward-visitor.c
> index a4b111e22a..25d098aa8a 100644
> --- a/qapi/qapi-forward-visitor.c
> +++ b/qapi/qapi-forward-visitor.c
> @@ -246,25 +246,27 @@ static void forward_field_optional(Visitor *v, const
> char *name, bool *present)
>      visit_optional(ffv->target, name, present);
>  }
>
> -static bool forward_field_deprecated_accept(Visitor *v, const char *name,
> -                                            Error **errp)
> +static bool forward_field_policy_reject(Visitor *v, const char *name,
> +                                        unsigned special_features,
> +                                        Error **errp)
>  {
>      ForwardFieldVisitor *ffv = to_ffv(v);
>
>      if (!forward_field_translate_name(ffv, &name, errp)) {
>          return false;
>      }
> -    return visit_deprecated_accept(ffv->target, name, errp);
> +    return visit_policy_reject(ffv->target, name, special_features,
errp);
>  }
>
> -static bool forward_field_deprecated(Visitor *v, const char *name)
> +static bool forward_field_policy_skip(Visitor *v, const char *name,
> +                                      unsigned special_features)
>  {
>      ForwardFieldVisitor *ffv = to_ffv(v);
>
>      if (!forward_field_translate_name(ffv, &name, NULL)) {
>          return false;
>      }
> -    return visit_deprecated(ffv->target, name);
> +    return visit_policy_skip(ffv->target, name, special_features);
>  }
>
>  static void forward_field_complete(Visitor *v, void *opaque)
> @@ -313,8 +315,8 @@ Visitor *visitor_forward_field(Visitor *target, const
> char *from, const char *to
>      v->visitor.type_any = forward_field_type_any;
>      v->visitor.type_null = forward_field_type_null;
>      v->visitor.optional = forward_field_optional;
> -    v->visitor.deprecated_accept = forward_field_deprecated_accept;
> -    v->visitor.deprecated = forward_field_deprecated;
> +    v->visitor.policy_reject = forward_field_policy_reject;
> +    v->visitor.policy_skip = forward_field_policy_skip;
>      v->visitor.complete = forward_field_complete;
>      v->visitor.free = forward_field_free;
>
> diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
> index 49136ae88e..b4a81f1757 100644
> --- a/qapi/qapi-visit-core.c
> +++ b/qapi/qapi-visit-core.c
> @@ -139,22 +139,24 @@ bool visit_optional(Visitor *v, const char *name,
> bool *present)
>      return *present;
>  }
>
> -bool visit_deprecated_accept(Visitor *v, const char *name, Error **errp)
> +bool visit_policy_reject(Visitor *v, const char *name,
> +                         unsigned special_features, Error **errp)
>  {
> -    trace_visit_deprecated_accept(v, name);
> -    if (v->deprecated_accept) {
> -        return v->deprecated_accept(v, name, errp);
> +    trace_visit_policy_reject(v, name);
> +    if (v->policy_reject) {
> +        return v->policy_reject(v, name, special_features, errp);
>      }
> -    return true;
> +    return false;
>  }
>
> -bool visit_deprecated(Visitor *v, const char *name)
> +bool visit_policy_skip(Visitor *v, const char *name,
> +                       unsigned special_features)
>  {
> -    trace_visit_deprecated(v, name);
> -    if (v->deprecated) {
> -        return v->deprecated(v, name);
> +    trace_visit_policy_skip(v, name);
> +    if (v->policy_skip) {
> +        return v->policy_skip(v, name, special_features);
>      }
> -    return true;
> +    return false;
>  }
>
>  void visit_set_policy(Visitor *v, CompatPolicy *policy)
> diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
> index 71b24a4429..fda485614b 100644
> --- a/qapi/qobject-input-visitor.c
> +++ b/qapi/qobject-input-visitor.c
> @@ -662,16 +662,21 @@ static void qobject_input_optional(Visitor *v, const
> char *name, bool *present)
>      *present = true;
>  }
>
> -static bool qobject_input_deprecated_accept(Visitor *v, const char *name,
> -                                            Error **errp)
> +static bool qobject_input_policy_reject(Visitor *v, const char *name,
> +                                        unsigned special_features,
> +                                        Error **errp)
>  {
> +    if (!(special_features && 1u << QAPI_DEPRECATED)) {
> +        return false;
> +    }
> +
>      switch (v->compat_policy.deprecated_input) {
>      case COMPAT_POLICY_INPUT_ACCEPT:
> -        return true;
> +        return false;
>      case COMPAT_POLICY_INPUT_REJECT:
>          error_setg(errp, "Deprecated parameter '%s' disabled
by policy",
>                     name);
> -        return false;
> +        return true;
>      case COMPAT_POLICY_INPUT_CRASH:
>      default:
>          abort();
> @@ -712,7 +717,7 @@ static QObjectInputVisitor
> *qobject_input_visitor_base_new(QObject *obj)
>      v->visitor.end_list = qobject_input_end_list;
>      v->visitor.start_alternate = qobject_input_start_alternate;
>      v->visitor.optional = qobject_input_optional;
> -    v->visitor.deprecated_accept = qobject_input_deprecated_accept;
> +    v->visitor.policy_reject = qobject_input_policy_reject;
>      v->visitor.free = qobject_input_free;
>
>      v->root = qobject_ref(obj);
> diff --git a/qapi/qobject-output-visitor.c b/qapi/qobject-output-visitor.c
> index 9b7f510036..b5c6564cbb 100644
> --- a/qapi/qobject-output-visitor.c
> +++ b/qapi/qobject-output-visitor.c
> @@ -13,6 +13,7 @@
>   */
>
>  #include "qemu/osdep.h"
> +#include "qapi/compat-policy.h"
>  #include "qapi/qobject-output-visitor.h"
>  #include "qapi/visitor-impl.h"
>  #include "qemu/queue.h"
> @@ -208,9 +209,11 @@ static bool qobject_output_type_null(Visitor *v,
> const char *name,
>      return true;
>  }
>
> -static bool qobject_output_deprecated(Visitor *v, const char *name)
> +static bool qobject_output_policy_skip(Visitor *v, const char *name,
> +                                       unsigned special_features)
>  {
> -    return v->compat_policy.deprecated_output !>
COMPAT_POLICY_OUTPUT_HIDE;
> +    return !(special_features && 1u << QAPI_DEPRECATED)
> +        || v->compat_policy.deprecated_output =>
COMPAT_POLICY_OUTPUT_HIDE;
>  }
>
>  /* Finish building, and return the root object.
> @@ -262,7 +265,7 @@ Visitor *qobject_output_visitor_new(QObject **result)
>      v->visitor.type_number = qobject_output_type_number;
>      v->visitor.type_any = qobject_output_type_any;
>      v->visitor.type_null = qobject_output_type_null;
> -    v->visitor.deprecated = qobject_output_deprecated;
> +    v->visitor.policy_skip = qobject_output_policy_skip;
>      v->visitor.complete = qobject_output_complete;
>      v->visitor.free = qobject_output_free;
>
> diff --git a/qapi/trace-events b/qapi/trace-events
> index cccafc07e5..ab108c4f0e 100644
> --- a/qapi/trace-events
> +++ b/qapi/trace-events
> @@ -17,8 +17,8 @@ visit_start_alternate(void *v, const char *name, void
> *obj, size_t size) "v=%p n
>  visit_end_alternate(void *v, void *obj) "v=%p obj=%p"
>
>  visit_optional(void *v, const char *name, bool *present) "v=%p
name=%s
> present=%p"
> -visit_deprecated_accept(void *v, const char *name) "v=%p
name=%s"
> -visit_deprecated(void *v, const char *name) "v=%p name=%s"
> +visit_policy_reject(void *v, const char *name) "v=%p name=%s"
> +visit_policy_skip(void *v, const char *name) "v=%p name=%s"
>
>  visit_type_enum(void *v, const char *name, int *obj) "v=%p name=%s
obj=%p"
>  visit_type_int(void *v, const char *name, int64_t *obj) "v=%p name=%s
> obj=%p"
> diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
> index 9d9196a143..e13bbe4292 100644
> --- a/scripts/qapi/visit.py
> +++ b/scripts/qapi/visit.py
> @@ -21,7 +21,7 @@
>      indent,
>      mcgen,
>  )
> -from .gen import QAPISchemaModularCVisitor, ifcontext
> +from .gen import QAPISchemaModularCVisitor, gen_special_features,
> ifcontext
>  from .schema import (
>      QAPISchema,
>      QAPISchemaEnumMember,
> @@ -76,7 +76,6 @@ def gen_visit_object_members(name: str,
>                       c_type=base.c_name())
>
>      for memb in members:
> -        deprecated = 'deprecated' in [f.name for f in
memb.features]
>          ret += memb.ifcond.gen_if()
>          if memb.optional:
>              ret += mcgen('''
> @@ -84,14 +83,15 @@ def gen_visit_object_members(name: str,
>  ''',
>                           name=memb.name, c_name=c_name(memb.name))
>              indent.increase()
> -        if deprecated:
> +        special_features = gen_special_features(memb.features)
> +        if special_features != '0':
>
Would it be possible for gen_special_features to return something false-y
instead of '0'? Do we actually *use* the '0' return anywhere
other than to
test it to see if we should include additional code?

If you actually use the '0' anywhere: Go ahead and treat this as an ack.
If
you don't, can we clean this up?
(Sorry, I find the mcgen stuff hard to read in patch form and I am trying
to give you a quick review instead of NO review.)

--js

>              ret += mcgen('''
> -    if (!visit_deprecated_accept(v, "%(name)s", errp)) {
> +    if (visit_policy_reject(v, "%(name)s", %(special_features)s,
errp)) {
>          return false;
>      }
> -    if (visit_deprecated(v, "%(name)s")) {
> +    if (!visit_policy_skip(v, "%(name)s", %(special_features)s))
{
>  ''',
> -                         name=memb.name)
> +                         name=memb.name,
> special_features=special_features)
>              indent.increase()
>          ret += mcgen('''
>      if (!visit_type_%(c_type)s(v, "%(name)s",
&obj->%(c_name)s, errp)) {
> @@ -100,7 +100,7 @@ def gen_visit_object_members(name: str,
>  ''',
>                       c_type=memb.type.c_name(), name=memb.name,
>                       c_name=c_name(memb.name))
> -        if deprecated:
> +        if special_features != '0':
>              indent.decrease()
>              ret += mcgen('''
>      }
> --
> 2.31.1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://listman.redhat.com/archives/libguestfs/attachments/20211025/ba08b823/attachment.htm>

On 10/25/21 07:25, Markus Armbruster wrote:
> The generated visitor functions call visit_deprecated_accept() and
> visit_deprecated() when visiting a struct member with special feature
> flag 'deprecated'.  This makes the feature flag visible to the
actual
> visitors.  I want to make feature flag 'unstable' visible there as
> well, so I can add policy for it.
> 
> To let me make it visible, replace these functions by
> visit_policy_reject() and visit_policy_skip(), which take the member's
> special features as an argument.  Note that the new functions have the
> opposite sense, i.e. the return value flips.
> 
> Signed-off-by: Markus Armbruster <armbru at redhat.com>
> ---
>  include/qapi/visitor-impl.h   |  6 ++++--
>  include/qapi/visitor.h        | 17 +++++++++++++----
>  qapi/qapi-forward-visitor.c   | 16 +++++++++-------
>  qapi/qapi-visit-core.c        | 22 ++++++++++++----------
>  qapi/qobject-input-visitor.c  | 15 ++++++++++-----
>  qapi/qobject-output-visitor.c |  9 ++++++---
>  qapi/trace-events             |  4 ++--
>  scripts/qapi/visit.py         | 14 +++++++-------
>  8 files changed, 63 insertions(+), 40 deletions(-)
> diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
> index 71b24a4429..fda485614b 100644
> --- a/qapi/qobject-input-visitor.c
> +++ b/qapi/qobject-input-visitor.c
> @@ -662,16 +662,21 @@ static void qobject_input_optional(Visitor *v, const
char *name, bool *present)
>      *present = true;
>  }
>  
> -static bool qobject_input_deprecated_accept(Visitor *v, const char *name,
> -                                            Error **errp)
> +static bool qobject_input_policy_reject(Visitor *v, const char *name,
> +                                        unsigned special_features,
> +                                        Error **errp)
>  {
> +    if (!(special_features && 1u << QAPI_DEPRECATED)) {
Unreachable =) Proof than extract() is safer :P
> +        return false;
> +    }