| Message ID | 20211025052532.3859634-2-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Configurable policy for handling unstable interfaces | expand |
Markus Armbruster <armbru@redhat.com> wrote: > By convention, names starting with "x-" are experimental. The parts > of external interfaces so named may be withdrawn or changed > incompatibly in future releases. > > Drawback: promoting something from experimental to stable involves a > name change. Client code needs to be updated. > > Moreover, the convention is not universally observed: > > * QOM type "input-barrier" has properties "x-origin", "y-origin". > Looks accidental, but it's ABI since 4.2. > > * QOM types "memory-backend-file", "memory-backend-memfd", > "memory-backend-ram", and "memory-backend-epc" have a property > "x-use-canonical-path-for-ramblock-id" that is documented to be > stable despite its name. > > We could document these exceptions, but documentation helps only > humans. We want to recognize "unstable" in code, like "deprecated". > > Replace the convention by a new special feature flag "unstable". It > will be recognized by the QAPI generator, like the existing feature > flag "deprecated", and unlike regular feature flags. > > This commit updates documentation and prepares tests. The next commit > updates the QAPI schema. The remaining patches update the QAPI > generator and wire up -compat policy checking. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com>
On Mon, Oct 25, 2021 at 07:25:24AM +0200, Markus Armbruster wrote: > By convention, names starting with "x-" are experimental. The parts > of external interfaces so named may be withdrawn or changed > incompatibly in future releases. > > Drawback: promoting something from experimental to stable involves a > name change. Client code needs to be updated. > > Moreover, the convention is not universally observed: > > * QOM type "input-barrier" has properties "x-origin", "y-origin". > Looks accidental, but it's ABI since 4.2. > > * QOM types "memory-backend-file", "memory-backend-memfd", > "memory-backend-ram", and "memory-backend-epc" have a property > "x-use-canonical-path-for-ramblock-id" that is documented to be > stable despite its name. Looks like there's another stable property with an "x-" prefix: "x-remote-object", part of QOM type @RemoteObjectProperties. Given the above "x-" properties are now stable, I take it that they cannot be renamed now, as they might break any tools using them? My guess is the tedious way is not worth it: deprecate them, and add the non-x variants as "synonyms". > We could document these exceptions, but documentation helps only > humans. We want to recognize "unstable" in code, like "deprecated". > > Replace the convention by a new special feature flag "unstable". It > will be recognized by the QAPI generator, like the existing feature > flag "deprecated", and unlike regular feature flags. FWIW, sounds good to me. > This commit updates documentation and prepares tests. The next commit > updates the QAPI schema. The remaining patches update the QAPI > generator and wire up -compat policy checking. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > docs/devel/qapi-code-gen.rst | 9 ++++++--- > tests/qapi-schema/qapi-schema-test.json | 7 +++++-- > tests/qapi-schema/qapi-schema-test.out | 5 +++++ > 3 files changed, 16 insertions(+), 5 deletions(-) [...]
On 10/25/21 14:05, Kashyap Chamarthy wrote: > On Mon, Oct 25, 2021 at 07:25:24AM +0200, Markus Armbruster wrote: >> By convention, names starting with "x-" are experimental. The parts >> of external interfaces so named may be withdrawn or changed >> incompatibly in future releases. >> >> Drawback: promoting something from experimental to stable involves a >> name change. Client code needs to be updated. >> >> Moreover, the convention is not universally observed: >> >> * QOM type "input-barrier" has properties "x-origin", "y-origin". >> Looks accidental, but it's ABI since 4.2. >> >> * QOM types "memory-backend-file", "memory-backend-memfd", >> "memory-backend-ram", and "memory-backend-epc" have a property >> "x-use-canonical-path-for-ramblock-id" that is documented to be >> stable despite its name. > > Looks like there's another stable property with an "x-" prefix: > "x-remote-object", part of QOM type @RemoteObjectProperties. IIRC "x-remote-object" and RemoteObjectProperties are not stable.
On Mon, Oct 25, 2021 at 1:26 AM Markus Armbruster <armbru@redhat.com> wrote: > By convention, names starting with "x-" are experimental. The parts > of external interfaces so named may be withdrawn or changed > incompatibly in future releases. > > Drawback: promoting something from experimental to stable involves a > name change. Client code needs to be updated. > > Moreover, the convention is not universally observed: > > * QOM type "input-barrier" has properties "x-origin", "y-origin". > Looks accidental, but it's ABI since 4.2. > > * QOM types "memory-backend-file", "memory-backend-memfd", > "memory-backend-ram", and "memory-backend-epc" have a property > "x-use-canonical-path-for-ramblock-id" that is documented to be > stable despite its name. > > We could document these exceptions, but documentation helps only > humans. We want to recognize "unstable" in code, like "deprecated". > > Replace the convention by a new special feature flag "unstable". It > will be recognized by the QAPI generator, like the existing feature > flag "deprecated", and unlike regular feature flags. > > This commit updates documentation and prepares tests. The next commit > updates the QAPI schema. The remaining patches update the QAPI > generator and wire up -compat policy checking. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > docs/devel/qapi-code-gen.rst | 9 ++++++--- > tests/qapi-schema/qapi-schema-test.json | 7 +++++-- > tests/qapi-schema/qapi-schema-test.out | 5 +++++ > 3 files changed, 16 insertions(+), 5 deletions(-) > > diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst > index 4071c9074a..38f2d7aad3 100644 > --- a/docs/devel/qapi-code-gen.rst > +++ b/docs/devel/qapi-code-gen.rst > @@ -713,6 +713,10 @@ member as deprecated. It is not supported elsewhere > so far. > Interfaces so marked may be withdrawn in future releases in accordance > with QEMU's deprecation policy. > > +Feature "unstable" marks a command, event, enum value, or struct > +member as unstable. It is not supported elsewhere so far. Interfaces > +so marked may be withdrawn or changed incompatibly in future releases. > + > > Naming rules and reserved names > ------------------------------- > @@ -746,9 +750,8 @@ Member name ``u`` and names starting with ``has-`` or > ``has_`` are reserved > for the generator, which uses them for unions and for tracking > optional members. > > -Any name (command, event, type, member, or enum value) beginning with > -``x-`` is marked experimental, and may be withdrawn or changed > -incompatibly in a future release. > +Names beginning with ``x-`` used to signify "experimental". This > +convention has been replaced by special feature "unstable". > > Pragmas ``command-name-exceptions`` and ``member-name-exceptions`` let > you violate naming rules. Use for new code is strongly discouraged. See > diff --git a/tests/qapi-schema/qapi-schema-test.json > b/tests/qapi-schema/qapi-schema-test.json > index b677ab861d..43b8697002 100644 > --- a/tests/qapi-schema/qapi-schema-test.json > +++ b/tests/qapi-schema/qapi-schema-test.json > @@ -273,7 +273,7 @@ > 'data': { 'foo': { 'type': 'int', 'features': [ 'deprecated' ] } }, > 'features': [ 'feature1' ] } > { 'struct': 'FeatureStruct2', > - 'data': { 'foo': 'int' }, > + 'data': { 'foo': { 'type': 'int', 'features': [ 'unstable' ] } }, > 'features': [ { 'name': 'feature1' } ] } > { 'struct': 'FeatureStruct3', > 'data': { 'foo': 'int' }, > @@ -331,7 +331,7 @@ > { 'command': 'test-command-features1', > 'features': [ 'deprecated' ] } > { 'command': 'test-command-features3', > - 'features': [ 'feature1', 'feature2' ] } > + 'features': [ 'unstable', 'feature1', 'feature2' ] } > > { 'command': 'test-command-cond-features1', > 'features': [ { 'name': 'feature1', 'if': 'TEST_IF_FEATURE_1'} ] } > @@ -348,3 +348,6 @@ > > { 'event': 'TEST_EVENT_FEATURES1', > 'features': [ 'deprecated' ] } > + > +{ 'event': 'TEST_EVENT_FEATURES2', > + 'features': [ 'unstable' ] } > diff --git a/tests/qapi-schema/qapi-schema-test.out > b/tests/qapi-schema/qapi-schema-test.out > index 16846dbeb8..1f9585fa9b 100644 > --- a/tests/qapi-schema/qapi-schema-test.out > +++ b/tests/qapi-schema/qapi-schema-test.out > @@ -308,6 +308,7 @@ object FeatureStruct1 > feature feature1 > object FeatureStruct2 > member foo: int optional=False > + feature unstable > feature feature1 > object FeatureStruct3 > member foo: int optional=False > @@ -373,6 +374,7 @@ command test-command-features1 None -> None > feature deprecated > command test-command-features3 None -> None > gen=True success_response=True boxed=False oob=False preconfig=False > + feature unstable > feature feature1 > feature feature2 > command test-command-cond-features1 None -> None > @@ -394,6 +396,9 @@ event TEST_EVENT_FEATURES0 FeatureStruct1 > event TEST_EVENT_FEATURES1 None > boxed=False > feature deprecated > +event TEST_EVENT_FEATURES2 None > + boxed=False > + feature unstable > module include/sub-module.json > include sub-sub-module.json > object SecondArrayRef > -- > 2.31.1 > > Feels odd to combine the doc update *and* test prep, but eh, whatever. Reviewed-by: John Snow <jsnow@redhat.com>
John Snow <jsnow@redhat.com> writes: > On Mon, Oct 25, 2021 at 1:26 AM Markus Armbruster <armbru@redhat.com> wrote: > >> By convention, names starting with "x-" are experimental. The parts >> of external interfaces so named may be withdrawn or changed >> incompatibly in future releases. >> >> Drawback: promoting something from experimental to stable involves a >> name change. Client code needs to be updated. >> >> Moreover, the convention is not universally observed: >> >> * QOM type "input-barrier" has properties "x-origin", "y-origin". >> Looks accidental, but it's ABI since 4.2. >> >> * QOM types "memory-backend-file", "memory-backend-memfd", >> "memory-backend-ram", and "memory-backend-epc" have a property >> "x-use-canonical-path-for-ramblock-id" that is documented to be >> stable despite its name. >> >> We could document these exceptions, but documentation helps only >> humans. We want to recognize "unstable" in code, like "deprecated". >> >> Replace the convention by a new special feature flag "unstable". It >> will be recognized by the QAPI generator, like the existing feature >> flag "deprecated", and unlike regular feature flags. >> >> This commit updates documentation and prepares tests. The next commit >> updates the QAPI schema. The remaining patches update the QAPI >> generator and wire up -compat policy checking. >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> [...] > Feels odd to combine the doc update *and* test prep, but eh, whatever. I admit this is what was left after patch splitting and reshuffling. > Reviewed-by: John Snow <jsnow@redhat.com> Thanks!
Kashyap Chamarthy <kchamart@redhat.com> writes: > On Mon, Oct 25, 2021 at 07:25:24AM +0200, Markus Armbruster wrote: >> By convention, names starting with "x-" are experimental. The parts >> of external interfaces so named may be withdrawn or changed >> incompatibly in future releases. >> >> Drawback: promoting something from experimental to stable involves a >> name change. Client code needs to be updated. >> >> Moreover, the convention is not universally observed: >> >> * QOM type "input-barrier" has properties "x-origin", "y-origin". >> Looks accidental, but it's ABI since 4.2. >> >> * QOM types "memory-backend-file", "memory-backend-memfd", >> "memory-backend-ram", and "memory-backend-epc" have a property >> "x-use-canonical-path-for-ramblock-id" that is documented to be >> stable despite its name. > > Looks like there's another stable property with an "x-" prefix: > "x-remote-object", part of QOM type @RemoteObjectProperties. The union branch 'x-remote-object' isn't flagged 'unstable' (because union branches can't have feature flags), but the enumeration value 'x-remote-object' is. Sufficient, because you can't use the branch without using the enumeration value. Admittedly subtle. I wrote a bit of code (appended) to make sure I don't miss names. > Given the above "x-" properties are now stable, I take it that they > cannot be renamed now, as they might break any tools using them? My > guess is the tedious way is not worth it: deprecate them, and add the > non-x variants as "synonyms". "x-use-canonical-path-for-ramblock-id" goes back to commit fa0cb34d22 "hostmem: use object id for memory region name with >= 4.0" (v4.0). It may have been intended to be internal back then. It wasn't anymore when commit 8db0b20415 "machine: add missing doc for memory-backend option" (v6.0) documented it: And document that x-use-canonical-path-for-ramblock-id, is considered to be stable to make sure it won't go away by accident. x- was intended for unstable/iternal properties, and not supposed to be stable option. However it's too late to rename (drop x-) it as it would mean that users will have to mantain both x-use-canonical-path-for-ramblock-id (for QEMU 5.0-5.2) versions and prefix-less for later versions. Igor's reasoning still applies. "x-origin" has always been stable. Same argument. >> We could document these exceptions, but documentation helps only >> humans. We want to recognize "unstable" in code, like "deprecated". >> >> Replace the convention by a new special feature flag "unstable". It >> will be recognized by the QAPI generator, like the existing feature >> flag "deprecated", and unlike regular feature flags. > > FWIW, sounds good to me. Thanks! >> This commit updates documentation and prepares tests. The next commit >> updates the QAPI schema. The remaining patches update the QAPI >> generator and wire up -compat policy checking. >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- >> docs/devel/qapi-code-gen.rst | 9 ++++++--- >> tests/qapi-schema/qapi-schema-test.json | 7 +++++-- >> tests/qapi-schema/qapi-schema-test.out | 5 +++++ >> 3 files changed, 16 insertions(+), 5 deletions(-) > > [...] commit 415b71a9f6e5bc37e84895d2e767cf4cfacd279b (HEAD) Author: Markus Armbruster <armbru@redhat.com> Date: Sat Oct 9 09:01:21 2021 +0200 qapi: Find x- without feature unstable DBG diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index b7b3fc0ce4..f2af1d7eea 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -14,6 +14,8 @@ # TODO catching name collisions in generated code would be nice +import sys + from collections import OrderedDict import os import re @@ -118,6 +120,11 @@ def describe(self): return "%s '%s'" % (self.meta, self.name) +def check_have_feature_unstable(name, info, features): + if name.startswith('x-') and 'unstable' not in (f.name for f in features): + print(QAPISemError(info, f"XXX %{name} %{features}"), file=sys.stderr) + + class QAPISchemaVisitor: def visit_begin(self, schema): pass @@ -718,6 +725,7 @@ def __init__(self, name, info, ifcond=None, features=None): self.features = features or [] def connect_doc(self, doc): + check_have_feature_unstable(self.name, self.info, self.features) super().connect_doc(doc) if doc: for f in self.features: @@ -745,6 +753,7 @@ def __init__(self, name, info, typ, optional, ifcond=None, features=None): self.features = features or [] def check(self, schema): + check_have_feature_unstable(self.name, self.info, self.features) assert self.defined_in self.type = schema.resolve_type(self._type_name, self.info, self.describe) @@ -789,6 +798,7 @@ def __init__(self, name, info, doc, ifcond, features, def check(self, schema): super().check(schema) + check_have_feature_unstable(self.name, self.info, self.features) if self._arg_type_name: self.arg_type = schema.resolve_type( self._arg_type_name, self.info, "command's 'data'") @@ -844,6 +854,7 @@ def __init__(self, name, info, doc, ifcond, features, arg_type, boxed): def check(self, schema): super().check(schema) + check_have_feature_unstable(self.name, self.info, self.features) if self._arg_type_name: self.arg_type = schema.resolve_type( self._arg_type_name, self.info, "event's 'data'")
Am 25.10.2021 um 07:25 hat Markus Armbruster geschrieben: > By convention, names starting with "x-" are experimental. The parts > of external interfaces so named may be withdrawn or changed > incompatibly in future releases. > > Drawback: promoting something from experimental to stable involves a > name change. Client code needs to be updated. > > Moreover, the convention is not universally observed: > > * QOM type "input-barrier" has properties "x-origin", "y-origin". > Looks accidental, but it's ABI since 4.2. > > * QOM types "memory-backend-file", "memory-backend-memfd", > "memory-backend-ram", and "memory-backend-epc" have a property > "x-use-canonical-path-for-ramblock-id" that is documented to be > stable despite its name. > > We could document these exceptions, but documentation helps only > humans. We want to recognize "unstable" in code, like "deprecated". > > Replace the convention by a new special feature flag "unstable". It > will be recognized by the QAPI generator, like the existing feature > flag "deprecated", and unlike regular feature flags. > > This commit updates documentation and prepares tests. The next commit > updates the QAPI schema. The remaining patches update the QAPI > generator and wire up -compat policy checking. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> Obviously, replacing the old convention gets rid of the old drawbacks, but adds a new one: While using x- makes it very obvious for a human user that this is an unstable feature, a feature flag in the schema will almost certainly go unnoticed in manual use. Kevin
On Tue, Oct 26, 2021 at 09:15:30AM +0200, Markus Armbruster wrote: > Kashyap Chamarthy <kchamart@redhat.com> writes: > > > On Mon, Oct 25, 2021 at 07:25:24AM +0200, Markus Armbruster wrote: [...] > > Looks like there's another stable property with an "x-" prefix: > > "x-remote-object", part of QOM type @RemoteObjectProperties. > > The union branch 'x-remote-object' isn't flagged 'unstable' (because > union branches can't have feature flags), but the enumeration value > 'x-remote-object' is. Sufficient, because you can't use the branch > without using the enumeration value. Admittedly subtle. Ah, thanks for the explanation. I missed the union branch part, which I now notice in your "[PATCH 2/9] qapi: Mark unstable QMP parts with feature 'unstable'". > I wrote a bit of code (appended) to make sure I don't miss names. Thanks; looks good to me. > > Given the above "x-" properties are now stable, I take it that they > > cannot be renamed now, as they might break any tools using them? My > > guess is the tedious way is not worth it: deprecate them, and add the > > non-x variants as "synonyms". > > "x-use-canonical-path-for-ramblock-id" goes back to commit fa0cb34d22 > "hostmem: use object id for memory region name with >= 4.0" (v4.0). It > may have been intended to be internal back then. It wasn't anymore when > commit 8db0b20415 "machine: add missing doc for memory-backend option" > (v6.0) documented it: > > And document that x-use-canonical-path-for-ramblock-id, > is considered to be stable to make sure it won't go away by accident. > > x- was intended for unstable/iternal properties, and not supposed to > be stable option. However it's too late to rename (drop x-) > it as it would mean that users will have to mantain both > x-use-canonical-path-for-ramblock-id (for QEMU 5.0-5.2) versions > and prefix-less for later versions. > > Igor's reasoning still applies. > > "x-origin" has always been stable. Same argument. Yep, fair enough. [...] > commit 415b71a9f6e5bc37e84895d2e767cf4cfacd279b (HEAD) > Author: Markus Armbruster <armbru@redhat.com> > Date: Sat Oct 9 09:01:21 2021 +0200 > > qapi: Find x- without feature unstable DBG > > diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py > index b7b3fc0ce4..f2af1d7eea 100644 > --- a/scripts/qapi/schema.py > +++ b/scripts/qapi/schema.py > @@ -14,6 +14,8 @@ > > # TODO catching name collisions in generated code would be nice > > +import sys > + > from collections import OrderedDict > import os > import re > @@ -118,6 +120,11 @@ def describe(self): > return "%s '%s'" % (self.meta, self.name) > > > +def check_have_feature_unstable(name, info, features): > + if name.startswith('x-') and 'unstable' not in (f.name for f in features): > + print(QAPISemError(info, f"XXX %{name} %{features}"), file=sys.stderr) > + > + > class QAPISchemaVisitor: > def visit_begin(self, schema): > pass > @@ -718,6 +725,7 @@ def __init__(self, name, info, ifcond=None, features=None): > self.features = features or [] > > def connect_doc(self, doc): > + check_have_feature_unstable(self.name, self.info, self.features) > super().connect_doc(doc) > if doc: > for f in self.features: > @@ -745,6 +753,7 @@ def __init__(self, name, info, typ, optional, ifcond=None, features=None): > self.features = features or [] > > def check(self, schema): > + check_have_feature_unstable(self.name, self.info, self.features) > assert self.defined_in > self.type = schema.resolve_type(self._type_name, self.info, > self.describe) > @@ -789,6 +798,7 @@ def __init__(self, name, info, doc, ifcond, features, > > def check(self, schema): > super().check(schema) > + check_have_feature_unstable(self.name, self.info, self.features) > if self._arg_type_name: > self.arg_type = schema.resolve_type( > self._arg_type_name, self.info, "command's 'data'") > @@ -844,6 +854,7 @@ def __init__(self, name, info, doc, ifcond, features, arg_type, boxed): > > def check(self, schema): > super().check(schema) > + check_have_feature_unstable(self.name, self.info, self.features) > if self._arg_type_name: > self.arg_type = schema.resolve_type( > self._arg_type_name, self.info, "event's 'data'") TIL: the error class QAPISemError() Reviewed-by: Kashyap Chamarthy <kchamart@redhat.com>
On Tue, Oct 26, 2021 at 09:37:29AM +0200, Kevin Wolf wrote: > Am 25.10.2021 um 07:25 hat Markus Armbruster geschrieben: [...] > > This commit updates documentation and prepares tests. The next commit > > updates the QAPI schema. The remaining patches update the QAPI > > generator and wire up -compat policy checking. > > > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > > Obviously, replacing the old convention gets rid of the old drawbacks, > but adds a new one: While using x- makes it very obvious for a human > user that this is an unstable feature, a feature flag in the schema will > almost certainly go unnoticed in manual use. FWIW, I wondered about it too, as I like the visual reminder of the "x-" prefix. Then I thought, "how many people besides QEMU and related tooling developers -- who would read QAPI docs anyway :) -- launch experimental QEMU commands manually?" Maybe that's too big of a leap.
* Kevin Wolf (kwolf@redhat.com) wrote: > Am 25.10.2021 um 07:25 hat Markus Armbruster geschrieben: > > By convention, names starting with "x-" are experimental. The parts > > of external interfaces so named may be withdrawn or changed > > incompatibly in future releases. > > > > Drawback: promoting something from experimental to stable involves a > > name change. Client code needs to be updated. > > > > Moreover, the convention is not universally observed: > > > > * QOM type "input-barrier" has properties "x-origin", "y-origin". > > Looks accidental, but it's ABI since 4.2. > > > > * QOM types "memory-backend-file", "memory-backend-memfd", > > "memory-backend-ram", and "memory-backend-epc" have a property > > "x-use-canonical-path-for-ramblock-id" that is documented to be > > stable despite its name. > > > > We could document these exceptions, but documentation helps only > > humans. We want to recognize "unstable" in code, like "deprecated". > > > > Replace the convention by a new special feature flag "unstable". It > > will be recognized by the QAPI generator, like the existing feature > > flag "deprecated", and unlike regular feature flags. > > > > This commit updates documentation and prepares tests. The next commit > > updates the QAPI schema. The remaining patches update the QAPI > > generator and wire up -compat policy checking. > > > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > > Obviously, replacing the old convention gets rid of the old drawbacks, > but adds a new one: While using x- makes it very obvious for a human > user that this is an unstable feature, a feature flag in the schema will > almost certainly go unnoticed in manual use. Agreed, I'd keep the x- as well. Having said that, the x- represents a few different things (that we don't currently distinguish): - experimental - for internal use - for debugging/human use Dave > Kevin >
On Tue, Oct 26, 2021 at 10:22:15AM +0100, Dr. David Alan Gilbert wrote: > * Kevin Wolf (kwolf@redhat.com) wrote: > > Am 25.10.2021 um 07:25 hat Markus Armbruster geschrieben: > > > By convention, names starting with "x-" are experimental. The parts > > > of external interfaces so named may be withdrawn or changed > > > incompatibly in future releases. > > > > > > Drawback: promoting something from experimental to stable involves a > > > name change. Client code needs to be updated. > > > > > > Moreover, the convention is not universally observed: > > > > > > * QOM type "input-barrier" has properties "x-origin", "y-origin". > > > Looks accidental, but it's ABI since 4.2. > > > > > > * QOM types "memory-backend-file", "memory-backend-memfd", > > > "memory-backend-ram", and "memory-backend-epc" have a property > > > "x-use-canonical-path-for-ramblock-id" that is documented to be > > > stable despite its name. > > > > > > We could document these exceptions, but documentation helps only > > > humans. We want to recognize "unstable" in code, like "deprecated". > > > > > > Replace the convention by a new special feature flag "unstable". It > > > will be recognized by the QAPI generator, like the existing feature > > > flag "deprecated", and unlike regular feature flags. > > > > > > This commit updates documentation and prepares tests. The next commit > > > updates the QAPI schema. The remaining patches update the QAPI > > > generator and wire up -compat policy checking. > > > > > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > > > > Obviously, replacing the old convention gets rid of the old drawbacks, > > but adds a new one: While using x- makes it very obvious for a human > > user that this is an unstable feature, a feature flag in the schema will > > almost certainly go unnoticed in manual use. > > Agreed, I'd keep the x- as well. > > Having said that, the x- represents a few different things (that we > don't currently distinguish): > - experimental > - for internal use > - for debugging/human use All of those usage scenarios have the same implication though: Command/data format is liable to change in incompatible ways, or be deleted, with no prior warning. I don't think we need to distinguish the use cases - some commands may belong to two or three of those use cases. All that matters is that they're considered "unstable" from an API compat POV. Regards, Daniel
Kevin Wolf <kwolf@redhat.com> writes: > Am 25.10.2021 um 07:25 hat Markus Armbruster geschrieben: >> By convention, names starting with "x-" are experimental. The parts >> of external interfaces so named may be withdrawn or changed >> incompatibly in future releases. >> >> Drawback: promoting something from experimental to stable involves a >> name change. Client code needs to be updated. >> >> Moreover, the convention is not universally observed: >> >> * QOM type "input-barrier" has properties "x-origin", "y-origin". >> Looks accidental, but it's ABI since 4.2. >> >> * QOM types "memory-backend-file", "memory-backend-memfd", >> "memory-backend-ram", and "memory-backend-epc" have a property >> "x-use-canonical-path-for-ramblock-id" that is documented to be >> stable despite its name. >> >> We could document these exceptions, but documentation helps only >> humans. We want to recognize "unstable" in code, like "deprecated". >> >> Replace the convention by a new special feature flag "unstable". It >> will be recognized by the QAPI generator, like the existing feature >> flag "deprecated", and unlike regular feature flags. >> >> This commit updates documentation and prepares tests. The next commit >> updates the QAPI schema. The remaining patches update the QAPI >> generator and wire up -compat policy checking. >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> > > Obviously, replacing the old convention gets rid of the old drawbacks, > but adds a new one: While using x- makes it very obvious for a human > user that this is an unstable feature, a feature flag in the schema will > almost certainly go unnoticed in manual use. I thought about this, but neglected to put it in writing. My bad. Manual use of unstable interfaces is mostly fine. Human users can adapt to changing interfaces. HMP works that way. Management applications are better off with a feature flag than with a naming convention we sometimes ignore. The most potential for trouble is in between: programs that aren't full-fledged management applications. If we want to keep "unstable" obvious to the humans who write such programs, we can continue to require "x-", in addition to the feature flag. We pay for it with renames, and the risk of forgetting to rename in time (which is what got us the awkward stable "x-use-canonical-path-for-ramblock-id"). Tradeoff. I chose not to, but if y'all think we should... What we can't do, at least not easily, is to use *only* the "x-" convention: it is not reliable. We'd have to add a way to say 'this is stable even though the name starts with "x-"'.
Am 26.10.2021 um 11:37 hat Markus Armbruster geschrieben: > Kevin Wolf <kwolf@redhat.com> writes: > > > Am 25.10.2021 um 07:25 hat Markus Armbruster geschrieben: > >> By convention, names starting with "x-" are experimental. The parts > >> of external interfaces so named may be withdrawn or changed > >> incompatibly in future releases. > >> > >> Drawback: promoting something from experimental to stable involves a > >> name change. Client code needs to be updated. > >> > >> Moreover, the convention is not universally observed: > >> > >> * QOM type "input-barrier" has properties "x-origin", "y-origin". > >> Looks accidental, but it's ABI since 4.2. > >> > >> * QOM types "memory-backend-file", "memory-backend-memfd", > >> "memory-backend-ram", and "memory-backend-epc" have a property > >> "x-use-canonical-path-for-ramblock-id" that is documented to be > >> stable despite its name. > >> > >> We could document these exceptions, but documentation helps only > >> humans. We want to recognize "unstable" in code, like "deprecated". > >> > >> Replace the convention by a new special feature flag "unstable". It > >> will be recognized by the QAPI generator, like the existing feature > >> flag "deprecated", and unlike regular feature flags. > >> > >> This commit updates documentation and prepares tests. The next commit > >> updates the QAPI schema. The remaining patches update the QAPI > >> generator and wire up -compat policy checking. > >> > >> Signed-off-by: Markus Armbruster <armbru@redhat.com> > > > > Obviously, replacing the old convention gets rid of the old drawbacks, > > but adds a new one: While using x- makes it very obvious for a human > > user that this is an unstable feature, a feature flag in the schema will > > almost certainly go unnoticed in manual use. > > I thought about this, but neglected to put it in writing. My bad. > > Manual use of unstable interfaces is mostly fine. Human users can adapt > to changing interfaces. HMP works that way. > > Management applications are better off with a feature flag than with a > naming convention we sometimes ignore. > > The most potential for trouble is in between: programs that aren't > full-fledged management applications. > > If we want to keep "unstable" obvious to the humans who write such > programs, we can continue to require "x-", in addition to the feature > flag. We pay for it with renames, and the risk of forgetting to rename > in time (which is what got us the awkward stable > "x-use-canonical-path-for-ramblock-id"). Tradeoff. I chose not to, but > if y'all think we should... Just to clarify, I'm not implying that we should keep it. I'm merely pointing out that there is a tradeoff that requires us to make a choice. The decision for one of the options should be explicit rather than just happening as a side effect. Documenting that it was a conscious decision is probably best done by adding the reasoning for it to the commit message. > What we can't do, at least not easily, is to use *only* the "x-" > convention: it is not reliable. We'd have to add a way to say 'this is > stable even though the name starts with "x-"'. No question. Kevin
On Tue, Oct 26, 2021 at 11:37:19AM +0200, Markus Armbruster wrote: > Kevin Wolf <kwolf@redhat.com> writes: > > > Am 25.10.2021 um 07:25 hat Markus Armbruster geschrieben: > >> By convention, names starting with "x-" are experimental. The parts > >> of external interfaces so named may be withdrawn or changed > >> incompatibly in future releases. > >> > >> Drawback: promoting something from experimental to stable involves a > >> name change. Client code needs to be updated. > >> > >> Moreover, the convention is not universally observed: > >> > >> * QOM type "input-barrier" has properties "x-origin", "y-origin". > >> Looks accidental, but it's ABI since 4.2. > >> > >> * QOM types "memory-backend-file", "memory-backend-memfd", > >> "memory-backend-ram", and "memory-backend-epc" have a property > >> "x-use-canonical-path-for-ramblock-id" that is documented to be > >> stable despite its name. > >> > >> We could document these exceptions, but documentation helps only > >> humans. We want to recognize "unstable" in code, like "deprecated". > >> > >> Replace the convention by a new special feature flag "unstable". It > >> will be recognized by the QAPI generator, like the existing feature > >> flag "deprecated", and unlike regular feature flags. > >> > >> This commit updates documentation and prepares tests. The next commit > >> updates the QAPI schema. The remaining patches update the QAPI > >> generator and wire up -compat policy checking. > >> > >> Signed-off-by: Markus Armbruster <armbru@redhat.com> > > > > Obviously, replacing the old convention gets rid of the old drawbacks, > > but adds a new one: While using x- makes it very obvious for a human > > user that this is an unstable feature, a feature flag in the schema will > > almost certainly go unnoticed in manual use. > > I thought about this, but neglected to put it in writing. My bad. > > Manual use of unstable interfaces is mostly fine. Human users can adapt > to changing interfaces. HMP works that way. > > Management applications are better off with a feature flag than with a > naming convention we sometimes ignore. We will sometimes ignore/forget the feature flag too though, so I'm not convinced there's much difference there. > If we want to keep "unstable" obvious to the humans who write such > programs, we can continue to require "x-", in addition to the feature > flag. We pay for it with renames, and the risk of forgetting to rename > in time (which is what got us the awkward stable > "x-use-canonical-path-for-ramblock-id"). Tradeoff. I chose not to, but > if y'all think we should... IMHO the renames arguably make life easier for mgmt apps, as they only need to check for the name without the x- prefix, and they know they won't be accidentally using the fature from an older QEMU where it was unstable because the older QEMU had a different name they won't be checking for. We can just as easily forget to remove the "unstable" feature flag, as forget to rename. The plus point about the feature flag is that it is aligned with the "deprecated" feature flag, and potentially aligned with a future "insecure" feature flag. Regards, Daniel
Daniel P. Berrangé <berrange@redhat.com> writes: > On Tue, Oct 26, 2021 at 11:37:19AM +0200, Markus Armbruster wrote: >> Kevin Wolf <kwolf@redhat.com> writes: >> >> > Am 25.10.2021 um 07:25 hat Markus Armbruster geschrieben: >> >> By convention, names starting with "x-" are experimental. The parts >> >> of external interfaces so named may be withdrawn or changed >> >> incompatibly in future releases. >> >> >> >> Drawback: promoting something from experimental to stable involves a >> >> name change. Client code needs to be updated. >> >> >> >> Moreover, the convention is not universally observed: >> >> >> >> * QOM type "input-barrier" has properties "x-origin", "y-origin". >> >> Looks accidental, but it's ABI since 4.2. >> >> >> >> * QOM types "memory-backend-file", "memory-backend-memfd", >> >> "memory-backend-ram", and "memory-backend-epc" have a property >> >> "x-use-canonical-path-for-ramblock-id" that is documented to be >> >> stable despite its name. >> >> >> >> We could document these exceptions, but documentation helps only >> >> humans. We want to recognize "unstable" in code, like "deprecated". >> >> >> >> Replace the convention by a new special feature flag "unstable". It >> >> will be recognized by the QAPI generator, like the existing feature >> >> flag "deprecated", and unlike regular feature flags. >> >> >> >> This commit updates documentation and prepares tests. The next commit >> >> updates the QAPI schema. The remaining patches update the QAPI >> >> generator and wire up -compat policy checking. >> >> >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> > >> > Obviously, replacing the old convention gets rid of the old drawbacks, >> > but adds a new one: While using x- makes it very obvious for a human >> > user that this is an unstable feature, a feature flag in the schema will >> > almost certainly go unnoticed in manual use. >> >> I thought about this, but neglected to put it in writing. My bad. >> >> Manual use of unstable interfaces is mostly fine. Human users can adapt >> to changing interfaces. HMP works that way. >> >> Management applications are better off with a feature flag than with a >> naming convention we sometimes ignore. > > We will sometimes ignore/forget the feature flag too though, so I'm > not convinced there's much difference there. -compat unstable-input=reject,unstable-output=hide should help you stay on the straight & narrow :) >> If we want to keep "unstable" obvious to the humans who write such >> programs, we can continue to require "x-", in addition to the feature >> flag. We pay for it with renames, and the risk of forgetting to rename >> in time (which is what got us the awkward stable >> "x-use-canonical-path-for-ramblock-id"). Tradeoff. I chose not to, but >> if y'all think we should... > > IMHO the renames arguably make life easier for mgmt apps, as they > only need to check for the name without the x- prefix, and they > know they won't be accidentally using the fature from an older > QEMU where it was unstable because the older QEMU had a different > name they won't be checking for. > > We can just as easily forget to remove the "unstable" feature > flag, as forget to rename. > > The plus point about the feature flag is that it is aligned with > the "deprecated" feature flag, and potentially aligned with a > future "insecure" feature flag. Yup.
On Tue, Oct 26, 2021 at 05:15:10PM +0200, Markus Armbruster wrote: > Daniel P. Berrangé <berrange@redhat.com> writes: > > > On Tue, Oct 26, 2021 at 11:37:19AM +0200, Markus Armbruster wrote: > >> Kevin Wolf <kwolf@redhat.com> writes: > >> > >> > Am 25.10.2021 um 07:25 hat Markus Armbruster geschrieben: > >> >> By convention, names starting with "x-" are experimental. The parts > >> >> of external interfaces so named may be withdrawn or changed > >> >> incompatibly in future releases. > >> >> > >> >> Drawback: promoting something from experimental to stable involves a > >> >> name change. Client code needs to be updated. > >> >> > >> >> Moreover, the convention is not universally observed: > >> >> > >> >> * QOM type "input-barrier" has properties "x-origin", "y-origin". > >> >> Looks accidental, but it's ABI since 4.2. > >> >> > >> >> * QOM types "memory-backend-file", "memory-backend-memfd", > >> >> "memory-backend-ram", and "memory-backend-epc" have a property > >> >> "x-use-canonical-path-for-ramblock-id" that is documented to be > >> >> stable despite its name. > >> >> > >> >> We could document these exceptions, but documentation helps only > >> >> humans. We want to recognize "unstable" in code, like "deprecated". > >> >> > >> >> Replace the convention by a new special feature flag "unstable". It > >> >> will be recognized by the QAPI generator, like the existing feature > >> >> flag "deprecated", and unlike regular feature flags. > >> >> > >> >> This commit updates documentation and prepares tests. The next commit > >> >> updates the QAPI schema. The remaining patches update the QAPI > >> >> generator and wire up -compat policy checking. > >> >> > >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> > >> > > >> > Obviously, replacing the old convention gets rid of the old drawbacks, > >> > but adds a new one: While using x- makes it very obvious for a human > >> > user that this is an unstable feature, a feature flag in the schema will > >> > almost certainly go unnoticed in manual use. > >> > >> I thought about this, but neglected to put it in writing. My bad. > >> > >> Manual use of unstable interfaces is mostly fine. Human users can adapt > >> to changing interfaces. HMP works that way. > >> > >> Management applications are better off with a feature flag than with a > >> naming convention we sometimes ignore. > > > > We will sometimes ignore/forget the feature flag too though, so I'm > > not convinced there's much difference there. > > -compat unstable-input=reject,unstable-output=hide should help you stay > on the straight & narrow :) That's from the pov of the mgmt app. I meant from the POV of QEMU maintainers forgetting to add "unstable" flag, just as they might forget to add a "x-" prefix. Regards, Daniel
Daniel P. Berrangé <berrange@redhat.com> writes: > On Tue, Oct 26, 2021 at 05:15:10PM +0200, Markus Armbruster wrote: >> Daniel P. Berrangé <berrange@redhat.com> writes: >> >> > On Tue, Oct 26, 2021 at 11:37:19AM +0200, Markus Armbruster wrote: [...] >> >> Management applications are better off with a feature flag than with a >> >> naming convention we sometimes ignore. >> > >> > We will sometimes ignore/forget the feature flag too though, so I'm >> > not convinced there's much difference there. >> >> -compat unstable-input=reject,unstable-output=hide should help you stay >> on the straight & narrow :) > > That's from the pov of the mgmt app. I meant from the POV of QEMU > maintainers forgetting to add "unstable" flag, just as they might > forget to add a "x-" prefix. Got it. My point was that feature flag "unstable" is an unequivocal signal for "this thing is unstable", while a name starting with "x-" isn't: there are exceptions. The converse is a wash: we can forget to mark something unstable no matter how the mark works.
Kevin Wolf <kwolf@redhat.com> writes: > Am 26.10.2021 um 11:37 hat Markus Armbruster geschrieben: >> Kevin Wolf <kwolf@redhat.com> writes: >> >> > Am 25.10.2021 um 07:25 hat Markus Armbruster geschrieben: >> >> By convention, names starting with "x-" are experimental. The parts >> >> of external interfaces so named may be withdrawn or changed >> >> incompatibly in future releases. >> >> >> >> Drawback: promoting something from experimental to stable involves a >> >> name change. Client code needs to be updated. >> >> >> >> Moreover, the convention is not universally observed: >> >> >> >> * QOM type "input-barrier" has properties "x-origin", "y-origin". >> >> Looks accidental, but it's ABI since 4.2. >> >> >> >> * QOM types "memory-backend-file", "memory-backend-memfd", >> >> "memory-backend-ram", and "memory-backend-epc" have a property >> >> "x-use-canonical-path-for-ramblock-id" that is documented to be >> >> stable despite its name. >> >> >> >> We could document these exceptions, but documentation helps only >> >> humans. We want to recognize "unstable" in code, like "deprecated". >> >> >> >> Replace the convention by a new special feature flag "unstable". It >> >> will be recognized by the QAPI generator, like the existing feature >> >> flag "deprecated", and unlike regular feature flags. >> >> >> >> This commit updates documentation and prepares tests. The next commit >> >> updates the QAPI schema. The remaining patches update the QAPI >> >> generator and wire up -compat policy checking. >> >> >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> > >> > Obviously, replacing the old convention gets rid of the old drawbacks, >> > but adds a new one: While using x- makes it very obvious for a human >> > user that this is an unstable feature, a feature flag in the schema will >> > almost certainly go unnoticed in manual use. >> >> I thought about this, but neglected to put it in writing. My bad. >> >> Manual use of unstable interfaces is mostly fine. Human users can adapt >> to changing interfaces. HMP works that way. >> >> Management applications are better off with a feature flag than with a >> naming convention we sometimes ignore. >> >> The most potential for trouble is in between: programs that aren't >> full-fledged management applications. >> >> If we want to keep "unstable" obvious to the humans who write such >> programs, we can continue to require "x-", in addition to the feature >> flag. We pay for it with renames, and the risk of forgetting to rename >> in time (which is what got us the awkward stable >> "x-use-canonical-path-for-ramblock-id"). Tradeoff. I chose not to, but >> if y'all think we should... > > Just to clarify, I'm not implying that we should keep it. I'm merely > pointing out that there is a tradeoff that requires us to make a choice. > The decision for one of the options should be explicit rather than just > happening as a side effect. Documenting that it was a conscious decision > is probably best done by adding the reasoning for it to the commit > message. I rewrote the commit message for v2. Thanks! [...]
diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst index 4071c9074a..38f2d7aad3 100644 --- a/docs/devel/qapi-code-gen.rst +++ b/docs/devel/qapi-code-gen.rst @@ -713,6 +713,10 @@ member as deprecated. It is not supported elsewhere so far. Interfaces so marked may be withdrawn in future releases in accordance with QEMU's deprecation policy. +Feature "unstable" marks a command, event, enum value, or struct +member as unstable. It is not supported elsewhere so far. Interfaces +so marked may be withdrawn or changed incompatibly in future releases. + Naming rules and reserved names ------------------------------- @@ -746,9 +750,8 @@ Member name ``u`` and names starting with ``has-`` or ``has_`` are reserved for the generator, which uses them for unions and for tracking optional members. -Any name (command, event, type, member, or enum value) beginning with -``x-`` is marked experimental, and may be withdrawn or changed -incompatibly in a future release. +Names beginning with ``x-`` used to signify "experimental". This +convention has been replaced by special feature "unstable". Pragmas ``command-name-exceptions`` and ``member-name-exceptions`` let you violate naming rules. Use for new code is strongly discouraged. See diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json index b677ab861d..43b8697002 100644 --- a/tests/qapi-schema/qapi-schema-test.json +++ b/tests/qapi-schema/qapi-schema-test.json @@ -273,7 +273,7 @@ 'data': { 'foo': { 'type': 'int', 'features': [ 'deprecated' ] } }, 'features': [ 'feature1' ] } { 'struct': 'FeatureStruct2', - 'data': { 'foo': 'int' }, + 'data': { 'foo': { 'type': 'int', 'features': [ 'unstable' ] } }, 'features': [ { 'name': 'feature1' } ] } { 'struct': 'FeatureStruct3', 'data': { 'foo': 'int' }, @@ -331,7 +331,7 @@ { 'command': 'test-command-features1', 'features': [ 'deprecated' ] } { 'command': 'test-command-features3', - 'features': [ 'feature1', 'feature2' ] } + 'features': [ 'unstable', 'feature1', 'feature2' ] } { 'command': 'test-command-cond-features1', 'features': [ { 'name': 'feature1', 'if': 'TEST_IF_FEATURE_1'} ] } @@ -348,3 +348,6 @@ { 'event': 'TEST_EVENT_FEATURES1', 'features': [ 'deprecated' ] } + +{ 'event': 'TEST_EVENT_FEATURES2', + 'features': [ 'unstable' ] } diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out index 16846dbeb8..1f9585fa9b 100644 --- a/tests/qapi-schema/qapi-schema-test.out +++ b/tests/qapi-schema/qapi-schema-test.out @@ -308,6 +308,7 @@ object FeatureStruct1 feature feature1 object FeatureStruct2 member foo: int optional=False + feature unstable feature feature1 object FeatureStruct3 member foo: int optional=False @@ -373,6 +374,7 @@ command test-command-features1 None -> None feature deprecated command test-command-features3 None -> None gen=True success_response=True boxed=False oob=False preconfig=False + feature unstable feature feature1 feature feature2 command test-command-cond-features1 None -> None @@ -394,6 +396,9 @@ event TEST_EVENT_FEATURES0 FeatureStruct1 event TEST_EVENT_FEATURES1 None boxed=False feature deprecated +event TEST_EVENT_FEATURES2 None + boxed=False + feature unstable module include/sub-module.json include sub-sub-module.json object SecondArrayRef
By convention, names starting with "x-" are experimental. The parts of external interfaces so named may be withdrawn or changed incompatibly in future releases. Drawback: promoting something from experimental to stable involves a name change. Client code needs to be updated. Moreover, the convention is not universally observed: * QOM type "input-barrier" has properties "x-origin", "y-origin". Looks accidental, but it's ABI since 4.2. * QOM types "memory-backend-file", "memory-backend-memfd", "memory-backend-ram", and "memory-backend-epc" have a property "x-use-canonical-path-for-ramblock-id" that is documented to be stable despite its name. We could document these exceptions, but documentation helps only humans. We want to recognize "unstable" in code, like "deprecated". Replace the convention by a new special feature flag "unstable". It will be recognized by the QAPI generator, like the existing feature flag "deprecated", and unlike regular feature flags. This commit updates documentation and prepares tests. The next commit updates the QAPI schema. The remaining patches update the QAPI generator and wire up -compat policy checking. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- docs/devel/qapi-code-gen.rst | 9 ++++++--- tests/qapi-schema/qapi-schema-test.json | 7 +++++-- tests/qapi-schema/qapi-schema-test.out | 5 +++++ 3 files changed, 16 insertions(+), 5 deletions(-)