Re: [PATCH v4 29/34] qapi: Implement deprecated-output=hide for QMP events

[Eric Blake]

On 3/17/20 6:54 AM, Markus Armbruster wrote:
> This policy suppresses deprecated bits in output, and thus permits
> "testing the future".  Implement it for QMP events: suppress
> deprecated ones.
>
> No QMP event is deprecated right now.
>
> Signed-off-by: Markus Armbruster <address@hidden>
> ---

> @@ -140,6 +141,23 @@ static void test_event_d(TestEventData *data,
>       qobject_unref(data->expect);
>   }
>   
> +static void test_event_deprecated(TestEventData *data, const void *unused)
> +{
> +    data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST-EVENT-FEATURES1' 
> }");
> +
> +    memset(&compat_policy, 0, sizeof(compat_policy));
> +
> +    qapi_event_send_test_event_features1();
> +    g_assert(data->emitted);
> +
> +    compat_policy.deprecated_output = COMPAT_POLICY_OUTPUT_HIDE;

Umm, did you forget to set compat_policy.has_deprecated_output = true?

(proof that we really want QAPI defaults to be working, to get rid of 
pointless has_* members...)

> +    data->emitted = false;
> +    qapi_event_send_test_event_features1();
> +    g_assert(!data->emitted);
> +
> +    qobject_unref(data->expect);
> +}
> +

> +++ b/scripts/qapi/events.py
> @@ -61,7 +61,8 @@ def gen_param_var(typ):
>       return ret
>   
>   
> -def gen_event_send(name, arg_type, boxed, event_enum_name, event_emit):
> +def gen_event_send(name, arg_type, features, boxed,
> +                   event_enum_name, event_emit):
>       # FIXME: Our declaration of local variables (and of 'errp' in the
>       # parameter list) can collide with exploded members of the event's
>       # data type passed in as parameters.  If this collision ever hits in
> @@ -86,6 +87,14 @@ def gen_event_send(name, arg_type, boxed, event_enum_name, 
> event_emit):
>           if not boxed:
>               ret += gen_param_var(arg_type)
>   
> +    if 'deprecated' in [f.name for f in features]:
> +        ret += mcgen('''
> +
> +    if (compat_policy.deprecated_output == COMPAT_POLICY_OUTPUT_HIDE) {
> +        return;

Here, you took the shortcut that if we always 0-initialize, 
deprecated_output will never be true except when has_deprecated_output 
is also true.  But the test above violated that rule of thumb.

Otherwise, this patch makes sense.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org