Add special feature 'unstable' everywhere the name starts with 'x-',
except for InputBarrierProperties member x-origin and
MemoryBackendProperties member x-use-canonical-path-for-ramblock-id,
because these two are actually stable.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qapi/block-core.json | 123 +++++++++++++++++++++++++++++++------------
qapi/migration.json | 35 +++++++++---
qapi/misc.json | 6 ++-
qapi/qom.json | 11 ++--
4 files changed, 130 insertions(+), 45 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 6d3217abb6..ce2c1352cb 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1438,6 +1438,9 @@
#
# @x-perf: Performance options. (Since 6.0)
#
+# Features:
+# @unstable: Member @x-perf is experimental.
+#
# Note: @on-source-error and @on-target-error only affect background
# I/O. If an error occurs during a guest write request, the device's
# rerror/werror actions will be used.
@@ -1452,7 +1455,9 @@
'*on-source-error': 'BlockdevOnError',
'*on-target-error': 'BlockdevOnError',
'*auto-finalize': 'bool', '*auto-dismiss': 'bool',
- '*filter-node-name': 'str', '*x-perf': 'BackupPerf' } }
+ '*filter-node-name': 'str',
+ '*x-perf': { 'type': 'BackupPerf',
+ 'features': [ 'unstable' ] } } }
##
# @DriveBackup:
@@ -1916,9 +1921,13 @@
#
# Get the block graph.
#
+# Features:
+# @unstable: This command is meant for debugging.
+#
# Since: 4.0
##
-{ 'command': 'x-debug-query-block-graph', 'returns': 'XDbgBlockGraph' }
+{ 'command': 'x-debug-query-block-graph', 'returns': 'XDbgBlockGraph',
+ 'features': [ 'unstable' ] }
##
# @drive-mirror:
@@ -2257,6 +2266,9 @@
#
# Get bitmap SHA256.
#
+# Features:
+# @unstable: This command is meant for debugging.
+#
# Returns: - BlockDirtyBitmapSha256 on success
# - If @node is not a valid block device, DeviceNotFound
# - If @name is not found or if hashing has failed, GenericError with an
@@ -2265,7 +2277,8 @@
# Since: 2.10
##
{ 'command': 'x-debug-block-dirty-bitmap-sha256',
- 'data': 'BlockDirtyBitmap', 'returns': 'BlockDirtyBitmapSha256' }
+ 'data': 'BlockDirtyBitmap', 'returns': 'BlockDirtyBitmapSha256',
+ 'features': [ 'unstable' ] }
##
# @blockdev-mirror:
@@ -2495,27 +2508,57 @@
#
# Properties for throttle-group objects.
#
-# The options starting with x- are aliases for the same key without x- in
-# the @limits object. As indicated by the x- prefix, this is not a stable
-# interface and may be removed or changed incompatibly in the future. Use
-# @limits for a supported stable interface.
-#
# @limits: limits to apply for this throttle group
#
+# Features:
+# @unstable: All members starting with x- are aliases for the same key
+# without x- in the @limits object. This is not a stable
+# interface and may be removed or changed incompatibly in
+# the future. Use @limits for a supported stable
+# interface.
+#
# Since: 2.11
##
{ 'struct': 'ThrottleGroupProperties',
'data': { '*limits': 'ThrottleLimits',
- '*x-iops-total' : 'int', '*x-iops-total-max' : 'int',
- '*x-iops-total-max-length' : 'int', '*x-iops-read' : 'int',
- '*x-iops-read-max' : 'int', '*x-iops-read-max-length' : 'int',
- '*x-iops-write' : 'int', '*x-iops-write-max' : 'int',
- '*x-iops-write-max-length' : 'int', '*x-bps-total' : 'int',
- '*x-bps-total-max' : 'int', '*x-bps-total-max-length' : 'int',
- '*x-bps-read' : 'int', '*x-bps-read-max' : 'int',
- '*x-bps-read-max-length' : 'int', '*x-bps-write' : 'int',
- '*x-bps-write-max' : 'int', '*x-bps-write-max-length' : 'int',
- '*x-iops-size' : 'int' } }
+ '*x-iops-total': { 'type': 'int',
+ 'features': [ 'unstable' ] },
+ '*x-iops-total-max': { 'type': 'int',
+ 'features': [ 'unstable' ] },
+ '*x-iops-total-max-length': { 'type': 'int',
+ 'features': [ 'unstable' ] },
+ '*x-iops-read': { 'type': 'int',
+ 'features': [ 'unstable' ] },
+ '*x-iops-read-max': { 'type': 'int',
+ 'features': [ 'unstable' ] },
+ '*x-iops-read-max-length': { 'type': 'int',
+ 'features': [ 'unstable' ] },
+ '*x-iops-write': { 'type': 'int',
+ 'features': [ 'unstable' ] },
+ '*x-iops-write-max': { 'type': 'int',
+ 'features': [ 'unstable' ] },
+ '*x-iops-write-max-length': { 'type': 'int',
+ 'features': [ 'unstable' ] },
+ '*x-bps-total': { 'type': 'int',
+ 'features': [ 'unstable' ] },
+ '*x-bps-total-max': { 'type': 'int',
+ 'features': [ 'unstable' ] },
+ '*x-bps-total-max-length': { 'type': 'int',
+ 'features': [ 'unstable' ] },
+ '*x-bps-read': { 'type': 'int',
+ 'features': [ 'unstable' ] },
+ '*x-bps-read-max': { 'type': 'int',
+ 'features': [ 'unstable' ] },
+ '*x-bps-read-max-length': { 'type': 'int',
+ 'features': [ 'unstable' ] },
+ '*x-bps-write': { 'type': 'int',
+ 'features': [ 'unstable' ] },
+ '*x-bps-write-max': { 'type': 'int',
+ 'features': [ 'unstable' ] },
+ '*x-bps-write-max-length': { 'type': 'int',
+ 'features': [ 'unstable' ] },
+ '*x-iops-size': { 'type': 'int',
+ 'features': [ 'unstable' ] } } }
##
# @block-stream:
@@ -2916,6 +2959,7 @@
# read-only when the last writer is detached. This
# allows giving QEMU write permissions only on demand
# when an operation actually needs write access.
+# @unstable: Member x-check-cache-dropped is meant for debugging.
#
# Since: 2.9
##
@@ -2926,7 +2970,8 @@
'*aio': 'BlockdevAioOptions',
'*drop-cache': {'type': 'bool',
'if': 'CONFIG_LINUX'},
- '*x-check-cache-dropped': 'bool' },
+ '*x-check-cache-dropped': { 'type': 'bool',
+ 'features': [ 'unstable' ] } },
'features': [ { 'name': 'dynamic-auto-read-only',
'if': 'CONFIG_POSIX' } ] }
@@ -4041,13 +4086,16 @@
# future requests before a successful reconnect will
# immediately fail. Default 0 (Since 4.2)
#
+# Features:
+# @unstable: Member @x-dirty-bitmap is experimental.
+#
# Since: 2.9
##
{ 'struct': 'BlockdevOptionsNbd',
'data': { 'server': 'SocketAddress',
'*export': 'str',
'*tls-creds': 'str',
- '*x-dirty-bitmap': 'str',
+ '*x-dirty-bitmap': { 'type': 'str', 'features': [ 'unstable' ] },
'*reconnect-delay': 'uint32' } }
##
@@ -4865,13 +4913,17 @@
# and replacement of an active keyslot
# (possible loss of data if IO error happens)
#
+# Features:
+# @unstable: This command is experimental.
+#
# Since: 5.1
##
{ 'command': 'x-blockdev-amend',
'data': { 'job-id': 'str',
'node-name': 'str',
'options': 'BlockdevAmendOptions',
- '*force': 'bool' } }
+ '*force': 'bool' },
+ 'features': [ 'unstable' ] }
##
# @BlockErrorAction:
@@ -5242,16 +5294,18 @@
#
# @node: the name of the node that will be added.
#
-# Note: this command is experimental, and its API is not stable. It
-# does not support all kinds of operations, all kinds of children, nor
-# all block drivers.
+# Features:
+# @unstable: This command is experimental, and its API is not stable. It
+# does not support all kinds of operations, all kinds of
+# children, nor all block drivers.
#
-# FIXME Removing children from a quorum node means introducing gaps in the
-# child indices. This cannot be represented in the 'children' list of
-# BlockdevOptionsQuorum, as returned by .bdrv_refresh_filename().
+# FIXME Removing children from a quorum node means introducing
+# gaps in the child indices. This cannot be represented in the
+# 'children' list of BlockdevOptionsQuorum, as returned by
+# .bdrv_refresh_filename().
#
-# Warning: The data in a new quorum child MUST be consistent with that of
-# the rest of the array.
+# Warning: The data in a new quorum child MUST be consistent
+# with that of the rest of the array.
#
# Since: 2.7
#
@@ -5280,7 +5334,8 @@
{ 'command': 'x-blockdev-change',
'data' : { 'parent': 'str',
'*child': 'str',
- '*node': 'str' } }
+ '*node': 'str' },
+ 'features': [ 'unstable' ] }
##
# @x-blockdev-set-iothread:
@@ -5297,8 +5352,9 @@
# @force: true if the node and its children should be moved when a BlockBackend
# is already attached
#
-# Note: this command is experimental and intended for test cases that need
-# control over IOThreads only.
+# Features:
+# @unstable: This command is experimental and intended for test cases that
+# need control over IOThreads only.
#
# Since: 2.12
#
@@ -5320,7 +5376,8 @@
{ 'command': 'x-blockdev-set-iothread',
'data' : { 'node-name': 'str',
'iothread': 'StrOrNull',
- '*force': 'bool' } }
+ '*force': 'bool' },
+ 'features': [ 'unstable' ] }
##
# @QuorumOpType:
diff --git a/qapi/migration.json b/qapi/migration.json
index 88f07baedd..9aa8bc5759 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -452,14 +452,20 @@
# procedure starts. The VM RAM is saved with running VM.
# (since 6.0)
#
+# Features:
+# @unstable: Members @x-colo and @x-ignore-shared are experimental.
+#
# Since: 1.2
##
{ 'enum': 'MigrationCapability',
'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks',
- 'compress', 'events', 'postcopy-ram', 'x-colo', 'release-ram',
+ 'compress', 'events', 'postcopy-ram',
+ { 'name': 'x-colo', 'features': [ 'unstable' ] },
+ 'release-ram',
'block', 'return-path', 'pause-before-switchover', 'multifd',
'dirty-bitmaps', 'postcopy-blocktime', 'late-block-activate',
- 'x-ignore-shared', 'validate-uuid', 'background-snapshot'] }
+ { 'name': 'x-ignore-shared', 'features': [ 'unstable' ] },
+ 'validate-uuid', 'background-snapshot'] }
##
# @MigrationCapabilityStatus:
@@ -743,6 +749,9 @@
# block device name if there is one, and to their node name
# otherwise. (Since 5.2)
#
+# Features:
+# @unstable: Member @x-checkpoint-delay is experimental.
+#
# Since: 2.4
##
{ 'enum': 'MigrationParameter',
@@ -753,7 +762,9 @@
'cpu-throttle-initial', 'cpu-throttle-increment',
'cpu-throttle-tailslow',
'tls-creds', 'tls-hostname', 'tls-authz', 'max-bandwidth',
- 'downtime-limit', 'x-checkpoint-delay', 'block-incremental',
+ 'downtime-limit',
+ { 'name': 'x-checkpoint-delay', 'features': [ 'unstable' ] },
+ 'block-incremental',
'multifd-channels',
'xbzrle-cache-size', 'max-postcopy-bandwidth',
'max-cpu-throttle', 'multifd-compression',
@@ -903,6 +914,9 @@
# block device name if there is one, and to their node name
# otherwise. (Since 5.2)
#
+# Features:
+# @unstable: Member @x-checkpoint-delay is experimental.
+#
# Since: 2.4
##
# TODO either fuse back into MigrationParameters, or make
@@ -925,7 +939,8 @@
'*tls-authz': 'StrOrNull',
'*max-bandwidth': 'size',
'*downtime-limit': 'uint64',
- '*x-checkpoint-delay': 'uint32',
+ '*x-checkpoint-delay': { 'type': 'uint32',
+ 'features': [ 'unstable' ] },
'*block-incremental': 'bool',
'*multifd-channels': 'uint8',
'*xbzrle-cache-size': 'size',
@@ -1099,6 +1114,9 @@
# block device name if there is one, and to their node name
# otherwise. (Since 5.2)
#
+# Features:
+# @unstable: Member @x-checkpoint-delay is experimental.
+#
# Since: 2.4
##
{ 'struct': 'MigrationParameters',
@@ -1119,7 +1137,8 @@
'*tls-authz': 'str',
'*max-bandwidth': 'size',
'*downtime-limit': 'uint64',
- '*x-checkpoint-delay': 'uint32',
+ '*x-checkpoint-delay': { 'type': 'uint32',
+ 'features': [ 'unstable' ] },
'*block-incremental': 'bool',
'*multifd-channels': 'uint8',
'*xbzrle-cache-size': 'size',
@@ -1351,6 +1370,9 @@
# If sent to the Secondary, the Secondary side will run failover work,
# then takes over server operation to become the service VM.
#
+# Features:
+# @unstable: This command is experimental.
+#
# Since: 2.8
#
# Example:
@@ -1359,7 +1381,8 @@
# <- { "return": {} }
#
##
-{ 'command': 'x-colo-lost-heartbeat' }
+{ 'command': 'x-colo-lost-heartbeat',
+ 'features': [ 'unstable' ] }
##
# @migrate_cancel:
diff --git a/qapi/misc.json b/qapi/misc.json
index 5c2ca3b556..358548abe1 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -185,6 +185,9 @@
# available during the preconfig state (i.e. when the --preconfig command
# line option was in use).
#
+# Features:
+# @unstable: This command is experimental.
+#
# Since 3.0
#
# Returns: nothing
@@ -195,7 +198,8 @@
# <- { "return": {} }
#
##
-{ 'command': 'x-exit-preconfig', 'allow-preconfig': true }
+{ 'command': 'x-exit-preconfig', 'allow-preconfig': true,
+ 'features': [ 'unstable' ] }
##
# @human-monitor-command:
diff --git a/qapi/qom.json b/qapi/qom.json
index 7231ac3f34..ccd1167808 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -559,10 +559,8 @@
# for ramblock-id. Disable this for 4.0
# machine types or older to allow
# migration with newer QEMU versions.
-# This option is considered stable
-# despite the x- prefix. (default:
-# false generally, but true for machine
-# types <= 4.0)
+# (default: false generally,
+# but true for machine types <= 4.0)
#
# Note: prealloc=true and reserve=false cannot be set at the same time. With
# reserve=true, the behavior depends on the operating system: for example,
@@ -785,6 +783,9 @@
##
# @ObjectType:
#
+# Features:
+# @unstable: Member @x-remote-object is experimental.
+#
# Since: 6.0
##
{ 'enum': 'ObjectType',
@@ -836,7 +837,7 @@
'tls-creds-psk',
'tls-creds-x509',
'tls-cipher-suites',
- 'x-remote-object'
+ { 'name': 'x-remote-object', 'features': [ 'unstable' ] }
] }
##
--
2.31.1
Markus Armbruster <armbru@redhat.com> wrote: > Add special feature 'unstable' everywhere the name starts with 'x-', > except for InputBarrierProperties member x-origin and > MemoryBackendProperties member x-use-canonical-path-for-ramblock-id, > because these two are actually stable. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com>
On Mon, Oct 25, 2021 at 07:25:25AM +0200, Markus Armbruster wrote:
> Add special feature 'unstable' everywhere the name starts with 'x-',
> except for InputBarrierProperties member x-origin and
> MemoryBackendProperties member x-use-canonical-path-for-ramblock-id,
> because these two are actually stable.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> @@ -2495,27 +2508,57 @@
> #
> # Properties for throttle-group objects.
> #
> -# The options starting with x- are aliases for the same key without x- in
> -# the @limits object. As indicated by the x- prefix, this is not a stable
> -# interface and may be removed or changed incompatibly in the future. Use
> -# @limits for a supported stable interface.
> -#
> # @limits: limits to apply for this throttle group
> #
> +# Features:
> +# @unstable: All members starting with x- are aliases for the same key
> +# without x- in the @limits object. This is not a stable
> +# interface and may be removed or changed incompatibly in
> +# the future. Use @limits for a supported stable
> +# interface.
> +#
> # Since: 2.11
> ##
> { 'struct': 'ThrottleGroupProperties',
> 'data': { '*limits': 'ThrottleLimits',
> - '*x-iops-total' : 'int', '*x-iops-total-max' : 'int',
> + '*x-iops-total': { 'type': 'int',
> + 'features': [ 'unstable' ] },
This struct has been around since 381bd74 (v6.0); but was not listed
as deprecated at the time. Do we still need it in 6.2, or have we
gone enough release cycles with the saner naming without x- that we
could drop this? But that is a question independent of this patch.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Am 29.10.2021 um 15:07 hat Eric Blake geschrieben:
> On Mon, Oct 25, 2021 at 07:25:25AM +0200, Markus Armbruster wrote:
> > Add special feature 'unstable' everywhere the name starts with 'x-',
> > except for InputBarrierProperties member x-origin and
> > MemoryBackendProperties member x-use-canonical-path-for-ramblock-id,
> > because these two are actually stable.
> >
> > Signed-off-by: Markus Armbruster <armbru@redhat.com>
> > ---
> > @@ -2495,27 +2508,57 @@
> > #
> > # Properties for throttle-group objects.
> > #
> > -# The options starting with x- are aliases for the same key without x- in
> > -# the @limits object. As indicated by the x- prefix, this is not a stable
> > -# interface and may be removed or changed incompatibly in the future. Use
> > -# @limits for a supported stable interface.
> > -#
> > # @limits: limits to apply for this throttle group
> > #
> > +# Features:
> > +# @unstable: All members starting with x- are aliases for the same key
> > +# without x- in the @limits object. This is not a stable
> > +# interface and may be removed or changed incompatibly in
> > +# the future. Use @limits for a supported stable
> > +# interface.
> > +#
> > # Since: 2.11
> > ##
> > { 'struct': 'ThrottleGroupProperties',
> > 'data': { '*limits': 'ThrottleLimits',
> > - '*x-iops-total' : 'int', '*x-iops-total-max' : 'int',
>
> > + '*x-iops-total': { 'type': 'int',
> > + 'features': [ 'unstable' ] },
>
> This struct has been around since 381bd74 (v6.0); but was not listed
> as deprecated at the time. Do we still need it in 6.2, or have we
> gone enough release cycles with the saner naming without x- that we
> could drop this? But that is a question independent of this patch.
There is no reason any more to use the x- options, and I think libvirt
never used them anyway.
I actually have a commit in my QAPI object branch that removes these
properties, but I think it still broke some tests.
Anyway, something for a separate patch.
Kevin
On Mon, Oct 25, 2021 at 1:25 AM Markus Armbruster <armbru@redhat.com> wrote:
> Add special feature 'unstable' everywhere the name starts with 'x-',
> except for InputBarrierProperties member x-origin and
> MemoryBackendProperties member x-use-canonical-path-for-ramblock-id,
> because these two are actually stable.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> qapi/block-core.json | 123 +++++++++++++++++++++++++++++++------------
> qapi/migration.json | 35 +++++++++---
> qapi/misc.json | 6 ++-
> qapi/qom.json | 11 ++--
> 4 files changed, 130 insertions(+), 45 deletions(-)
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 6d3217abb6..ce2c1352cb 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1438,6 +1438,9 @@
> #
> # @x-perf: Performance options. (Since 6.0)
> #
> +# Features:
> +# @unstable: Member @x-perf is experimental.
> +#
>
It'd be a lot cooler if we could annotate the unstable member directly
instead of confusing it with the syntax that might describe the entire
struct/union/command/etc, but ... eh, it's just a doc field, so I'm not
gonna press on this. I don't have the energy to get into a doc formatting
standard discussion right now, so: sure, why not?
> # Note: @on-source-error and @on-target-error only affect background
> # I/O. If an error occurs during a guest write request, the
> device's
> # rerror/werror actions will be used.
> @@ -1452,7 +1455,9 @@
> '*on-source-error': 'BlockdevOnError',
> '*on-target-error': 'BlockdevOnError',
> '*auto-finalize': 'bool', '*auto-dismiss': 'bool',
> - '*filter-node-name': 'str', '*x-perf': 'BackupPerf' } }
> + '*filter-node-name': 'str',
> + '*x-perf': { 'type': 'BackupPerf',
> + 'features': [ 'unstable' ] } } }
>
> ##
> # @DriveBackup:
> @@ -1916,9 +1921,13 @@
> #
> # Get the block graph.
> #
> +# Features:
> +# @unstable: This command is meant for debugging.
> +#
> # Since: 4.0
> ##
> -{ 'command': 'x-debug-query-block-graph', 'returns': 'XDbgBlockGraph' }
> +{ 'command': 'x-debug-query-block-graph', 'returns': 'XDbgBlockGraph',
> + 'features': [ 'unstable' ] }
>
> ##
> # @drive-mirror:
> @@ -2257,6 +2266,9 @@
> #
> # Get bitmap SHA256.
> #
> +# Features:
> +# @unstable: This command is meant for debugging.
> +#
> # Returns: - BlockDirtyBitmapSha256 on success
> # - If @node is not a valid block device, DeviceNotFound
> # - If @name is not found or if hashing has failed, GenericError
> with an
> @@ -2265,7 +2277,8 @@
> # Since: 2.10
> ##
> { 'command': 'x-debug-block-dirty-bitmap-sha256',
> - 'data': 'BlockDirtyBitmap', 'returns': 'BlockDirtyBitmapSha256' }
> + 'data': 'BlockDirtyBitmap', 'returns': 'BlockDirtyBitmapSha256',
> + 'features': [ 'unstable' ] }
>
> ##
> # @blockdev-mirror:
> @@ -2495,27 +2508,57 @@
> #
> # Properties for throttle-group objects.
> #
> -# The options starting with x- are aliases for the same key without x- in
> -# the @limits object. As indicated by the x- prefix, this is not a stable
> -# interface and may be removed or changed incompatibly in the future. Use
> -# @limits for a supported stable interface.
> -#
> # @limits: limits to apply for this throttle group
> #
> +# Features:
> +# @unstable: All members starting with x- are aliases for the same key
> +# without x- in the @limits object. This is not a stable
> +# interface and may be removed or changed incompatibly in
> +# the future. Use @limits for a supported stable
> +# interface.
> +#
> # Since: 2.11
> ##
> { 'struct': 'ThrottleGroupProperties',
> 'data': { '*limits': 'ThrottleLimits',
> - '*x-iops-total' : 'int', '*x-iops-total-max' : 'int',
> - '*x-iops-total-max-length' : 'int', '*x-iops-read' : 'int',
> - '*x-iops-read-max' : 'int', '*x-iops-read-max-length' : 'int',
> - '*x-iops-write' : 'int', '*x-iops-write-max' : 'int',
> - '*x-iops-write-max-length' : 'int', '*x-bps-total' : 'int',
> - '*x-bps-total-max' : 'int', '*x-bps-total-max-length' : 'int',
> - '*x-bps-read' : 'int', '*x-bps-read-max' : 'int',
> - '*x-bps-read-max-length' : 'int', '*x-bps-write' : 'int',
> - '*x-bps-write-max' : 'int', '*x-bps-write-max-length' : 'int',
> - '*x-iops-size' : 'int' } }
> + '*x-iops-total': { 'type': 'int',
> + 'features': [ 'unstable' ] },
> + '*x-iops-total-max': { 'type': 'int',
> + 'features': [ 'unstable' ] },
> + '*x-iops-total-max-length': { 'type': 'int',
> + 'features': [ 'unstable' ] },
> + '*x-iops-read': { 'type': 'int',
> + 'features': [ 'unstable' ] },
> + '*x-iops-read-max': { 'type': 'int',
> + 'features': [ 'unstable' ] },
> + '*x-iops-read-max-length': { 'type': 'int',
> + 'features': [ 'unstable' ] },
> + '*x-iops-write': { 'type': 'int',
> + 'features': [ 'unstable' ] },
> + '*x-iops-write-max': { 'type': 'int',
> + 'features': [ 'unstable' ] },
> + '*x-iops-write-max-length': { 'type': 'int',
> + 'features': [ 'unstable' ] },
> + '*x-bps-total': { 'type': 'int',
> + 'features': [ 'unstable' ] },
> + '*x-bps-total-max': { 'type': 'int',
> + 'features': [ 'unstable' ] },
> + '*x-bps-total-max-length': { 'type': 'int',
> + 'features': [ 'unstable' ] },
> + '*x-bps-read': { 'type': 'int',
> + 'features': [ 'unstable' ] },
> + '*x-bps-read-max': { 'type': 'int',
> + 'features': [ 'unstable' ] },
> + '*x-bps-read-max-length': { 'type': 'int',
> + 'features': [ 'unstable' ] },
> + '*x-bps-write': { 'type': 'int',
> + 'features': [ 'unstable' ] },
> + '*x-bps-write-max': { 'type': 'int',
> + 'features': [ 'unstable' ] },
> + '*x-bps-write-max-length': { 'type': 'int',
> + 'features': [ 'unstable' ] },
> + '*x-iops-size': { 'type': 'int',
> + 'features': [ 'unstable' ] } } }
>
> ##
> # @block-stream:
> @@ -2916,6 +2959,7 @@
> # read-only when the last writer is detached.
> This
> # allows giving QEMU write permissions only on
> demand
> # when an operation actually needs write access.
> +# @unstable: Member x-check-cache-dropped is meant for debugging.
> #
> # Since: 2.9
> ##
> @@ -2926,7 +2970,8 @@
> '*aio': 'BlockdevAioOptions',
> '*drop-cache': {'type': 'bool',
> 'if': 'CONFIG_LINUX'},
> - '*x-check-cache-dropped': 'bool' },
> + '*x-check-cache-dropped': { 'type': 'bool',
> + 'features': [ 'unstable' ] } },
> 'features': [ { 'name': 'dynamic-auto-read-only',
> 'if': 'CONFIG_POSIX' } ] }
>
> @@ -4041,13 +4086,16 @@
> # future requests before a successful reconnect will
> # immediately fail. Default 0 (Since 4.2)
> #
> +# Features:
> +# @unstable: Member @x-dirty-bitmap is experimental.
> +#
> # Since: 2.9
> ##
> { 'struct': 'BlockdevOptionsNbd',
> 'data': { 'server': 'SocketAddress',
> '*export': 'str',
> '*tls-creds': 'str',
> - '*x-dirty-bitmap': 'str',
> + '*x-dirty-bitmap': { 'type': 'str', 'features': [ 'unstable'
> ] },
> '*reconnect-delay': 'uint32' } }
>
> ##
> @@ -4865,13 +4913,17 @@
> # and replacement of an active keyslot
> # (possible loss of data if IO error happens)
> #
> +# Features:
> +# @unstable: This command is experimental.
> +#
> # Since: 5.1
> ##
> { 'command': 'x-blockdev-amend',
> 'data': { 'job-id': 'str',
> 'node-name': 'str',
> 'options': 'BlockdevAmendOptions',
> - '*force': 'bool' } }
> + '*force': 'bool' },
> + 'features': [ 'unstable' ] }
>
> ##
> # @BlockErrorAction:
> @@ -5242,16 +5294,18 @@
> #
> # @node: the name of the node that will be added.
> #
> -# Note: this command is experimental, and its API is not stable. It
> -# does not support all kinds of operations, all kinds of children,
> nor
> -# all block drivers.
> +# Features:
> +# @unstable: This command is experimental, and its API is not stable. It
> +# does not support all kinds of operations, all kinds of
> +# children, nor all block drivers.
> #
> -# FIXME Removing children from a quorum node means introducing gaps
> in the
> -# child indices. This cannot be represented in the 'children' list
> of
> -# BlockdevOptionsQuorum, as returned by .bdrv_refresh_filename().
> +# FIXME Removing children from a quorum node means introducing
> +# gaps in the child indices. This cannot be represented in the
> +# 'children' list of BlockdevOptionsQuorum, as returned by
> +# .bdrv_refresh_filename().
> #
> -# Warning: The data in a new quorum child MUST be consistent with
> that of
> -# the rest of the array.
> +# Warning: The data in a new quorum child MUST be consistent
> +# with that of the rest of the array.
> #
> # Since: 2.7
> #
> @@ -5280,7 +5334,8 @@
> { 'command': 'x-blockdev-change',
> 'data' : { 'parent': 'str',
> '*child': 'str',
> - '*node': 'str' } }
> + '*node': 'str' },
> + 'features': [ 'unstable' ] }
>
> ##
> # @x-blockdev-set-iothread:
> @@ -5297,8 +5352,9 @@
> # @force: true if the node and its children should be moved when a
> BlockBackend
> # is already attached
> #
> -# Note: this command is experimental and intended for test cases that need
> -# control over IOThreads only.
> +# Features:
> +# @unstable: This command is experimental and intended for test cases that
> +# need control over IOThreads only.
> #
> # Since: 2.12
> #
> @@ -5320,7 +5376,8 @@
> { 'command': 'x-blockdev-set-iothread',
> 'data' : { 'node-name': 'str',
> 'iothread': 'StrOrNull',
> - '*force': 'bool' } }
> + '*force': 'bool' },
> + 'features': [ 'unstable' ] }
>
> ##
> # @QuorumOpType:
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 88f07baedd..9aa8bc5759 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -452,14 +452,20 @@
> # procedure starts. The VM RAM is saved with
> running VM.
> # (since 6.0)
> #
> +# Features:
> +# @unstable: Members @x-colo and @x-ignore-shared are experimental.
> +#
> # Since: 1.2
> ##
> { 'enum': 'MigrationCapability',
> 'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks',
> - 'compress', 'events', 'postcopy-ram', 'x-colo', 'release-ram',
> + 'compress', 'events', 'postcopy-ram',
> + { 'name': 'x-colo', 'features': [ 'unstable' ] },
> + 'release-ram',
> 'block', 'return-path', 'pause-before-switchover', 'multifd',
> 'dirty-bitmaps', 'postcopy-blocktime', 'late-block-activate',
> - 'x-ignore-shared', 'validate-uuid', 'background-snapshot'] }
> + { 'name': 'x-ignore-shared', 'features': [ 'unstable' ] },
> + 'validate-uuid', 'background-snapshot'] }
>
> ##
> # @MigrationCapabilityStatus:
> @@ -743,6 +749,9 @@
> # block device name if there is one, and to their
> node name
> # otherwise. (Since 5.2)
> #
> +# Features:
> +# @unstable: Member @x-checkpoint-delay is experimental.
> +#
> # Since: 2.4
> ##
> { 'enum': 'MigrationParameter',
> @@ -753,7 +762,9 @@
> 'cpu-throttle-initial', 'cpu-throttle-increment',
> 'cpu-throttle-tailslow',
> 'tls-creds', 'tls-hostname', 'tls-authz', 'max-bandwidth',
> - 'downtime-limit', 'x-checkpoint-delay', 'block-incremental',
> + 'downtime-limit',
> + { 'name': 'x-checkpoint-delay', 'features': [ 'unstable' ] },
> + 'block-incremental',
> 'multifd-channels',
> 'xbzrle-cache-size', 'max-postcopy-bandwidth',
> 'max-cpu-throttle', 'multifd-compression',
> @@ -903,6 +914,9 @@
> # block device name if there is one, and to their
> node name
> # otherwise. (Since 5.2)
> #
> +# Features:
> +# @unstable: Member @x-checkpoint-delay is experimental.
> +#
> # Since: 2.4
> ##
> # TODO either fuse back into MigrationParameters, or make
> @@ -925,7 +939,8 @@
> '*tls-authz': 'StrOrNull',
> '*max-bandwidth': 'size',
> '*downtime-limit': 'uint64',
> - '*x-checkpoint-delay': 'uint32',
> + '*x-checkpoint-delay': { 'type': 'uint32',
> + 'features': [ 'unstable' ] },
> '*block-incremental': 'bool',
> '*multifd-channels': 'uint8',
> '*xbzrle-cache-size': 'size',
> @@ -1099,6 +1114,9 @@
> # block device name if there is one, and to their
> node name
> # otherwise. (Since 5.2)
> #
> +# Features:
> +# @unstable: Member @x-checkpoint-delay is experimental.
> +#
> # Since: 2.4
> ##
> { 'struct': 'MigrationParameters',
> @@ -1119,7 +1137,8 @@
> '*tls-authz': 'str',
> '*max-bandwidth': 'size',
> '*downtime-limit': 'uint64',
> - '*x-checkpoint-delay': 'uint32',
> + '*x-checkpoint-delay': { 'type': 'uint32',
> + 'features': [ 'unstable' ] },
> '*block-incremental': 'bool',
> '*multifd-channels': 'uint8',
> '*xbzrle-cache-size': 'size',
> @@ -1351,6 +1370,9 @@
> # If sent to the Secondary, the Secondary side will run failover work,
> # then takes over server operation to become the service VM.
> #
> +# Features:
> +# @unstable: This command is experimental.
> +#
> # Since: 2.8
> #
> # Example:
> @@ -1359,7 +1381,8 @@
> # <- { "return": {} }
> #
> ##
> -{ 'command': 'x-colo-lost-heartbeat' }
> +{ 'command': 'x-colo-lost-heartbeat',
> + 'features': [ 'unstable' ] }
>
> ##
> # @migrate_cancel:
> diff --git a/qapi/misc.json b/qapi/misc.json
> index 5c2ca3b556..358548abe1 100644
> --- a/qapi/misc.json
> +++ b/qapi/misc.json
> @@ -185,6 +185,9 @@
> # available during the preconfig state (i.e. when the --preconfig command
> # line option was in use).
> #
> +# Features:
> +# @unstable: This command is experimental.
> +#
> # Since 3.0
> #
> # Returns: nothing
> @@ -195,7 +198,8 @@
> # <- { "return": {} }
> #
> ##
> -{ 'command': 'x-exit-preconfig', 'allow-preconfig': true }
> +{ 'command': 'x-exit-preconfig', 'allow-preconfig': true,
> + 'features': [ 'unstable' ] }
>
> ##
> # @human-monitor-command:
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 7231ac3f34..ccd1167808 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -559,10 +559,8 @@
> # for ramblock-id. Disable this
> for 4.0
> # machine types or older to allow
> # migration with newer QEMU
> versions.
> -# This option is considered stable
> -# despite the x- prefix. (default:
> -# false generally, but true for
> machine
> -# types <= 4.0)
> +# (default: false generally,
> +# but true for machine types <=
> 4.0)
> #
> # Note: prealloc=true and reserve=false cannot be set at the same time.
> With
> # reserve=true, the behavior depends on the operating system: for
> example,
> @@ -785,6 +783,9 @@
> ##
> # @ObjectType:
> #
> +# Features:
> +# @unstable: Member @x-remote-object is experimental.
> +#
> # Since: 6.0
> ##
> { 'enum': 'ObjectType',
> @@ -836,7 +837,7 @@
> 'tls-creds-psk',
> 'tls-creds-x509',
> 'tls-cipher-suites',
> - 'x-remote-object'
> + { 'name': 'x-remote-object', 'features': [ 'unstable' ] }
> ] }
>
> ##
> --
> 2.31.1
>
>
Seems OK, but I didn't audit for false positives/negatives. Trusting your
judgment here. (It looks like Phil started to audit this in his reply to
your previous commit, so I'll trust that.)
Acked-by: John Snow <jsnow@redhat.com>
John Snow <jsnow@redhat.com> writes:
> On Mon, Oct 25, 2021 at 1:25 AM Markus Armbruster <armbru@redhat.com> wrote:
>
>> Add special feature 'unstable' everywhere the name starts with 'x-',
>> except for InputBarrierProperties member x-origin and
>> MemoryBackendProperties member x-use-canonical-path-for-ramblock-id,
>> because these two are actually stable.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> qapi/block-core.json | 123 +++++++++++++++++++++++++++++++------------
>> qapi/migration.json | 35 +++++++++---
>> qapi/misc.json | 6 ++-
>> qapi/qom.json | 11 ++--
>> 4 files changed, 130 insertions(+), 45 deletions(-)
>>
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index 6d3217abb6..ce2c1352cb 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -1438,6 +1438,9 @@
>> #
>> # @x-perf: Performance options. (Since 6.0)
>> #
>> +# Features:
>> +# @unstable: Member @x-perf is experimental.
>> +#
>>
>
> It'd be a lot cooler if we could annotate the unstable member directly
> instead of confusing it with the syntax that might describe the entire
> struct/union/command/etc, but ... eh, it's just a doc field, so I'm not
> gonna press on this. I don't have the energy to get into a doc formatting
> standard discussion right now, so: sure, why not?
By design, we have a single doc comment block for the entire definition.
When Kevin invented feature flags (merge commit 4747524f9f2), he added
them just to struct types. He added "feature sections" for documenting
features. It mirrors the "argument sections" for documenting members.
Makes sense.
Aside: he neglected to update qapi-code-gen.rst section "Definition
documentation", and I failed to catch it. I'll make up for it.
Peter and I then added feature flags to the remaining definitions
(commit 23394b4c39 and 013b4efc9b). Feature sections make sense there,
too.
I then added them to struct members (commit 84ab008687). Instead of
doing something fancy for documenting feature flags of members, I simply
used the existing feature sections. This conflates member features with
struct features.
What could "something fancy" be? All we have for members is "argument
sections", which are basically a name plus descriptive text. We'd have
to add structure to that, say nest feature sections into argument
sections. I have no appetite for that right now.
>
>
>> # Note: @on-source-error and @on-target-error only affect background
>> # I/O. If an error occurs during a guest write request, the
>> device's
>> # rerror/werror actions will be used.
>> @@ -1452,7 +1455,9 @@
>> '*on-source-error': 'BlockdevOnError',
>> '*on-target-error': 'BlockdevOnError',
>> '*auto-finalize': 'bool', '*auto-dismiss': 'bool',
>> - '*filter-node-name': 'str', '*x-perf': 'BackupPerf' } }
>> + '*filter-node-name': 'str',
>> + '*x-perf': { 'type': 'BackupPerf',
>> + 'features': [ 'unstable' ] } } }
>>
>> ##
>> # @DriveBackup:
[...]
> Seems OK, but I didn't audit for false positives/negatives. Trusting your
> judgment here. (It looks like Phil started to audit this in his reply to
> your previous commit, so I'll trust that.)
I'm pretty sure the x- names that don't get feature 'unstable' are
actually stable; see my reply to Kashyap.
I did check git history for each that does get feature 'unstable'.
Double-checking is of course welcome.
> Acked-by: John Snow <jsnow@redhat.com>
Thanks!
On Tue, Oct 26, 2021 at 3:56 AM Markus Armbruster <armbru@redhat.com> wrote:
> John Snow <jsnow@redhat.com> writes:
>
> > On Mon, Oct 25, 2021 at 1:25 AM Markus Armbruster <armbru@redhat.com>
> wrote:
> >
> >> Add special feature 'unstable' everywhere the name starts with 'x-',
> >> except for InputBarrierProperties member x-origin and
> >> MemoryBackendProperties member x-use-canonical-path-for-ramblock-id,
> >> because these two are actually stable.
> >>
> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >> ---
> >> qapi/block-core.json | 123 +++++++++++++++++++++++++++++++------------
> >> qapi/migration.json | 35 +++++++++---
> >> qapi/misc.json | 6 ++-
> >> qapi/qom.json | 11 ++--
> >> 4 files changed, 130 insertions(+), 45 deletions(-)
> >>
> >> diff --git a/qapi/block-core.json b/qapi/block-core.json
> >> index 6d3217abb6..ce2c1352cb 100644
> >> --- a/qapi/block-core.json
> >> +++ b/qapi/block-core.json
> >> @@ -1438,6 +1438,9 @@
> >> #
> >> # @x-perf: Performance options. (Since 6.0)
> >> #
> >> +# Features:
> >> +# @unstable: Member @x-perf is experimental.
> >> +#
> >>
> >
> > It'd be a lot cooler if we could annotate the unstable member directly
> > instead of confusing it with the syntax that might describe the entire
> > struct/union/command/etc, but ... eh, it's just a doc field, so I'm not
> > gonna press on this. I don't have the energy to get into a doc formatting
> > standard discussion right now, so: sure, why not?
>
> By design, we have a single doc comment block for the entire definition.
>
> When Kevin invented feature flags (merge commit 4747524f9f2), he added
> them just to struct types. He added "feature sections" for documenting
> features. It mirrors the "argument sections" for documenting members.
> Makes sense.
>
> Aside: he neglected to update qapi-code-gen.rst section "Definition
> documentation", and I failed to catch it. I'll make up for it.
>
> Peter and I then added feature flags to the remaining definitions
> (commit 23394b4c39 and 013b4efc9b). Feature sections make sense there,
> too.
>
> I then added them to struct members (commit 84ab008687). Instead of
> doing something fancy for documenting feature flags of members, I simply
> used the existing feature sections. This conflates member features with
> struct features.
>
>
Yeah, that's the part I don't like. If this isn't the first instance of
breaking the seal, though, this is the wrong time for me to comment on it.
Don't worry about it for this series.
> What could "something fancy" be? All we have for members is "argument
> sections", which are basically a name plus descriptive text. We'd have
> to add structure to that, say nest feature sections into argument
> sections. I have no appetite for that right now.
>
>
(Tangent below, unrelated to acceptance of this series)
Yeah, I don't have an appetite for it at the moment either. I'll have to
read Marc-Andre's recent sphinx patches and see if I want to do more work
-- I had a bigger prototype a few months back I didn't bring all the way
home, but I am still thinking about reworking our QAPIDoc format. It's ...
well. I don't really want to, but I am not sure how else to bring some of
the features I want home, and I think I need some more time to think
carefully through exactly what I want to do and why.
I wouldn't mind chatting about it with you sometime soon -- there's a few
things to balance here, such as:
(1) Reworking the doc format would be an obnoxious amount of churn, ...
(2) ...but the Sphinx internals are really not meant to be used the way
we're using them right now, ...
(3) ...but I also don't want to write our QAPI docstrings in a way that
*targets* Sphinx. Not that I think we'll be dropping it any time soon, but
it still feels like the wrong idea to tie them so closely together.
I'm hoping there's an opportunity to make the parsing nicer with minimal
changes to the parsing and format, though. It just might require enforcing
a *pinch* more structure. I could see how I feel about per-field
annotations at that point. I consider them interesting for things like the
Python SDK where I may want to enable/disable warnings for using deprecated
stuff at the client-level. (e.g., let's say you're using Python SDK 6.2 to
talk to a 6.1 client. Nothing stops you from doing this, but some commands
will simply be rejected by QEMU as it won't know what you're talking about.
Using deprecated fields or commands to talk to an older client will produce
no visible warning from QEMU either, as it wasn't deprecated at that point
in time. Still, the client may wish to know that they're asking for future
trouble -- so it's a thought that client-side warnings have some play here.)
Ehhhhhhhhhhhhhhhh don't worry about it for now, I guess I'll plow forward a
bit with the Sphinx stuff I'm thinking of at some point Soon:tm: and worry
about where the immovable objects are when I get there. This is foot-based
landmine-detection, and it works 100% of the time.
>
> >
> >> # Note: @on-source-error and @on-target-error only affect background
> >> # I/O. If an error occurs during a guest write request, the
> >> device's
> >> # rerror/werror actions will be used.
> >> @@ -1452,7 +1455,9 @@
> >> '*on-source-error': 'BlockdevOnError',
> >> '*on-target-error': 'BlockdevOnError',
> >> '*auto-finalize': 'bool', '*auto-dismiss': 'bool',
> >> - '*filter-node-name': 'str', '*x-perf': 'BackupPerf' } }
> >> + '*filter-node-name': 'str',
> >> + '*x-perf': { 'type': 'BackupPerf',
> >> + 'features': [ 'unstable' ] } } }
> >>
> >> ##
> >> # @DriveBackup:
>
> [...]
>
> > Seems OK, but I didn't audit for false positives/negatives. Trusting your
> > judgment here. (It looks like Phil started to audit this in his reply to
> > your previous commit, so I'll trust that.)
>
> I'm pretty sure the x- names that don't get feature 'unstable' are
> actually stable; see my reply to Kashyap.
>
> I did check git history for each that does get feature 'unstable'.
> Double-checking is of course welcome.
>
>
Yeh, just explaining why it's not an R-B. I'm trying to be a bit better
about reviews by not forcing myself to do "all or nothing". I trust your
work, of course -- I just also did not double check it :)
I need to change the way in which I read and discuss code so that I can be
more responsive.
> > Acked-by: John Snow <jsnow@redhat.com>
>
> Thanks!
>
>
John Snow <jsnow@redhat.com> writes:
> On Tue, Oct 26, 2021 at 3:56 AM Markus Armbruster <armbru@redhat.com> wrote:
>
>> John Snow <jsnow@redhat.com> writes:
>>
>> > On Mon, Oct 25, 2021 at 1:25 AM Markus Armbruster <armbru@redhat.com>
>> wrote:
>> >
>> >> Add special feature 'unstable' everywhere the name starts with 'x-',
>> >> except for InputBarrierProperties member x-origin and
>> >> MemoryBackendProperties member x-use-canonical-path-for-ramblock-id,
>> >> because these two are actually stable.
>> >>
>> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> >> ---
>> >> qapi/block-core.json | 123 +++++++++++++++++++++++++++++++------------
>> >> qapi/migration.json | 35 +++++++++---
>> >> qapi/misc.json | 6 ++-
>> >> qapi/qom.json | 11 ++--
>> >> 4 files changed, 130 insertions(+), 45 deletions(-)
>> >>
>> >> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> >> index 6d3217abb6..ce2c1352cb 100644
>> >> --- a/qapi/block-core.json
>> >> +++ b/qapi/block-core.json
>> >> @@ -1438,6 +1438,9 @@
>> >> #
>> >> # @x-perf: Performance options. (Since 6.0)
>> >> #
>> >> +# Features:
>> >> +# @unstable: Member @x-perf is experimental.
>> >> +#
>> >>
>> >
>> > It'd be a lot cooler if we could annotate the unstable member directly
>> > instead of confusing it with the syntax that might describe the entire
>> > struct/union/command/etc, but ... eh, it's just a doc field, so I'm not
>> > gonna press on this. I don't have the energy to get into a doc formatting
>> > standard discussion right now, so: sure, why not?
>>
>> By design, we have a single doc comment block for the entire definition.
>>
>> When Kevin invented feature flags (merge commit 4747524f9f2), he added
>> them just to struct types. He added "feature sections" for documenting
>> features. It mirrors the "argument sections" for documenting members.
>> Makes sense.
>>
>> Aside: he neglected to update qapi-code-gen.rst section "Definition
>> documentation", and I failed to catch it. I'll make up for it.
>>
>> Peter and I then added feature flags to the remaining definitions
>> (commit 23394b4c39 and 013b4efc9b). Feature sections make sense there,
>> too.
>>
>> I then added them to struct members (commit 84ab008687). Instead of
>> doing something fancy for documenting feature flags of members, I simply
>> used the existing feature sections. This conflates member features with
>> struct features.
>>
>>
> Yeah, that's the part I don't like. If this isn't the first instance of
> breaking the seal, though, this is the wrong time for me to comment on it.
> Don't worry about it for this series.
Okay.
>> What could "something fancy" be? All we have for members is "argument
>> sections", which are basically a name plus descriptive text. We'd have
>> to add structure to that, say nest feature sections into argument
>> sections. I have no appetite for that right now.
>>
>>
> (Tangent below, unrelated to acceptance of this series)
>
> Yeah, I don't have an appetite for it at the moment either. I'll have to
> read Marc-Andre's recent sphinx patches and see if I want to do more work
> -- I had a bigger prototype a few months back I didn't bring all the way
> home, but I am still thinking about reworking our QAPIDoc format. It's ...
> well. I don't really want to, but I am not sure how else to bring some of
> the features I want home, and I think I need some more time to think
> carefully through exactly what I want to do and why.
>
> I wouldn't mind chatting about it with you sometime soon -- there's a few
> things to balance here, such as:
>
> (1) Reworking the doc format would be an obnoxious amount of churn, ...
Yes. But that need not be the end of the argument, it may be the
beginning of one.
> (2) ...but the Sphinx internals are really not meant to be used the way
> we're using them right now, ...
Happy to trust you on this one.
> (3) ...but I also don't want to write our QAPI docstrings in a way that
> *targets* Sphinx. Not that I think we'll be dropping it any time soon, but
> it still feels like the wrong idea to tie them so closely together.
Maybe. Depends on what we get for it.
> I'm hoping there's an opportunity to make the parsing nicer with minimal
> changes to the parsing and format, though. It just might require enforcing
> a *pinch* more structure. I could see how I feel about per-field
> annotations at that point.
The doc comment language is the result of a series of pragmatic
decisions that got us from semi-accidental comment conventions to where
we are now. Each of the decisions made sense at the time. The result
is messy to describe and to parse. It's limiting and hard to grow
further.
> I consider them interesting for things like the
> Python SDK where I may want to enable/disable warnings for using deprecated
> stuff at the client-level. (e.g., let's say you're using Python SDK 6.2 to
> talk to a 6.1 client. Nothing stops you from doing this, but some commands
> will simply be rejected by QEMU as it won't know what you're talking about.
> Using deprecated fields or commands to talk to an older client will produce
> no visible warning from QEMU either, as it wasn't deprecated at that point
> in time. Still, the client may wish to know that they're asking for future
> trouble -- so it's a thought that client-side warnings have some play here.)
>
> Ehhhhhhhhhhhhhhhh don't worry about it for now, I guess I'll plow forward a
> bit with the Sphinx stuff I'm thinking of at some point Soon:tm: and worry
> about where the immovable objects are when I get there. This is foot-based
> landmine-detection, and it works 100% of the time.
Soldier, hand me the binoculars before you enter the minefield. Good
luck!
Back to serious: I feel we need to wrap the typing hints work before we
start the next big QAPI schema language project.
>> >> # Note: @on-source-error and @on-target-error only affect background
>> >> # I/O. If an error occurs during a guest write request, the
>> >> device's
>> >> # rerror/werror actions will be used.
>> >> @@ -1452,7 +1455,9 @@
>> >> '*on-source-error': 'BlockdevOnError',
>> >> '*on-target-error': 'BlockdevOnError',
>> >> '*auto-finalize': 'bool', '*auto-dismiss': 'bool',
>> >> - '*filter-node-name': 'str', '*x-perf': 'BackupPerf' } }
>> >> + '*filter-node-name': 'str',
>> >> + '*x-perf': { 'type': 'BackupPerf',
>> >> + 'features': [ 'unstable' ] } } }
>> >>
>> >> ##
>> >> # @DriveBackup:
>>
>> [...]
>>
>> > Seems OK, but I didn't audit for false positives/negatives. Trusting your
>> > judgment here. (It looks like Phil started to audit this in his reply to
>> > your previous commit, so I'll trust that.)
>>
>> I'm pretty sure the x- names that don't get feature 'unstable' are
>> actually stable; see my reply to Kashyap.
>>
>> I did check git history for each that does get feature 'unstable'.
>> Double-checking is of course welcome.
>>
>>
> Yeh, just explaining why it's not an R-B. I'm trying to be a bit better
> about reviews by not forcing myself to do "all or nothing". I trust your
> work, of course -- I just also did not double check it :)
You stating the limits of your review is useful.
Me stating the limits of my own research is also useful :)
> I need to change the way in which I read and discuss code so that I can be
> more responsive.
>
>
>> > Acked-by: John Snow <jsnow@redhat.com>
>>
>> Thanks!
>>
>>
© 2016 - 2026 Red Hat, Inc.