1. Patchew
  2. Libvirt
  3. [PATCH 0/9] Configurable policy for handling unstable interfaces
  4. View patch

[PATCH 4/9] qapi: Tools for sets of special feature flags in generated code

Markus Armbruster posted 9 patches 4 years, 3 months ago
There is a newer version of this series
[PATCH 4/9] qapi: Tools for sets of special feature flags in generated code
Posted by Markus Armbruster 4 years, 3 months ago 
New enum QapiSpecialFeature enumerates the special feature flags.

New helper gen_special_features() returns code to represent a
collection of special feature flags as a bitset.

The next few commits will put them to use.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/util.h    |  4 ++++
 scripts/qapi/gen.py    | 13 +++++++++++++
 scripts/qapi/schema.py |  3 +++
 3 files changed, 20 insertions(+)

diff --git a/include/qapi/util.h b/include/qapi/util.h
index 257c600f99..7a8d5c7d72 100644
--- a/include/qapi/util.h
+++ b/include/qapi/util.h
@@ -11,6 +11,10 @@
 #ifndef QAPI_UTIL_H
 #define QAPI_UTIL_H
 
+typedef enum {
+    QAPI_DEPRECATED,
+} QapiSpecialFeature;
+
 /* QEnumLookup flags */
 #define QAPI_ENUM_DEPRECATED 1
 
diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index 2ec1e7b3b6..9d07b88cf6 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -29,6 +29,7 @@
     mcgen,
 )
 from .schema import (
+    QAPISchemaFeature,
     QAPISchemaIfCond,
     QAPISchemaModule,
     QAPISchemaObjectType,
@@ -37,6 +38,18 @@
 from .source import QAPISourceInfo
 
 
+def gen_special_features(features: QAPISchemaFeature):
+    ret = ''
+    sep = ''
+
+    for feat in features:
+        if feat.is_special():
+            ret += ('%s1u << QAPI_%s' % (sep, feat.name.upper()))
+            sep = ' | '
+
+    return ret or '0'
+
+
 class QAPIGen:
     def __init__(self, fname: str):
         self.fname = fname
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 6d5f46509a..55f82d7389 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -725,6 +725,9 @@ def connect_doc(self, doc):
 class QAPISchemaFeature(QAPISchemaMember):
     role = 'feature'
 
+    def is_special(self):
+        return self.name in ('deprecated')
+
 
 class QAPISchemaObjectTypeMember(QAPISchemaMember):
     def __init__(self, name, info, typ, optional, ifcond=None, features=None):
-- 
2.31.1
Re: [PATCH 4/9] qapi: Tools for sets of special feature flags in generated code
Posted by John Snow 4 years, 3 months ago 
On Mon, Oct 25, 2021 at 1:25 AM Markus Armbruster <armbru@redhat.com> wrote:

> New enum QapiSpecialFeature enumerates the special feature flags.
>
> New helper gen_special_features() returns code to represent a
> collection of special feature flags as a bitset.
>
> The next few commits will put them to use.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  include/qapi/util.h    |  4 ++++
>  scripts/qapi/gen.py    | 13 +++++++++++++
>  scripts/qapi/schema.py |  3 +++
>  3 files changed, 20 insertions(+)
>
> diff --git a/include/qapi/util.h b/include/qapi/util.h
> index 257c600f99..7a8d5c7d72 100644
> --- a/include/qapi/util.h
> +++ b/include/qapi/util.h
> @@ -11,6 +11,10 @@
>  #ifndef QAPI_UTIL_H
>  #define QAPI_UTIL_H
>
> +typedef enum {
> +    QAPI_DEPRECATED,
> +} QapiSpecialFeature;
> +
>  /* QEnumLookup flags */
>  #define QAPI_ENUM_DEPRECATED 1
>
> diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
> index 2ec1e7b3b6..9d07b88cf6 100644
> --- a/scripts/qapi/gen.py
> +++ b/scripts/qapi/gen.py
> @@ -29,6 +29,7 @@
>      mcgen,
>  )
>  from .schema import (
> +    QAPISchemaFeature,
>      QAPISchemaIfCond,
>      QAPISchemaModule,
>      QAPISchemaObjectType,
> @@ -37,6 +38,18 @@
>  from .source import QAPISourceInfo
>
>
> +def gen_special_features(features: QAPISchemaFeature):
> +    ret = ''
> +    sep = ''
> +
> +    for feat in features:
> +        if feat.is_special():
> +            ret += ('%s1u << QAPI_%s' % (sep, feat.name.upper()))
>

Building the constant name here "feels" fragile, but I'll trust that the
test suite and/or the compiler will catch us if we accidentally goof up
this mapping. In the interest of simplicity, then, "sure, why not."


> +            sep = ' | '
> +
>
+    return ret or '0'
> +
>

Subjectively more pythonic:

special_features = [f"1u << QAPI_{feat.name.upper()}" for feat in features
if feat.is_special()]
ret = ' | '.join(special_features)
return ret or '0'

A bit more dense, but more functional. Up to you, but I find join() easier
to read and reason about for the presence of separators.


> +
>  class QAPIGen:
>      def __init__(self, fname: str):
>          self.fname = fname
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index 6d5f46509a..55f82d7389 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -725,6 +725,9 @@ def connect_doc(self, doc):
>  class QAPISchemaFeature(QAPISchemaMember):
>      role = 'feature'
>
> +    def is_special(self):
> +        return self.name in ('deprecated')
> +
>

alrighty.

(Briefly wondered: is it worth naming special features as a property of the
class, but with only two names, it's probably fine enough to leave it
embedded in the method logic. Only a style thing and doesn't have any
actual impact that I can imagine, so ... nevermind.)


>
>  class QAPISchemaObjectTypeMember(QAPISchemaMember):
>      def __init__(self, name, info, typ, optional, ifcond=None,
> features=None):
> --
> 2.31.1
>
>
Well, either way:

Reviewed-by: John Snow <jsnow@redhat.com>
Re: [PATCH 4/9] qapi: Tools for sets of special feature flags in generated code
Posted by Markus Armbruster 4 years, 3 months ago 
John Snow <jsnow@redhat.com> writes:

> On Mon, Oct 25, 2021 at 1:25 AM Markus Armbruster <armbru@redhat.com> wrote:
>
>> New enum QapiSpecialFeature enumerates the special feature flags.
>>
>> New helper gen_special_features() returns code to represent a
>> collection of special feature flags as a bitset.
>>
>> The next few commits will put them to use.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  include/qapi/util.h    |  4 ++++
>>  scripts/qapi/gen.py    | 13 +++++++++++++
>>  scripts/qapi/schema.py |  3 +++
>>  3 files changed, 20 insertions(+)
>>
>> diff --git a/include/qapi/util.h b/include/qapi/util.h
>> index 257c600f99..7a8d5c7d72 100644
>> --- a/include/qapi/util.h
>> +++ b/include/qapi/util.h
>> @@ -11,6 +11,10 @@
>>  #ifndef QAPI_UTIL_H
>>  #define QAPI_UTIL_H
>>
>> +typedef enum {
>> +    QAPI_DEPRECATED,
>> +} QapiSpecialFeature;
>> +
>>  /* QEnumLookup flags */
>>  #define QAPI_ENUM_DEPRECATED 1
>>
>> diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
>> index 2ec1e7b3b6..9d07b88cf6 100644
>> --- a/scripts/qapi/gen.py
>> +++ b/scripts/qapi/gen.py
>> @@ -29,6 +29,7 @@
>>      mcgen,
>>  )
>>  from .schema import (
>> +    QAPISchemaFeature,
>>      QAPISchemaIfCond,
>>      QAPISchemaModule,
>>      QAPISchemaObjectType,
>> @@ -37,6 +38,18 @@
>>  from .source import QAPISourceInfo
>>
>>
>> +def gen_special_features(features: QAPISchemaFeature):
>> +    ret = ''
>> +    sep = ''
>> +
>> +    for feat in features:
>> +        if feat.is_special():
>> +            ret += ('%s1u << QAPI_%s' % (sep, feat.name.upper()))
>>
>
> Building the constant name here "feels" fragile, but I'll trust that the
> test suite and/or the compiler will catch us if we accidentally goof up
> this mapping. In the interest of simplicity, then, "sure, why not."

It relies on .is_special() remaining consistent with enum
QapiSpecialFeature.  The C compiler should catch any screwups.

>
>> +            sep = ' | '
>> +
>>
> +    return ret or '0'
>> +
>>
>
> Subjectively more pythonic:
>
> special_features = [f"1u << QAPI_{feat.name.upper()}" for feat in features
> if feat.is_special()]
> ret = ' | '.join(special_features)
> return ret or '0'
>
> A bit more dense, but more functional. Up to you, but I find join() easier
> to read and reason about for the presence of separators.

Sold!

>> +
>>  class QAPIGen:
>>      def __init__(self, fname: str):
>>          self.fname = fname
>> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
>> index 6d5f46509a..55f82d7389 100644
>> --- a/scripts/qapi/schema.py
>> +++ b/scripts/qapi/schema.py
>> @@ -725,6 +725,9 @@ def connect_doc(self, doc):
>>  class QAPISchemaFeature(QAPISchemaMember):
>>      role = 'feature'
>>
>> +    def is_special(self):
>> +        return self.name in ('deprecated')
>> +
>>
>
> alrighty.
>
> (Briefly wondered: is it worth naming special features as a property of the
> class, but with only two names, it's probably fine enough to leave it
> embedded in the method logic. Only a style thing and doesn't have any
> actual impact that I can imagine, so ... nevermind.)

Let's keep it simple.

>>  class QAPISchemaObjectTypeMember(QAPISchemaMember):
>>      def __init__(self, name, info, typ, optional, ifcond=None,
>> features=None):
>> --
>> 2.31.1
>>
>>
> Well, either way:
>
> Reviewed-by: John Snow <jsnow@redhat.com>

Thanks!

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.