subject:"\[PATCH 6\/9\] qapi\: Generalize command policy checking"

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

2021-10-26 Thread Markus Armbruster

Philippe Mathieu-Daudé  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 
>> ---
>>  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é 

Thanks!

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

2021-10-25 Thread John Snow

On Mon, Oct 25, 2021 at 1:25 AM 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 
> ---
>  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);
>  const QmpCommand *qmp_find_command(const QmpCommandList *cmds,
> const char *name);
>  void qmp_disable_command(QmpCommandList *cmds, const char *name,
> diff --git a/monitor/misc.c b/monitor/misc.c
> index 3556b177f6..c2d227a07c 100644
> --- a/monitor/misc.c
> +++ b/monitor/misc.c
> @@ -230,11 +230,13 @@ static void monitor_init_qmp_commands(void)
>
>  qmp_init_marshal(_commands);
>
> -qmp_register_command(_commands, "device_add", qmp_device_add, 0);
> +qmp_register_command(_commands, "device_add",
> + qmp_device_add, 0, 0);
>
>  QTAILQ_INIT(_cap_negotiation_commands);
>  qmp_register_command(_cap_negotiation_commands,
> "qmp_capabilities",
> - qmp_marshal_qmp_capabilities,
> QCO_ALLOW_PRECONFIG);
> + qmp_marshal_qmp_capabilities,
> + QCO_ALLOW_PRECONFIG, 0);
>  }
>
>  /* Set the current CPU defined by the user. Callers must hold BQL. */
> diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
> index 7e943a0af5..8cca18c891 100644
> --- a/qapi/qmp-dispatch.c
> +++ b/qapi/qmp-dispatch.c
> @@ -176,7 +176,7 @@ QDict *qmp_dispatch(const QmpCommandList *cmds,
> QObject *request,
>"The command %s has not been found", command);
>  goto out;
>  }
> -if (cmd->options & QCO_DEPRECATED) {
> +if (cmd->special_features & 1u << QAPI_DEPRECATED) {
>  switch (compat_policy.deprecated_input) {
>  case COMPAT_POLICY_INPUT_ACCEPT:
>  break;
> diff --git a/qapi/qmp-registry.c b/qapi/qmp-registry.c
> index f78c064aae..485bc5e6fc 100644
> --- a/qapi/qmp-registry.c
> +++ b/qapi/qmp-registry.c
> @@ -16,7 +16,8 @@
>  #include "qapi/qmp/dispatch.h"
>
>  void qmp_register_command(QmpCommandList *cmds, const char *name,
> -  QmpCommandFunc *fn, QmpCommandOptions options)
> +  QmpCommandFunc *fn, QmpCommandOptions options,
> +  unsigned special_features)
>  {
>  QmpCommand *cmd = g_malloc0(sizeof(*cmd));
>
> @@ -27,6 +28,7 @@ void qmp_register_command(QmpCommandList *cmds, const
> char *name,
>  cmd->fn = fn;
>  cmd->enabled = true;
>  cmd->options = options;
> +cmd->special_features = special_features;
>  QTAILQ_INSERT_TAIL(cmds, cmd, node);
>  }
>
> diff --git a/storage-daemon/qemu-storage-daemon.c
> b/storage-daemon/qemu-storage-daemon.c
> index 10a1a33761..52cf17e8ac 100644
> --- a/storage-daemon/qemu-storage-daemon.c
> +++ b/storage-daemon/qemu-storage-daemon.c
> @@ -146,7 +146,8 @@ static void init_qmp_commands(void)
>
>  QTAILQ_INIT(_cap_negotiation_commands);
>  qmp_register_command(_cap_negotiation_commands,
> "qmp_capabilities",
> - qmp_marshal_qmp_capabilities,
> QCO_ALLOW_PRECONFIG);
> +

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

2021-10-25 Thread Philippe Mathieu-Daudé

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 
> ---
>  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;

?

Otherwise:
Reviewed-by: Philippe Mathieu-Daudé

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

2021-10-24 Thread Markus Armbruster

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 
---
 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);
 const QmpCommand *qmp_find_command(const QmpCommandList *cmds,
const char *name);
 void qmp_disable_command(QmpCommandList *cmds, const char *name,
diff --git a/monitor/misc.c b/monitor/misc.c
index 3556b177f6..c2d227a07c 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -230,11 +230,13 @@ static void monitor_init_qmp_commands(void)
 
 qmp_init_marshal(_commands);
 
-qmp_register_command(_commands, "device_add", qmp_device_add, 0);
+qmp_register_command(_commands, "device_add",
+ qmp_device_add, 0, 0);
 
 QTAILQ_INIT(_cap_negotiation_commands);
 qmp_register_command(_cap_negotiation_commands, "qmp_capabilities",
- qmp_marshal_qmp_capabilities, QCO_ALLOW_PRECONFIG);
+ qmp_marshal_qmp_capabilities,
+ QCO_ALLOW_PRECONFIG, 0);
 }
 
 /* Set the current CPU defined by the user. Callers must hold BQL. */
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 7e943a0af5..8cca18c891 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -176,7 +176,7 @@ QDict *qmp_dispatch(const QmpCommandList *cmds, QObject 
*request,
   "The command %s has not been found", command);
 goto out;
 }
-if (cmd->options & QCO_DEPRECATED) {
+if (cmd->special_features & 1u << QAPI_DEPRECATED) {
 switch (compat_policy.deprecated_input) {
 case COMPAT_POLICY_INPUT_ACCEPT:
 break;
diff --git a/qapi/qmp-registry.c b/qapi/qmp-registry.c
index f78c064aae..485bc5e6fc 100644
--- a/qapi/qmp-registry.c
+++ b/qapi/qmp-registry.c
@@ -16,7 +16,8 @@
 #include "qapi/qmp/dispatch.h"
 
 void qmp_register_command(QmpCommandList *cmds, const char *name,
-  QmpCommandFunc *fn, QmpCommandOptions options)
+  QmpCommandFunc *fn, QmpCommandOptions options,
+  unsigned special_features)
 {
 QmpCommand *cmd = g_malloc0(sizeof(*cmd));
 
@@ -27,6 +28,7 @@ void qmp_register_command(QmpCommandList *cmds, const char 
*name,
 cmd->fn = fn;
 cmd->enabled = true;
 cmd->options = options;
+cmd->special_features = special_features;
 QTAILQ_INSERT_TAIL(cmds, cmd, node);
 }
 
diff --git a/storage-daemon/qemu-storage-daemon.c 
b/storage-daemon/qemu-storage-daemon.c
index 10a1a33761..52cf17e8ac 100644
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -146,7 +146,8 @@ static void init_qmp_commands(void)
 
 QTAILQ_INIT(_cap_negotiation_commands);
 qmp_register_command(_cap_negotiation_commands, "qmp_capabilities",
- qmp_marshal_qmp_capabilities, QCO_ALLOW_PRECONFIG);
+ qmp_marshal_qmp_capabilities,
+ QCO_ALLOW_PRECONFIG, 0);
 }
 
 static int getopt_set_loc(int argc, char **argv, const char *optstring,
diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index c8a975528f..21001bbd6b 100644

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

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

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

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

4 matches

Site Navigation

Mail list logo

Footer information