| Message ID | 20211025052532.3859634-6-armbru@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Configurable policy for handling unstable interfaces | expand |
On Mon, Oct 25, 2021 at 1:25 AM Markus Armbruster <armbru@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@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 > >
John Snow <jsnow@redhat.com> writes: > On Mon, Oct 25, 2021 at 1:25 AM Markus Armbruster <armbru@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@redhat.com> [...] >> 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? gen_special_features() returns a string holding C code for a special features bitset. The empty bitset is 0. The "natural" use is interpolating this value into C code. Two instances are visible below. The use right here is for testing "have we got special features", so we can skip generating code that is of no use when we don't have any. It's admittedly slightly unclean. > (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.) Any kind of review is appreciated :) > --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) These are the "natural" uses I mentioned. If gen_special_features() returned something other than '0', we'd have to replace it by '0' here. >> 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 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@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; > + }
Philippe Mathieu-Daudé <philmd@redhat.com> writes: > 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@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 Good eyes, thank you! I actually like extract & desposit macros when the width is greater than one. Then, the longhand C code is illegible anyway, and having to remember what the macros mean is no worse. For width 1 it feels like a wash. Universal use of the macros could build familiarity and thus tip the balance. I count more than a thousand instances of '& (1 <<'. I wasn't even aware the macros existed in QEMU[*]. > >> + return false; >> + } [*] I may well have seen them before, but my memory is limited and lossy.
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(''' }
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@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(-)