| Message ID | 20211028102520.747396-9-armbru@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Configurable policy for handling unstable interfaces | expand |
Markus Armbruster <armbru@redhat.com> writes: > The code to check policy for handling deprecated input is triplicated. > Factor it out into compat_policy_input_ok() before I mess with it in > the next commit. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > include/qapi/compat-policy.h | 7 +++++ > qapi/qapi-visit-core.c | 18 +++++-------- > qapi/qmp-dispatch.c | 51 +++++++++++++++++++++++++++--------- > qapi/qobject-input-visitor.c | 19 +++----------- > 4 files changed, 55 insertions(+), 40 deletions(-) > > diff --git a/include/qapi/compat-policy.h b/include/qapi/compat-policy.h > index 1083f95122..8b7b25c0b5 100644 > --- a/include/qapi/compat-policy.h > +++ b/include/qapi/compat-policy.h > @@ -13,10 +13,17 @@ > #ifndef QAPI_COMPAT_POLICY_H > #define QAPI_COMPAT_POLICY_H > > +#include "qapi/error.h" > #include "qapi/qapi-types-compat.h" > > extern CompatPolicy compat_policy; > > +bool compat_policy_input_ok(unsigned special_features, > + const CompatPolicy *policy, > + ErrorClass error_class, > + const char *kind, const char *name, > + Error **errp); > + > /* > * Create a QObject input visitor for @obj for use with QMP > * > diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c > index 34c59286b2..6c13510a2b 100644 > --- a/qapi/qapi-visit-core.c > +++ b/qapi/qapi-visit-core.c > @@ -13,6 +13,7 @@ > */ > > #include "qemu/osdep.h" > +#include "qapi/compat-policy.h" > #include "qapi/error.h" > #include "qapi/qmp/qerror.h" > #include "qapi/visitor.h" > @@ -409,18 +410,11 @@ static bool input_type_enum(Visitor *v, const char *name, int *obj, > } > > if (lookup->special_features > - && (lookup->special_features[value] & QAPI_DEPRECATED)) { > - switch (v->compat_policy.deprecated_input) { > - case COMPAT_POLICY_INPUT_ACCEPT: > - break; > - case COMPAT_POLICY_INPUT_REJECT: > - error_setg(errp, "Deprecated value '%s' disabled by policy", > - enum_str); > - return false; > - case COMPAT_POLICY_INPUT_CRASH: > - default: > - abort(); > - } > + && !compat_policy_input_ok(lookup->special_features[value], > + &v->compat_policy, > + ERROR_CLASS_GENERIC_ERROR, > + "value", enum_str, errp)) { > + return false; > } > > *obj = value; > diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c > index 8cca18c891..e29ade134c 100644 > --- a/qapi/qmp-dispatch.c > +++ b/qapi/qmp-dispatch.c > @@ -28,6 +28,40 @@ > > CompatPolicy compat_policy; > > +static bool compat_policy_input_ok1(const char *adjective, > + CompatPolicyInput policy, > + ErrorClass error_class, > + const char *kind, const char *name, > + Error **errp) > +{ > + switch (policy) { > + case COMPAT_POLICY_INPUT_ACCEPT: > + return true; > + case COMPAT_POLICY_INPUT_REJECT: > + error_set(errp, error_class, "%s %s %s disabled by policy", > + adjective, kind, name); > + return false; > + case COMPAT_POLICY_INPUT_CRASH: > + default: > + abort(); > + } > +} > + > +bool compat_policy_input_ok(unsigned special_features, > + const CompatPolicy *policy, > + ErrorClass error_class, > + const char *kind, const char *name, > + Error **errp) > +{ > + if ((special_features & 1u << QAPI_DEPRECATED) > + && !compat_policy_input_ok1("Deprecated", > + policy->deprecated_input, > + error_class, kind, name, errp)) { > + return false; > + } > + return true; > +} > + > Visitor *qobject_input_visitor_new_qmp(QObject *obj) > { > Visitor *v = qobject_input_visitor_new(obj); I'm moving the new functions along with @compat_policy to qapi-util.c, so the QAPI visitors survive linking without qmp-dispatch.o. > @@ -176,19 +210,10 @@ QDict *qmp_dispatch(const QmpCommandList *cmds, QObject *request, > "The command %s has not been found", command); > goto out; > } > - if (cmd->special_features & 1u << QAPI_DEPRECATED) { > - switch (compat_policy.deprecated_input) { > - case COMPAT_POLICY_INPUT_ACCEPT: > - break; > - case COMPAT_POLICY_INPUT_REJECT: > - error_set(&err, ERROR_CLASS_COMMAND_NOT_FOUND, > - "Deprecated command %s disabled by policy", > - command); > - goto out; > - case COMPAT_POLICY_INPUT_CRASH: > - default: > - abort(); > - } > + if (!compat_policy_input_ok(cmd->special_features, &compat_policy, > + ERROR_CLASS_COMMAND_NOT_FOUND, > + "command", command, &err)) { > + goto out; > } > if (!cmd->enabled) { > error_set(&err, ERROR_CLASS_COMMAND_NOT_FOUND, > diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c > index c120dbdd92..f0b4c7ca9d 100644 > --- a/qapi/qobject-input-visitor.c > +++ b/qapi/qobject-input-visitor.c > @@ -14,6 +14,7 @@ > > #include "qemu/osdep.h" > #include <math.h> > +#include "qapi/compat-policy.h" > #include "qapi/error.h" > #include "qapi/qobject-input-visitor.h" > #include "qapi/visitor-impl.h" > @@ -666,21 +667,9 @@ 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 false; > - case COMPAT_POLICY_INPUT_REJECT: > - error_setg(errp, "Deprecated parameter '%s' disabled by policy", > - name); > - return true; > - case COMPAT_POLICY_INPUT_CRASH: > - default: > - abort(); > - } > + return !compat_policy_input_ok(special_features, &v->compat_policy, > + ERROR_CLASS_GENERIC_ERROR, > + "parameter", name, errp); > } > > static void qobject_input_free(Visitor *v)
On 10/29/21 18:55, Markus Armbruster wrote: > Markus Armbruster <armbru@redhat.com> writes: > >> The code to check policy for handling deprecated input is triplicated. >> Factor it out into compat_policy_input_ok() before I mess with it in >> the next commit. >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> --- >> include/qapi/compat-policy.h | 7 +++++ >> qapi/qapi-visit-core.c | 18 +++++-------- >> qapi/qmp-dispatch.c | 51 +++++++++++++++++++++++++++--------- >> qapi/qobject-input-visitor.c | 19 +++----------- >> 4 files changed, 55 insertions(+), 40 deletions(-) >> +bool compat_policy_input_ok(unsigned special_features, >> + const CompatPolicy *policy, >> + ErrorClass error_class, >> + const char *kind, const char *name, >> + Error **errp) >> +{ >> + if ((special_features & 1u << QAPI_DEPRECATED) >> + && !compat_policy_input_ok1("Deprecated", >> + policy->deprecated_input, >> + error_class, kind, name, errp)) { >> + return false; >> + } >> + return true; >> +} >> + >> Visitor *qobject_input_visitor_new_qmp(QObject *obj) >> { >> Visitor *v = qobject_input_visitor_new(obj); > > I'm moving the new functions along with @compat_policy to qapi-util.c, > so the QAPI visitors survive linking without qmp-dispatch.o. OK. I am waiting your series to get in before respining my QAPI sysemu/user series which will re-scramble meson.build here. Probably too late for this release. Not super important, it just saves energy avoiding generating / building unused files.
diff --git a/include/qapi/compat-policy.h b/include/qapi/compat-policy.h index 1083f95122..8b7b25c0b5 100644 --- a/include/qapi/compat-policy.h +++ b/include/qapi/compat-policy.h @@ -13,10 +13,17 @@ #ifndef QAPI_COMPAT_POLICY_H #define QAPI_COMPAT_POLICY_H +#include "qapi/error.h" #include "qapi/qapi-types-compat.h" extern CompatPolicy compat_policy; +bool compat_policy_input_ok(unsigned special_features, + const CompatPolicy *policy, + ErrorClass error_class, + const char *kind, const char *name, + Error **errp); + /* * Create a QObject input visitor for @obj for use with QMP * diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c index 34c59286b2..6c13510a2b 100644 --- a/qapi/qapi-visit-core.c +++ b/qapi/qapi-visit-core.c @@ -13,6 +13,7 @@ */ #include "qemu/osdep.h" +#include "qapi/compat-policy.h" #include "qapi/error.h" #include "qapi/qmp/qerror.h" #include "qapi/visitor.h" @@ -409,18 +410,11 @@ static bool input_type_enum(Visitor *v, const char *name, int *obj, } if (lookup->special_features - && (lookup->special_features[value] & QAPI_DEPRECATED)) { - switch (v->compat_policy.deprecated_input) { - case COMPAT_POLICY_INPUT_ACCEPT: - break; - case COMPAT_POLICY_INPUT_REJECT: - error_setg(errp, "Deprecated value '%s' disabled by policy", - enum_str); - return false; - case COMPAT_POLICY_INPUT_CRASH: - default: - abort(); - } + && !compat_policy_input_ok(lookup->special_features[value], + &v->compat_policy, + ERROR_CLASS_GENERIC_ERROR, + "value", enum_str, errp)) { + return false; } *obj = value; diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c index 8cca18c891..e29ade134c 100644 --- a/qapi/qmp-dispatch.c +++ b/qapi/qmp-dispatch.c @@ -28,6 +28,40 @@ CompatPolicy compat_policy; +static bool compat_policy_input_ok1(const char *adjective, + CompatPolicyInput policy, + ErrorClass error_class, + const char *kind, const char *name, + Error **errp) +{ + switch (policy) { + case COMPAT_POLICY_INPUT_ACCEPT: + return true; + case COMPAT_POLICY_INPUT_REJECT: + error_set(errp, error_class, "%s %s %s disabled by policy", + adjective, kind, name); + return false; + case COMPAT_POLICY_INPUT_CRASH: + default: + abort(); + } +} + +bool compat_policy_input_ok(unsigned special_features, + const CompatPolicy *policy, + ErrorClass error_class, + const char *kind, const char *name, + Error **errp) +{ + if ((special_features & 1u << QAPI_DEPRECATED) + && !compat_policy_input_ok1("Deprecated", + policy->deprecated_input, + error_class, kind, name, errp)) { + return false; + } + return true; +} + Visitor *qobject_input_visitor_new_qmp(QObject *obj) { Visitor *v = qobject_input_visitor_new(obj); @@ -176,19 +210,10 @@ QDict *qmp_dispatch(const QmpCommandList *cmds, QObject *request, "The command %s has not been found", command); goto out; } - if (cmd->special_features & 1u << QAPI_DEPRECATED) { - switch (compat_policy.deprecated_input) { - case COMPAT_POLICY_INPUT_ACCEPT: - break; - case COMPAT_POLICY_INPUT_REJECT: - error_set(&err, ERROR_CLASS_COMMAND_NOT_FOUND, - "Deprecated command %s disabled by policy", - command); - goto out; - case COMPAT_POLICY_INPUT_CRASH: - default: - abort(); - } + if (!compat_policy_input_ok(cmd->special_features, &compat_policy, + ERROR_CLASS_COMMAND_NOT_FOUND, + "command", command, &err)) { + goto out; } if (!cmd->enabled) { error_set(&err, ERROR_CLASS_COMMAND_NOT_FOUND, diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c index c120dbdd92..f0b4c7ca9d 100644 --- a/qapi/qobject-input-visitor.c +++ b/qapi/qobject-input-visitor.c @@ -14,6 +14,7 @@ #include "qemu/osdep.h" #include <math.h> +#include "qapi/compat-policy.h" #include "qapi/error.h" #include "qapi/qobject-input-visitor.h" #include "qapi/visitor-impl.h" @@ -666,21 +667,9 @@ 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 false; - case COMPAT_POLICY_INPUT_REJECT: - error_setg(errp, "Deprecated parameter '%s' disabled by policy", - name); - return true; - case COMPAT_POLICY_INPUT_CRASH: - default: - abort(); - } + return !compat_policy_input_ok(special_features, &v->compat_policy, + ERROR_CLASS_GENERIC_ERROR, + "parameter", name, errp); } static void qobject_input_free(Visitor *v)