Re: [PATCH 6/9] qapi: Generalize command policy checking

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> On 10/25/21 07:25, Markus Armbruster wrote:
>> The code to check command policy can see special feature flag
>> 'deprecated' as command flag QCO_DEPRECATED.  I want to make feature
>> flag 'unstable' visible there as well, so I can add policy for it.
>> 
>> To let me make it visible, add member @special_features (a bitset of
>> QapiSpecialFeature) to QmpCommand, and adjust the generator to pass it
>> through qmp_register_command().  Then replace "QCO_DEPRECATED in
>> @flags" by QAPI_DEPRECATED in @special_features", and drop
>> QCO_DEPRECATED.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  include/qapi/qmp/dispatch.h          | 5 +++--
>>  monitor/misc.c                       | 6 ++++--
>>  qapi/qmp-dispatch.c                  | 2 +-
>>  qapi/qmp-registry.c                  | 4 +++-
>>  storage-daemon/qemu-storage-daemon.c | 3 ++-
>>  scripts/qapi/commands.py             | 9 ++++-----
>>  6 files changed, 17 insertions(+), 12 deletions(-)
>> 
>> diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
>> index 0ce88200b9..1e4240fd0d 100644
>> --- a/include/qapi/qmp/dispatch.h
>> +++ b/include/qapi/qmp/dispatch.h
>> @@ -25,7 +25,6 @@ typedef enum QmpCommandOptions
>>      QCO_ALLOW_OOB             =  (1U << 1),
>>      QCO_ALLOW_PRECONFIG       =  (1U << 2),
>>      QCO_COROUTINE             =  (1U << 3),
>> -    QCO_DEPRECATED            =  (1U << 4),
>>  } QmpCommandOptions;
>>  
>>  typedef struct QmpCommand
>> @@ -34,6 +33,7 @@ typedef struct QmpCommand
>>      /* Runs in coroutine context if QCO_COROUTINE is set */
>>      QmpCommandFunc *fn;
>>      QmpCommandOptions options;
>> +    unsigned special_features;
>>      QTAILQ_ENTRY(QmpCommand) node;
>>      bool enabled;
>>      const char *disable_reason;
>> @@ -42,7 +42,8 @@ typedef struct QmpCommand
>>  typedef QTAILQ_HEAD(QmpCommandList, QmpCommand) QmpCommandList;
>>  
>>  void qmp_register_command(QmpCommandList *cmds, const char *name,
>> -                          QmpCommandFunc *fn, QmpCommandOptions options);
>> +                          QmpCommandFunc *fn, QmpCommandOptions options,
>> +                          unsigned special_features);
>
> What about:
>
>   typedef unsigned QapiFeatureMask;
>
> ?

I think the name @special_features makes the connection to
QapiSpecialFeature clear enough.

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

Thanks!