Re: [PATCH] qapi/qom: make some QOM properties depend on the build settings
I dropped this on the floor. Sorry for the delay!
Stefano Garzarella writes:
> Some QOM properties are associated with ObjectTypes that already
> depend on CONFIG_* switches. So to avoid generating dead code,
> let's also make the definition of those properties dependent on
> the corresponding CONFIG_*.
>
> Suggested-by: Markus Armbruster
> Signed-off-by: Stefano Garzarella
> ---
> qapi/qom.json | 21 ++---
> 1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 38dde6d785..ae93313a60 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -222,7 +222,8 @@
> ##
> { 'struct': 'CanHostSocketcanProperties',
>'data': { 'if': 'str',
> -'canbus': 'str' } }
> +'canbus': 'str' },
> + 'if': 'CONFIG_LINUX' }
>
> ##
> # @ColoCompareProperties:
> @@ -305,7 +306,8 @@
> ##
> { 'struct': 'CryptodevVhostUserProperties',
>'base': 'CryptodevBackendProperties',
> - 'data': { 'chardev': 'str' } }
> + 'data': { 'chardev': 'str' },
> + 'if': 'CONFIG_VHOST_CRYPTO' }
>
> ##
> # @DBusVMStateProperties:
> @@ -514,7 +516,8 @@
>'data': { 'evdev': 'str',
> '*grab_all': 'bool',
> '*repeat': 'bool',
> -'*grab-toggle': 'GrabToggleKeys' } }
> +'*grab-toggle': 'GrabToggleKeys' },
> + 'if': 'CONFIG_LINUX' }
>
> ##
> # @EventLoopBaseProperties:
> @@ -719,7 +722,8 @@
>'base': 'MemoryBackendProperties',
>'data': { '*hugetlb': 'bool',
> '*hugetlbsize': 'size',
> -'*seal': 'bool' } }
> +'*seal': 'bool' },
> + 'if': 'CONFIG_LINUX' }
>
> ##
> # @MemoryBackendEpcProperties:
> @@ -736,7 +740,8 @@
> ##
> { 'struct': 'MemoryBackendEpcProperties',
>'base': 'MemoryBackendProperties',
> - 'data': {} }
> + 'data': {},
> + 'if': 'CONFIG_LINUX' }
>
> ##
> # @PrManagerHelperProperties:
> @@ -749,7 +754,8 @@
> # Since: 2.11
> ##
> { 'struct': 'PrManagerHelperProperties',
> - 'data': { 'path': 'str' } }
> + 'data': { 'path': 'str' },
> + 'if': 'CONFIG_LINUX' }
>
> ##
> # @QtestProperties:
> @@ -872,7 +878,8 @@
> ##
> { 'struct': 'RngRandomProperties',
>'base': 'RngProperties',
> - 'data': { '*filename': 'str' } }
> + 'data': { '*filename': 'str' },
> + 'if': 'CONFIG_POSIX' }
>
> ##
> # @SevGuestProperties:
Reviewed-by: Markus Armbruster
Squashing in
diff --git a/qapi/crypto.json b/qapi/crypto.json
index e102be337b..9b216cee8e 100644
--- a/qapi/crypto.json
+++ b/qapi/crypto.json
@@ -488,7 +488,8 @@
##
{ 'struct': 'SecretKeyringProperties',
'base': 'SecretCommonProperties',
- 'data': { 'serial': 'int32' } }
+ 'data': { 'serial': 'int32' },
+ 'if': 'CONFIG_SECRET_KEYRING' }
##
# @TlsCredsProperties:
Queued, thanks!
Re: [PATCH v3] target/s390x: filter deprecated properties based on model expansion type
Collin Walling writes:
> Currently, there is no way to execute the query-cpu-model-expansion
> command to retrieve a comprehenisve list of deprecated properties, as
> the result is dependent per-model. To enable this, the expansion output
> is modified as such:
>
> When reporting a "full" CPU model, show the *entire* list of deprecated
> properties regardless if they are supported on the model. A full
> expansion outputs all known CPU model properties anyway, so it makes
> sense to report all deprecated properties here too.
>
> This allows management apps to query a single model (e.g. host) to
> acquire the full list of deprecated properties.
>
> Additionally, when reporting a "static" CPU model, the command will
> only show deprecated properties that are a subset of the model's
> *enabled* properties. This is more accurate than how the query was
> handled before, which blindly reported deprecated properties that
> were never otherwise introduced for certain models.
>
> Acked-by: David Hildenbrand
> Suggested-by: Jiri Denemark
> Signed-off-by: Collin Walling
> ---
>
> Changelog:
>
> v3
> - Removed the 'note' and cleaned up documentation
> - Revised commit message
>
> v2
> - Changed commit message
> - Added documentation reflecting this change
> - Made code changes that more accurately filter the deprecated
> properties based on expansion type. This change makes it
> so that the deprecated-properties reported for a static model
> expansion are a subset of the model's properties instead of
> the model's full-definition properties.
>
> ---
> qapi/machine-target.json | 5 +++--
> target/s390x/cpu_models_sysemu.c | 16 +---
> 2 files changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
> index a8d9ec87f5..67086f006f 100644
> --- a/qapi/machine-target.json
> +++ b/qapi/machine-target.json
> @@ -21,8 +21,9 @@
> # @props: a dictionary of QOM properties to be applied
> #
> # @deprecated-props: a list of properties that are flagged as deprecated
> -# by the CPU vendor. These props are a subset of the full model's
> -# definition list of properties. (since 9.1)
> +# by the CPU vendor. These properties are either a subset of the
> +# properties enabled on the CPU model, or a set of properties
> +# deprecated across all models for the architecture.
When is it "a subset of the properties enabled on the CPU model", and
when is it "a set of properties deprecated across all models for the
architecture"?
My guess based on the commit message: it's the former when
query-cpu-model-expansion's type is "static", and the latter when it's
"full".
> #
> # Since: 2.8
> ##
> diff --git a/target/s390x/cpu_models_sysemu.c
> b/target/s390x/cpu_models_sysemu.c
> index 977fbc6522..e28ecf7ab9 100644
> --- a/target/s390x/cpu_models_sysemu.c
> +++ b/target/s390x/cpu_models_sysemu.c
> @@ -174,11 +174,15 @@ static void cpu_info_from_model(CpuModelInfo *info,
> const S390CPUModel *model,
> bool delta_changes)
> {
> QDict *qdict = qdict_new();
> -S390FeatBitmap bitmap;
> +S390FeatBitmap bitmap, deprecated;
>
> /* always fallback to the static base model */
> info->name = g_strdup_printf("%s-base", model->def->name);
>
> +/* features flagged as deprecated */
> +bitmap_zero(deprecated, S390_FEAT_MAX);
> +s390_get_deprecated_features(deprecated);
> +
> if (delta_changes) {
> /* features deleted from the base feature set */
> bitmap_andnot(bitmap, model->def->base_feat, model->features,
> @@ -193,6 +197,9 @@ static void cpu_info_from_model(CpuModelInfo *info, const
> S390CPUModel *model,
> if (!bitmap_empty(bitmap, S390_FEAT_MAX)) {
> s390_feat_bitmap_to_ascii(bitmap, qdict, qdict_add_enabled_feat);
> }
> +
> +/* deprecated features that are a subset of the model's enabled
> features */
Recommend to wrap this line for legibility.
> +bitmap_and(deprecated, deprecated, model->features, S390_FEAT_MAX);
> } else {
> /* expand all features */
> s390_feat_bitmap_to_ascii(model->features, qdict,
> @@ -207,12 +214,7 @@ static void cpu_info_from_model(CpuModelInfo *info,
> const S390CPUModel *model,
> info->props = QOBJECT(qdict);
> }
>
> -/* features flagged as deprecated */
> -bitmap_zero(bitmap, S390_FEAT_MAX);
> -s390_get_deprecated_features(bitmap);
> -
> -bitmap_and(bitmap, bitmap, model->def->full_feat, S390_FEAT_MAX);
> -s390_feat_bitmap_to_ascii(bitmap, >deprecated_props,
> list_add_feat);
> +s390_feat_bitmap_to_ascii(deprecated, >deprecated_props,
> list_add_feat);
> info->has_deprecated_props = !!info->deprecated_props;
> }
Re: [PATCH v6 4/4] docs: add test for firmware.json QAPI
Thomas Weißschuh writes:
> To make sure that the QAPI description stays valid, add a testcase.
>
> Suggested-by: Philippe Mathieu-Daudé
> Link:
> https://lore.kernel.org/qemu-devel/d9ce0234-4beb-4b90-b14c-76810d3b8...@linaro.org/
> Reviewed-by: Daniel P. Berrangé
> Signed-off-by: Thomas Weißschuh
> ---
> docs/meson.build | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/docs/meson.build b/docs/meson.build
> index 9040f860ae1a..bcca45a342a3 100644
> --- a/docs/meson.build
> +++ b/docs/meson.build
> @@ -99,3 +99,8 @@ if build_docs
>alias_target('html', sphinxdocs)
>alias_target('man', sphinxmans)
> endif
> +
> +test('QAPI firmware.json regression tests', python,
> + args: [qapi_gen.full_path(), '-o', meson.current_build_dir() / 'qapi',
> +meson.current_source_dir() / 'interop/firmware.json'],
> + env: test_env, suite: ['qapi-schema', 'qapi-interop'])
Acked-by: Markus Armbruster
Re: [PATCH v6 1/4] docs/interop/firmware.json: add new enum FirmwareFormat
Thomas Weißschuh writes: > Only a small subset of all blockdev drivers make sense for firmware > images. Introduce and use a new enum to represent this. > > This also reduces the dependency on firmware.json from the global qapi > definitions. > > Claim "Since: 3.0" for the new enum, because that's correct for its > members, and the members are what matters in the interface. > > Suggested-by: Daniel P. Berrangé > Reviewed-by: Daniel P. Berrangé > Signed-off-by: Thomas Weißschuh Reviewed-by: Markus Armbruster
Re: [PATCH v6 2/4] docs/interop/firmware.json: add new enum FirmwareArchitecture
Thomas Weißschuh writes:
> Only a small subset of all architectures supported by qemu make use of
> firmware files. Introduce and use a new enum to represent this.
>
> This also removes the dependency to machine.json from the global qapi
> definitions.
>
> Claim "Since: 3.0" for the new enum, because that's correct for most of
> its members, and the members are what matters in the interface.
>
> Suggested-by: Daniel P. Berrangé
> Reviewed-by: Daniel P. Berrangé
> Signed-off-by: Thomas Weißschuh
> ---
> docs/interop/firmware.json | 28 ++--
> 1 file changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json
> index d5d4c17f230b..ae4179994479 100644
> --- a/docs/interop/firmware.json
> +++ b/docs/interop/firmware.json
> @@ -14,7 +14,10 @@
> # = Firmware
> ##
>
> -{ 'include' : 'machine.json' }
> +{ 'pragma': {
> +'member-name-exceptions': [
> +'FirmwareArchitecture' # x86_64
> +] } }
>
> ##
> # @FirmwareOSInterface:
> @@ -59,6 +62,27 @@
> { 'enum' : 'FirmwareDevice',
>'data' : [ 'flash', 'kernel', 'memory' ] }
>
> +##
> +# @FirmwareArchitecture:
> +#
> +# Enumeration of architectures for which Qemu uses additional
QEMU
> +# firmware files.
> +#
> +# @aarch64: 64-bit Arm.
> +#
> +# @arm: 32-bit Arm.
> +#
> +# @i386: 32-bit x86.
> +#
> +# @loongarch64: 64-bit LoongArch. (since: 7.1)
> +#
> +# @x86_64: 64-bit x86.
> +#
> +# Since: 3.0
> +##
> +{ 'enum' : 'FirmwareArchitecture',
> + 'data' : [ 'aarch64', 'arm', 'i386', 'loongarch64', 'x86_64' ] }
> +
> ##
> # @FirmwareTarget:
> #
> @@ -80,7 +104,7 @@
> # Since: 3.0
> ##
> { 'struct' : 'FirmwareTarget',
> - 'data' : { 'architecture' : 'SysEmuTarget',
> + 'data' : { 'architecture' : 'FirmwareArchitecture',
> 'machines' : [ 'str' ] } }
>
> ##
Reviewed-by: Markus Armbruster
Re: [PATCH v2] target/s390x: filter deprecated properties based on model expansion type
Thomas Huth writes: > On 18/07/2024 20.22, Collin Walling wrote: >> On 7/18/24 9:39 AM, Markus Armbruster wrote: >>> Collin Walling writes: >>> >>>> As s390 CPU models progress and deprecated properties are dropped >>>> outright, it will be cumbersome for management apps to query the host >>>> for a comprehensive list of deprecated properties that will need to be >>>> disabled on older models. To remedy this, the query-cpu-model-expansion >>>> output now behaves by filtering deprecated properties based on the >>>> expansion type instead of filtering based off of the model's full set >>>> of features: >>>> >>>> When reporting a static CPU model, only show deprecated properties that >>>> are a subset of the model's enabled features. >>>> >>>> When reporting a full CPU model, show the entire list of deprecated >>>> properties regardless if they are supported on the model. >>>> >>>> Suggested-by: Jiri Denemark >>>> Signed-off-by: Collin Walling [...] >>>> diff --git a/qapi/machine-target.json b/qapi/machine-target.json >>>> index a8d9ec87f5..d151504f25 100644 >>>> --- a/qapi/machine-target.json >>>> +++ b/qapi/machine-target.json >>>> @@ -21,8 +21,12 @@ >>>> # @props: a dictionary of QOM properties to be applied >>>> # >>>> # @deprecated-props: a list of properties that are flagged as deprecated >>>> -# by the CPU vendor. These props are a subset of the full model's >>>> -# definition list of properties. (since 9.1) >>>> +# by the CPU vendor. (since 9.1). >>>> +# >>>> +# .. note:: Since 9.1, the list of deprecated props were always a subset >>>> +#of the model's full-definition list of properites. Now, this list is >>>> +#populated with the model's enabled property set when delta changes >>>> +#are applied. All deprecated properties are reported otherwise. >>> >>> I'm confused. >>> >>> "Since 9.1, the list of deprecated props were ..." and "Now, this list >>> is" sounds like you're explaining behavior before and after a change. >>> What change? Since only released behavior matters, and >>> @deprecated-props is new, there is no old behavior to document, isn't >>> it? >> >> I admittedly had some difficulty articulating the change introduced by >> this patch. The @deprecated-props array, as well as a way for s390x to >> populate it, was introduced in release 9.1. Prior to this patch, the >> deprecated-props list was filtered by the CPU model's full feature set. >> I attempted to explain this with: >> "Since 9.1, the list of deprecated props were always a subset of the >> model's full-definition list of properties." > > Version 9.1 has not been released yet (see > https://wiki.qemu.org/Planning/9.1), so I agree with Markus, this sounds > confusing/wrong to me, too. User-visible changes between releases need to be documented in release notes and the manual. Don't for changes within a release, instead update documentation to reflect the new state of things. Regardless, do explain the change in the commit message. Specifics are more helpful than such generalities, so let me try despite my relative ignorance of the subject matter. 1. Update the description of @deprecated-props to match the new code. Ask yourself what people need to know to use this interface. 2. Explain in the commit message how semantics of @deprecated-props change in this patch.
Re: [PATCH v5 3/3] qapi: introduce device-sync-config
Vladimir Sementsov-Ogievskiy writes: > On 18.07.24 11:27, Markus Armbruster wrote: >> Vladimir Sementsov-Ogievskiy writes: >> >>> Add command to sync config from vhost-user backend to the device. It >>> may be helpful when VHOST_USER_SLAVE_CONFIG_CHANGE_MSG failed or not >>> triggered interrupt to the guest or just not available (not supported >>> by vhost-user server). >>> >>> Command result is racy if allow it during migration. Let's allow the >>> sync only in RUNNING state. >> >> Is this still accurate? The runstate_is_running() check is gone in >> v4, the migration_is_running() check remains. > > Right, better to fix commit message like: > > Command result is racy if allow it during migration. Let's not allow it. Suggest "Let's not allow that." Thanks! >>> Signed-off-by: Vladimir Sementsov-Ogievskiy >> >> QAPI schema and QMP part: >> Signed-off-by: Markus Armbruster
Re: [PATCH v2] block-backend: per-device throttling of BLOCK_IO_ERROR reports
Kevin Wolf writes:
> Am 19.07.2024 um 06:54 hat Markus Armbruster geschrieben:
>> Kevin Wolf writes:
>>
>> > Am 09.01.2024 um 14:13 hat Vladimir Sementsov-Ogievskiy geschrieben:
>> >> From: Leonid Kaplan
>> >>
>> >> BLOCK_IO_ERROR events comes from guest, so we must throttle them.
>> >> We still want per-device throttling, so let's use device id as a key.
>> >>
>> >> Signed-off-by: Leonid Kaplan
>> >> Signed-off-by: Vladimir Sementsov-Ogievskiy
>> >> ---
>> >>
>> >> v2: add Note: to QAPI doc
>> >>
>> >> monitor/monitor.c| 10 ++
>> >> qapi/block-core.json | 2 ++
>> >> 2 files changed, 12 insertions(+)
>> >>
>> >> diff --git a/monitor/monitor.c b/monitor/monitor.c
>> >> index 01ede1babd..ad0243e9d7 100644
>> >> --- a/monitor/monitor.c
>> >> +++ b/monitor/monitor.c
>> >> @@ -309,6 +309,7 @@ int error_printf_unless_qmp(const char *fmt, ...)
>> >> static MonitorQAPIEventConf monitor_qapi_event_conf[QAPI_EVENT__MAX] = {
>> >> /* Limit guest-triggerable events to 1 per second */
>> >> [QAPI_EVENT_RTC_CHANGE]= { 1000 * SCALE_MS },
>> >> +[QAPI_EVENT_BLOCK_IO_ERROR]= { 1000 * SCALE_MS },
>> >> [QAPI_EVENT_WATCHDOG] = { 1000 * SCALE_MS },
>> >> [QAPI_EVENT_BALLOON_CHANGE]= { 1000 * SCALE_MS },
>> >> [QAPI_EVENT_QUORUM_REPORT_BAD] = { 1000 * SCALE_MS },
>> >> @@ -498,6 +499,10 @@ static unsigned int qapi_event_throttle_hash(const
>> >> void *key)
>> >> hash += g_str_hash(qdict_get_str(evstate->data, "qom-path"));
>> >> }
>> >>
>> >> +if (evstate->event == QAPI_EVENT_BLOCK_IO_ERROR) {
>> >> +hash += g_str_hash(qdict_get_str(evstate->data, "device"));
>> >> +}
>> >
>> > Using "device" only works with -drive, i.e. when the BlockBackend
>> > actually has a name. In modern configurations with a -blockdev
>> > referenced by -device, the BlockBackend doesn't have a name any more.
>> >
>> > Maybe we should be using the qdev id (or more generally, QOM path) here,
>> > but that's something the event doesn't even contain yet.
>>
>> Uh, does the event reliably identify the I/O error's node or not? If
>> not, then that's a serious design defect.
>>
>> There's @node-name. Several commands use "either @device or @node-name"
>> to identify a node. Is that sufficient here?
>
> Possibly. The QAPI event is sent by a device, not by the backend, and
> the commit message claims per-device throttling. That's what made me
> think that we should base it on the device id.
This is an argument for having the event point at the device. Events
that already point at the device carry a mandatory @qom-path, and some
also carry an optional qdev @id for backward compatibility.
As far as I can tell, not all devices implement this event. If that's
true, then documentation should spell it out.
> But it's true that the error does originate in the backend (and it's
> unlikely that two devices are attached to the same node anyway), so the
> node-name could be good enough if we don't have a BlockBackend name. We
> should claim per-block-node rather then per-device throttling then.
Yes.
Re: [PATCH v6 3/5] migration: Add migration parameters for QATzip
Yichen Wang writes: > From: Bryan Zhang > > Adds support for migration parameters to control QATzip compression > level and to enable/disable software fallback when QAT hardware is > unavailable. This is a preparatory commit for a subsequent commit that > will actually use QATzip compression. Not sure splitting these two makes sense, but we're at v6, so let's not rock the boat. > Signed-off-by: Bryan Zhang > Signed-off-by: Hao Xiang > Signed-off-by: Yichen Wang QAPI schema Acked-by: Markus Armbruster
Re: [PATCH v2] block-backend: per-device throttling of BLOCK_IO_ERROR reports
Kevin Wolf writes:
> Am 09.01.2024 um 14:13 hat Vladimir Sementsov-Ogievskiy geschrieben:
>> From: Leonid Kaplan
>>
>> BLOCK_IO_ERROR events comes from guest, so we must throttle them.
>> We still want per-device throttling, so let's use device id as a key.
>>
>> Signed-off-by: Leonid Kaplan
>> Signed-off-by: Vladimir Sementsov-Ogievskiy
>> ---
>>
>> v2: add Note: to QAPI doc
>>
>> monitor/monitor.c| 10 ++
>> qapi/block-core.json | 2 ++
>> 2 files changed, 12 insertions(+)
>>
>> diff --git a/monitor/monitor.c b/monitor/monitor.c
>> index 01ede1babd..ad0243e9d7 100644
>> --- a/monitor/monitor.c
>> +++ b/monitor/monitor.c
>> @@ -309,6 +309,7 @@ int error_printf_unless_qmp(const char *fmt, ...)
>> static MonitorQAPIEventConf monitor_qapi_event_conf[QAPI_EVENT__MAX] = {
>> /* Limit guest-triggerable events to 1 per second */
>> [QAPI_EVENT_RTC_CHANGE]= { 1000 * SCALE_MS },
>> +[QAPI_EVENT_BLOCK_IO_ERROR]= { 1000 * SCALE_MS },
>> [QAPI_EVENT_WATCHDOG] = { 1000 * SCALE_MS },
>> [QAPI_EVENT_BALLOON_CHANGE]= { 1000 * SCALE_MS },
>> [QAPI_EVENT_QUORUM_REPORT_BAD] = { 1000 * SCALE_MS },
>> @@ -498,6 +499,10 @@ static unsigned int qapi_event_throttle_hash(const void
>> *key)
>> hash += g_str_hash(qdict_get_str(evstate->data, "qom-path"));
>> }
>>
>> +if (evstate->event == QAPI_EVENT_BLOCK_IO_ERROR) {
>> +hash += g_str_hash(qdict_get_str(evstate->data, "device"));
>> +}
>
> Using "device" only works with -drive, i.e. when the BlockBackend
> actually has a name. In modern configurations with a -blockdev
> referenced by -device, the BlockBackend doesn't have a name any more.
>
> Maybe we should be using the qdev id (or more generally, QOM path) here,
> but that's something the event doesn't even contain yet.
Uh, does the event reliably identify the I/O error's node or not? If
not, then that's a serious design defect.
There's @node-name. Several commands use "either @device or @node-name"
to identify a node. Is that sufficient here?
[...]
Re: [PATCH v6 4/5] migration: Introduce 'qatzip' compression method
Yichen Wang writes: > From: Bryan Zhang > > Adds support for 'qatzip' as an option for the multifd compression > method parameter, and implements using QAT for 'qatzip' compression and > decompression. > > Signed-off-by: Bryan Zhang > Signed-off-by: Hao Xiang > Signed-off-by: Yichen Wang QAPI schema Acked-by: Markus Armbruster
[PATCH] qapi/machine: Belatedly document target loongarch64 is since 7.1
Fixes: a8a506c39070 (hw/loongarch: Add support loongson3 virt machine type.) Signed-off-by: Markus Armbruster --- qapi/machine.json | 2 ++ 1 file changed, 2 insertions(+) diff --git a/qapi/machine.json b/qapi/machine.json index f9ea6b3e97..fcfd249e2d 100644 --- a/qapi/machine.json +++ b/qapi/machine.json @@ -24,6 +24,8 @@ # # @avr: since 5.1 # +# @loongarch64: since 7.1 +# # .. note:: The resulting QMP strings can be appended to the #"qemu-system-" prefix to produce the corresponding QEMU executable #name. This is true even for "qemu-system-x86_64". -- 2.45.0
Re: [PATCH v5 2/4] docs/interop/firmware.json: add new enum FirmwareArchitecture
Thomas Weißschuh writes:
> On Thu, Jul 18, 2024 at 03:18:07PM GMT, Markus Armbruster wrote:
>> Thomas Weißschuh writes:
>>
>> > Only a small subset of all architectures supported by qemu make use of
>> > firmware files. Introduce and use a new enum to represent this.
>> >
>> > This also removes the dependency to machine.json from the global qapi
>> > definitions.
>> >
>> > Suggested-by: Daniel P. Berrangé
>> > Reviewed-by: Daniel P. Berrangé
>> > Signed-off-by: Thomas Weißschuh
>> > ---
>> > docs/interop/firmware.json | 29 +++--
>> > 1 file changed, 27 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json
>> > index a26fe81bf2fe..2eb0be11d595 100644
>> > --- a/docs/interop/firmware.json
>> > +++ b/docs/interop/firmware.json
>> > @@ -14,7 +14,10 @@
>> > # = Firmware
>> > ##
>> >
>> > -{ 'include' : 'machine.json' }
>> > +{ 'pragma': {
>> > +'member-name-exceptions': [
>> > +'FirmwareArchitecture' # x86_64
>> > +] } }
>> >
>> > ##
>> > # @FirmwareOSInterface:
>> > @@ -59,6 +62,28 @@
>> > { 'enum' : 'FirmwareDevice',
>> >'data' : [ 'flash', 'kernel', 'memory' ] }
>> >
>> > +##
>> > +# @FirmwareArchitecture:
>> > +#
>> > +# Enumerations of architectures for which Qemu uses additional firmware
>> > files.
>>
>> docs/devel/qapi-code-gen.rst section "Documentation markup":
>>
>> For legibility, wrap text paragraphs so every line is at most 70
>> characters long.
>>
>> > +# The values are a subset of the enum SysEmuTarget.
>
> Ack.
>
>> Will consumers of firmware.json care for this?
>
> Most probably not.
>
>> Or is it just a reminder for developers to keep the two enums in sync?
>
> I guess so.
> Should I drop it?
You can drop it.
If you think it's useful for developers, you can instead make it a
non-doc comment, like ...
>>
>> > +#
>> > +# @aarch64: 64-bit Arm.
>> > +#
>> > +# @arm: 32-bit Arm.
>> > +#
>> > +# @i386: 32-bit x86.
>> > +#
>> > +# @loongarch64: 64-bit LoongArch.
>> > +#
>> > +# @x86_64: 64-bit x86.
>> > +#
>> > +# Since: 9.1
>>
>> The enum type is indeed since 9.1, but its members are since 3.0, and
>> that's what matters. Except for @loongarch, which is since 7.1.0 (not
>> documented in qapi/machine.json; I'll fix that).
>>
>> > +##
>> > +{ 'enum' : 'FirmwareArchitecture',
>> > + 'data' : [ 'aarch64', 'arm', 'i386', 'loongarch64', 'x86_64' ] }
... here:
# The values are a subset of the enum SysEmuTarget defined in
# qapi/machine.json.
If you do, consider adding a similar note to FirmwareFormat in the
previous patch.
>> > +
>> > +
>>
>> Drop one blank line, please.
>
> Ack.
>
>>
>> > ##
>> > # @FirmwareTarget:
>> > #
>> > @@ -80,7 +105,7 @@
>> > # Since: 3.0
>> > ##
>> > { 'struct' : 'FirmwareTarget',
>> > - 'data' : { 'architecture' : 'SysEmuTarget',
>> > + 'data' : { 'architecture' : 'FirmwareArchitecture',
>> > 'machines' : [ 'str' ] } }
>> >
>> > ##
>>
Re: [PATCH v5 1/4] docs/interop/firmware.json: add new enum FirmwareFormat
Thomas Weißschuh writes:
> On Thu, Jul 18, 2024 at 03:09:37PM GMT, Markus Armbruster wrote:
>> Thomas Weißschuh writes:
>>
>> > Only a small subset of all blockdev drivers make sense for firmware
>> > images. Introduce and use a new enum to represent this.
>> >
>> > This also reduces the dependency on firmware.json from the global qapi
>> > definitions.
>> >
>> > Suggested-by: Daniel P. Berrangé
>> > Reviewed-by: Daniel P. Berrangé
>> > Signed-off-by: Thomas Weißschuh
>> > ---
>> > docs/interop/firmware.json | 17 +++--
>> > 1 file changed, 15 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json
>> > index 54a1fc6c1041..a26fe81bf2fe 100644
>> > --- a/docs/interop/firmware.json
>> > +++ b/docs/interop/firmware.json
>> > @@ -15,7 +15,6 @@
>> > ##
>> >
>> > { 'include' : 'machine.json' }
>> > -{ 'include' : 'block-core.json' }
>> >
>> > ##
>> > # @FirmwareOSInterface:
>> > @@ -200,6 +199,20 @@
>> > 'enrolled-keys', 'requires-smm', 'secure-boot',
>> > 'verbose-dynamic', 'verbose-static' ] }
>> >
>> > +##
>> > +# @FirmwareFormat:
>> > +#
>> > +# Formats that are supported for firmware images.
>> > +#
>> > +# @raw: Raw disk image format.
>> > +#
>> > +# @qcow2: QEMU image format.
>>
>> It's not the only QEMU image format... Maybe "The QCOW2 image format."
>> Almost tautological, but I don't have better ideas.
>
> I used the wording from qemu-img(1).
Interesting, wasn't aware.
> If you prefer "The QCOW2 image format", I'll switch to that.
Up to you, unless Kevin or Hanna have a preference.
>> > +#
>> > +# Since: 9.1
>>
>> The enum type is indeed since 9.1, but its two members are since 3.0,
>> and that's what matters.
>
> Ack.
>
> So I change the Since: of the whole enum?
> And not on the individual members?
Whole enum is simpler, so that's what I'd do. Perhaps with a brief
explanation in the commit message. Here's my try:
Claim "Since: 3.0" for the new enum, because that's correct for its
members, and the members are what matters in the interface.
>> > +##
>> > +{ 'enum': 'FirmwareFormat',
>> > + 'data': [ 'raw', 'qcow2' ] }
>> > +
>> > ##
>> > # @FirmwareFlashFile:
>> > #
>> > @@ -219,7 +232,7 @@
>> > ##
>> > { 'struct' : 'FirmwareFlashFile',
>> >'data' : { 'filename' : 'str',
>> > - 'format' : 'BlockdevDriver' } }
>> > + 'format' : 'FirmwareFormat' } }
>> >
>> >
>> > ##
>>
Re: [PATCH v2] target/s390x: filter deprecated properties based on model expansion type
Collin Walling writes: > As s390 CPU models progress and deprecated properties are dropped > outright, it will be cumbersome for management apps to query the host > for a comprehensive list of deprecated properties that will need to be > disabled on older models. To remedy this, the query-cpu-model-expansion > output now behaves by filtering deprecated properties based on the > expansion type instead of filtering based off of the model's full set > of features: > > When reporting a static CPU model, only show deprecated properties that > are a subset of the model's enabled features. > > When reporting a full CPU model, show the entire list of deprecated > properties regardless if they are supported on the model. > > Suggested-by: Jiri Denemark > Signed-off-by: Collin Walling > --- > > Changelog: > > v2 > - Changed commit message > - Added documentation reflecting this change > - Made code changes that more accurately filter the deprecated > properties based on expansion type. This change makes it > so that the deprecated-properties reported for a static model > expansion are a subset of the model's properties instead of > the model's full-definition properties. > > For example: > > Previously, the z900 static model would report 'bpb' in the > list of deprecated-properties. However, this prop is *not* > a part of the model's feature set, leading to some inaccuracy > (albeit harmless). > > Now, this feature will not show during a static expansion. > It will, however, show up in a full expansion (along with > the rest of the list: 'csske', 'te', 'cte'). > > @David, I've elected to respectully forgo adding your ack-by on this > iteration since I have changed the code (and therefore the behavior) > between this version and the previous in case you do not agree with > these adjustments. > > --- > qapi/machine-target.json | 8 ++-- > target/s390x/cpu_models_sysemu.c | 16 +--- > 2 files changed, 15 insertions(+), 9 deletions(-) > > diff --git a/qapi/machine-target.json b/qapi/machine-target.json > index a8d9ec87f5..d151504f25 100644 > --- a/qapi/machine-target.json > +++ b/qapi/machine-target.json > @@ -21,8 +21,12 @@ > # @props: a dictionary of QOM properties to be applied > # > # @deprecated-props: a list of properties that are flagged as deprecated > -# by the CPU vendor. These props are a subset of the full model's > -# definition list of properties. (since 9.1) > +# by the CPU vendor. (since 9.1). > +# > +# .. note:: Since 9.1, the list of deprecated props were always a subset > +#of the model's full-definition list of properites. Now, this list is > +#populated with the model's enabled property set when delta changes > +#are applied. All deprecated properties are reported otherwise. I'm confused. "Since 9.1, the list of deprecated props were ..." and "Now, this list is" sounds like you're explaining behavior before and after a change. What change? Since only released behavior matters, and @deprecated-props is new, there is no old behavior to document, isn't it? docs/devel/qapi-code-gen.rst section "Documentation markup": For legibility, wrap text paragraphs so every line is at most 70 characters long. Separate sentences with two spaces. > # > # Since: 2.8 > ## [...]
Re: [PATCH v5 3/4] docs/interop/firmware.json: convert "Example" section
Thomas Weißschuh writes:
> Since commit 3c5f6114d9ff ("qapi: remove "Example" doc section")
> the "Example" section is not valid anymore.
> It has been replaced by the "qmp-example" role.
Make that "directive" instead of role.
> This was not detected earlier as firmware.json was not validated.
Sorry about that!
> As this validation is about to be added, adapt firmware.json.
>
> Signed-off-by: Thomas Weißschuh
> ---
> docs/interop/firmware.json | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json
> index 2eb0be11d595..eea82eef3a07 100644
> --- a/docs/interop/firmware.json
> +++ b/docs/interop/firmware.json
> @@ -471,7 +471,7 @@
> #
> # Since: 3.0
> #
> -# Examples:
> +# .. qmp-example::
> #
> # {
> # "description": "SeaBIOS",
Reviewed-by: Markus Armbruster
Re: [PATCH v5 2/4] docs/interop/firmware.json: add new enum FirmwareArchitecture
Thomas Weißschuh writes:
> Only a small subset of all architectures supported by qemu make use of
> firmware files. Introduce and use a new enum to represent this.
>
> This also removes the dependency to machine.json from the global qapi
> definitions.
>
> Suggested-by: Daniel P. Berrangé
> Reviewed-by: Daniel P. Berrangé
> Signed-off-by: Thomas Weißschuh
> ---
> docs/interop/firmware.json | 29 +++--
> 1 file changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json
> index a26fe81bf2fe..2eb0be11d595 100644
> --- a/docs/interop/firmware.json
> +++ b/docs/interop/firmware.json
> @@ -14,7 +14,10 @@
> # = Firmware
> ##
>
> -{ 'include' : 'machine.json' }
> +{ 'pragma': {
> +'member-name-exceptions': [
> +'FirmwareArchitecture' # x86_64
> +] } }
>
> ##
> # @FirmwareOSInterface:
> @@ -59,6 +62,28 @@
> { 'enum' : 'FirmwareDevice',
>'data' : [ 'flash', 'kernel', 'memory' ] }
>
> +##
> +# @FirmwareArchitecture:
> +#
> +# Enumerations of architectures for which Qemu uses additional firmware
> files.
docs/devel/qapi-code-gen.rst section "Documentation markup":
For legibility, wrap text paragraphs so every line is at most 70
characters long.
> +# The values are a subset of the enum SysEmuTarget.
Will consumers of firmware.json care for this? Or is it just a reminder
for developers to keep the two enums in sync?
> +#
> +# @aarch64: 64-bit Arm.
> +#
> +# @arm: 32-bit Arm.
> +#
> +# @i386: 32-bit x86.
> +#
> +# @loongarch64: 64-bit LoongArch.
> +#
> +# @x86_64: 64-bit x86.
> +#
> +# Since: 9.1
The enum type is indeed since 9.1, but its members are since 3.0, and
that's what matters. Except for @loongarch, which is since 7.1.0 (not
documented in qapi/machine.json; I'll fix that).
> +##
> +{ 'enum' : 'FirmwareArchitecture',
> + 'data' : [ 'aarch64', 'arm', 'i386', 'loongarch64', 'x86_64' ] }
> +
> +
Drop one blank line, please.
> ##
> # @FirmwareTarget:
> #
> @@ -80,7 +105,7 @@
> # Since: 3.0
> ##
> { 'struct' : 'FirmwareTarget',
> - 'data' : { 'architecture' : 'SysEmuTarget',
> + 'data' : { 'architecture' : 'FirmwareArchitecture',
> 'machines' : [ 'str' ] } }
>
> ##
Re: [PATCH v5 1/4] docs/interop/firmware.json: add new enum FirmwareFormat
Thomas Weißschuh writes:
> Only a small subset of all blockdev drivers make sense for firmware
> images. Introduce and use a new enum to represent this.
>
> This also reduces the dependency on firmware.json from the global qapi
> definitions.
>
> Suggested-by: Daniel P. Berrangé
> Reviewed-by: Daniel P. Berrangé
> Signed-off-by: Thomas Weißschuh
> ---
> docs/interop/firmware.json | 17 +++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json
> index 54a1fc6c1041..a26fe81bf2fe 100644
> --- a/docs/interop/firmware.json
> +++ b/docs/interop/firmware.json
> @@ -15,7 +15,6 @@
> ##
>
> { 'include' : 'machine.json' }
> -{ 'include' : 'block-core.json' }
>
> ##
> # @FirmwareOSInterface:
> @@ -200,6 +199,20 @@
> 'enrolled-keys', 'requires-smm', 'secure-boot',
> 'verbose-dynamic', 'verbose-static' ] }
>
> +##
> +# @FirmwareFormat:
> +#
> +# Formats that are supported for firmware images.
> +#
> +# @raw: Raw disk image format.
> +#
> +# @qcow2: QEMU image format.
It's not the only QEMU image format... Maybe "The QCOW2 image format."
Almost tautological, but I don't have better ideas.
> +#
> +# Since: 9.1
The enum type is indeed since 9.1, but its two members are since 3.0,
and that's what matters.
> +##
> +{ 'enum': 'FirmwareFormat',
> + 'data': [ 'raw', 'qcow2' ] }
> +
> ##
> # @FirmwareFlashFile:
> #
> @@ -219,7 +232,7 @@
> ##
> { 'struct' : 'FirmwareFlashFile',
>'data' : { 'filename' : 'str',
> - 'format' : 'BlockdevDriver' } }
> + 'format' : 'FirmwareFormat' } }
>
>
> ##
[PATCH] qapi-block-core: Clean up blockdev-snapshot-internal-sync doc
BlockdevSnapshotInternal is the arguments type of command
blockdev-snapshot-internal-sync. Its doc comment contains this note:
# .. note:: In a transaction, if @name is empty or any snapshot matching
#@name exists, the operation will fail. Only some image formats
#support it; for example, qcow2, and rbd.
"In a transaction" is misleading, and "if @name is empty or any
snapshot matching @name exists, the operation will fail" is redundant
with the command's Errors documentation. Drop.
The remainder is fine. Move it to the command's doc comment, where it
is more prominently visible, with a slight rephrasing for clarity.
Signed-off-by: Markus Armbruster
---
qapi/block-core.json | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index f400b334c8..994e384a71 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -6046,10 +6046,6 @@
#
# @name: the name of the internal snapshot to be created
#
-# .. note:: In a transaction, if @name is empty or any snapshot matching
-#@name exists, the operation will fail. Only some image formats
-#support it; for example, qcow2, and rbd.
-#
# Since: 1.7
##
{ 'struct': 'BlockdevSnapshotInternal',
@@ -6070,6 +6066,9 @@
# - If the format of the image used does not support it,
# GenericError
#
+# .. note:: Only some image formats such as qcow2 and rbd support
+#internal snapshots.
+#
# Since: 1.7
#
# .. qmp-example::
--
2.45.0
Re: [PATCH v9 4/7] qapi: add blockdev-replace command
Vladimir Sementsov-Ogievskiy writes:
> Add a command that can replace bs in following BdrvChild structures:
>
> - qdev blk root child
> - block-export blk root child
> - any child of BlockDriverState selected by child-name
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> blockdev.c | 56 +++
> qapi/block-core.json | 88 ++
> stubs/blk-by-qdev-id.c | 13 +++
> stubs/meson.build | 1 +
> 4 files changed, 158 insertions(+)
> create mode 100644 stubs/blk-by-qdev-id.c
>
> diff --git a/blockdev.c b/blockdev.c
> index ba7e90b06e..2190467022 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -3559,6 +3559,62 @@ void qmp_x_blockdev_set_iothread(const char
> *node_name, StrOrNull *iothread,
> bdrv_try_change_aio_context(bs, new_context, NULL, errp);
> }
>
> +void qmp_blockdev_replace(BlockdevReplace *repl, Error **errp)
> +{
> +BdrvChild *child = NULL;
> +BlockDriverState *new_child_bs;
> +
> +if (repl->parent_type == BLOCK_PARENT_TYPE_DRIVER) {
> +BlockDriverState *parent_bs;
> +
> +parent_bs = bdrv_find_node(repl->u.driver.node_name);
> +if (!parent_bs) {
> +error_setg(errp, "Block driver node with node-name '%s' not "
> + "found", repl->u.driver.node_name);
> +return;
> +}
> +
> +child = bdrv_find_child(parent_bs, repl->u.driver.child);
> +if (!child) {
> +error_setg(errp, "Block driver node '%s' doesn't have child "
> + "named '%s'", repl->u.driver.node_name,
> + repl->u.driver.child);
> +return;
> +}
> +} else {
> +/* Other types are similar, they work through blk */
> +BlockBackend *blk;
> +bool is_qdev = repl->parent_type == BLOCK_PARENT_TYPE_QDEV;
> +const char *id =
> +is_qdev ? repl->u.qdev.qdev_id : repl->u.export.export_id;
> +
> +assert(is_qdev || repl->parent_type == BLOCK_PARENT_TYPE_EXPORT);
> +
> +blk = is_qdev ? blk_by_qdev_id(id, errp) : blk_by_export_id(id,
> errp);
blk_by_export_id() finds export @exp, and returns the associated block
backend exp->blk. Fine.
blk_by_qdev_id() finds the device, and then searches @block_backends for
a blk with blk->dev == blk. If a device has more than one block
backend, you get the one first in @block_backends. I figure that's the
one created first.
Interface issue: when a device has multiple block backends, only one of
them can be replaced, and which one is kind of random.
Do such devices exist?
If no, could they exist?
If yes, what should we do about it now?
> +if (!blk) {
> +return;
> +}
> +
> +child = blk_root(blk);
> +if (!child) {
> +error_setg(errp, "%s '%s' is empty, nothing to replace",
> + is_qdev ? "Device" : "Export", id);
> +return;
> +}
> +}
> +
> +assert(child);
> +assert(child->bs);
> +
> +new_child_bs = bdrv_find_node(repl->new_child);
> +if (!new_child_bs) {
> +error_setg(errp, "Node '%s' not found", repl->new_child);
> +return;
> +}
> +
> +bdrv_replace_child_bs(child, new_child_bs, errp);
> +}
> +
> QemuOptsList qemu_common_drive_opts = {
> .name = "drive",
> .head = QTAILQ_HEAD_INITIALIZER(qemu_common_drive_opts.head),
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index df5e07debd..0a6f08a6e0 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -6148,3 +6148,91 @@
> ##
> { 'struct': 'DummyBlockCoreForceArrays',
>'data': { 'unused-block-graph-info': ['BlockGraphInfo'] } }
> +
> +##
> +# @BlockParentType:
> +#
> +# @qdev: block device, such as created by device_add, and denoted by
> +# qdev-id
> +#
> +# @driver: block driver node, such as created by blockdev-add, and
> +# denoted by node-name
node-name and child?
> +#
> +# @export: block export, such created by block-export-add, and
> +# denoted by export-id
> +#
> +# Since 9.1
> +##
I'm kind of unhappy with this doc comment. I feel makes sense only in
the context of its use. Let me try to avoid that:
# @driver: the parent is a block node, the child is one of its
# children.
#
# @export: the parent is a block export, the child is its block
# backend.
#
# @qdev: the parent is a device, the child is its block backend.
> +{ 'enum': 'BlockParentType',
> + 'data': ['qdev', 'driver', 'export'] }
If you take my comment, change the order here as well.
> +
> +##
> +# @BdrvChildRefQdev:
> +#
> +# @qdev-id: the device's ID or QOM path
> +#
> +# Since 9.1
> +##
> +{ 'struct': 'BdrvChildRefQdev',
> + 'data': { 'qdev-id': 'str' } }
> +
> +##
> +# @BdrvChildRefExport:
> +#
> +# @export-id: block export identifier
block-export.json calls this "block export id" in some places, and
"block export identifier" in
Re: [PATCH v9 2/7] block/export: add blk_by_export_id()
Vladimir Sementsov-Ogievskiy writes:
> We need it for further blockdev-replace functionality.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> block/export/export.c | 18 ++
> include/sysemu/block-backend-global-state.h | 1 +
> 2 files changed, 19 insertions(+)
>
> diff --git a/block/export/export.c b/block/export/export.c
> index 6d51ae8ed7..57beae7982 100644
> --- a/block/export/export.c
> +++ b/block/export/export.c
> @@ -355,3 +355,21 @@ BlockExportInfoList *qmp_query_block_exports(Error
> **errp)
>
> return head;
> }
> +
> +BlockBackend *blk_by_export_id(const char *id, Error **errp)
> +{
> +BlockExport *exp;
> +
> +exp = blk_exp_find(id);
> +if (exp == NULL) {
> +error_setg(errp, "Export '%s' not found", id);
> +return NULL;
> +}
> +
> +if (!exp->blk) {
> +error_setg(errp, "Export '%s' is empty", id);
Can this happen?
> +return NULL;
> +}
> +
> +return exp->blk;
> +}
> diff --git a/include/sysemu/block-backend-global-state.h
> b/include/sysemu/block-backend-global-state.h
> index ccb35546a1..410d0cc5c7 100644
> --- a/include/sysemu/block-backend-global-state.h
> +++ b/include/sysemu/block-backend-global-state.h
> @@ -74,6 +74,7 @@ void blk_detach_dev(BlockBackend *blk, DeviceState *dev);
> DeviceState *blk_get_attached_dev(BlockBackend *blk);
> BlockBackend *blk_by_dev(void *dev);
> BlockBackend *blk_by_qdev_id(const char *id, Error **errp);
> +BlockBackend *blk_by_export_id(const char *id, Error **errp);
> void blk_set_dev_ops(BlockBackend *blk, const BlockDevOps *ops, void
> *opaque);
>
> void blk_activate(BlockBackend *blk, Error **errp);
Re: [PATCH v2 6/7] qapi/block-core: derpecate block-job-change
Vladimir Sementsov-Ogievskiy writes: > That's a first step to move on newer job-* APIs. > > The difference between block-job-change and job-change is in > find_block_job_locked() vs find_job_locked() functions. What's > different? > > 1. find_block_job_locked() do check, is found job a block-job. This OK Do you mean something like find_block_job_locked() finds only block jobs, whereas find_job_locked() finds any kind of job? >when moving to more generic API, no needs to document this change. > > 2. find_block_job_locked() reports DeviceNotActive on failure, when >find_job_locked() reports GenericError. Still, for block-job-change >errors are not documented at all, so be silent in deprecated.txt as >well. > > Signed-off-by: Vladimir Sementsov-Ogievskiy
Re: [PATCH v2 5/7] qapi: add job-change
Vladimir Sementsov-Ogievskiy writes:
> Add a new-style command job-change, doing same thing as
> block-job-change. The aim is finally deprecate block-job-* APIs and
> move to job-* APIs.
>
> We add a new command to qapi/block-core.json, not to
> qapi/job.json to avoid resolving json file including loops for now.
> This all would be a lot simple to refactor when we finally drop
> deprecated block-job-* APIs.
>
> @type argument of the new command immediately becomes deprecated.
Where?
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> job-qmp.c| 14 ++
> qapi/block-core.json | 10 ++
> 2 files changed, 24 insertions(+)
>
> diff --git a/job-qmp.c b/job-qmp.c
> index c764bd3801..248e68f554 100644
> --- a/job-qmp.c
> +++ b/job-qmp.c
> @@ -139,6 +139,20 @@ void qmp_job_dismiss(const char *id, Error **errp)
> job_dismiss_locked(, errp);
> }
>
> +void qmp_job_change(JobChangeOptions *opts, Error **errp)
> +{
> +Job *job;
> +
> +JOB_LOCK_GUARD();
> +job = find_job_locked(opts->id, errp);
> +
> +if (!job) {
> +return;
> +}
> +
> +job_change_locked(job, opts, errp);
> +}
> +
> /* Called with job_mutex held. */
> static JobInfo *job_query_single_locked(Job *job, Error **errp)
> {
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 660c7f4a48..9087ce300c 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -3104,6 +3104,16 @@
> { 'command': 'block-job-change',
>'data': 'JobChangeOptions', 'boxed': true }
>
> +##
> +# @job-change:
> +#
> +# Change the block job's options.
> +#
> +# Since: 9.1
> +##
> +{ 'command': 'job-change',
> + 'data': 'JobChangeOptions', 'boxed': true }
> +
> ##
> # @BlockdevDiscardOptions:
> #
Re: [PATCH v2 3/7] qapi: block-job-change: make copy-mode parameter optional
Vladimir Sementsov-Ogievskiy writes:
> We are going to add more parameters to change. We want to make possible
> to change only one or any subset of available options. So all the
> options should be optional.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> block/mirror.c | 4
> qapi/block-core.json | 3 ++-
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/block/mirror.c b/block/mirror.c
> index 2816bb1042..60e8d83e4f 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -1272,6 +1272,10 @@ static void mirror_change(BlockJob *job,
> JobChangeOptions *opts,
>
> GLOBAL_STATE_CODE();
>
> +if (!change_opts->has_copy_mode) {
> +return;
> +}
> +
> if (qatomic_read(>copy_mode) == change_opts->copy_mode) {
> return;
> }
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 4ec5632596..660c7f4a48 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -3071,11 +3071,12 @@
> #
> # @copy-mode: Switch to this copy mode. Currently, only the switch
> # from 'background' to 'write-blocking' is implemented.
> +# If absent, copy mode remains the same. (optional since 9.1)
> #
> # Since: 8.2
> ##
> { 'struct': 'JobChangeOptionsMirror',
> - 'data': { 'copy-mode' : 'MirrorCopyMode' } }
> + 'data': { '*copy-mode' : 'MirrorCopyMode' } }
>
> ##
> # @JobChangeOptions:
Acked-by: Markus Armbruster
Re: [PATCH v2 1/7] qapi: rename BlockJobChangeOptions to JobChangeOptions
Vladimir Sementsov-Ogievskiy writes: > We are going to move change action from block-job to job > implementation, and then move to job-* extenral APIs, deprecating > block-job-* APIs. This commit simplifies further transition. > > The commit is made by command > > git grep -l BlockJobChangeOptions | \ > xargs sed -i 's/BlockJobChangeOptions/JobChangeOptions/g' > > Signed-off-by: Vladimir Sementsov-Ogievskiy Acked-by: Markus Armbruster
Re: [PATCH v3] chardev: add path option for pty backend
Daniel P. Berrangé writes:
> On Thu, Jul 18, 2024 at 08:15:01AM +0200, Markus Armbruster wrote:
>> Looks like this one fell through the cracks.
>>
>> Octavian Purdila writes:
>>
>> > Add path option to the pty char backend which will create a symbolic
>> > link to the given path that points to the allocated PTY.
>> >
>> > This avoids having to make QMP or HMP monitor queries to find out what
>> > the new PTY device path is.
>>
>> QMP commands chardev-add and chardev-change return the information you
>> want:
>>
>> # @pty: name of the slave pseudoterminal device, present if and only
>> # if a chardev of type 'pty' was created
>>
>> So does HMP command chardev-add. HMP chardev apparently doesn't, but
>> that could be fixed.
>
> It does print it:
>
> (qemu) chardev-add pty,id=bar
> char device redirected to /dev/pts/12 (label bar)
I fat-fingered "HMP chardev-change".
>> So, the use case is basically the command line, right?
>
> Also cli prints it
>
> $ qemu-system-x86_64 -chardev pty,id=foo -monitor stdio -display none
> char device redirected to /dev/pts/10 (label foo)
Good enough for ad hoc use by humans.
Management applications should use QMP, which returns it.
I guess there's scripts in between.
>> > Based on patch from Paulo Neves:
>> >
>> > https://patchew.org/QEMU/1548509635-15776-1-git-send-email-ptsne...@gmail.com/
>> >
>> > Tested with the following invocations that the link is created and
>> > removed when qemu stops:
>> >
>> > qemu-system-x86_64 -nodefaults -mon chardev=compat_monitor \
>> > -chardev pty,path=test,id=compat_monitor0
>> >
>> > qemu-system-x86_64 -nodefaults -monitor pty:test
>> >
>> > Also tested that when a link path is not passed invocations still work,
>> > e.g.:
>> >
>> > qemu-system-x86_64 -monitor pty
>> >
>> > Co-authored-by: Paulo Neves
>> > Signed-off-by: Paulo Neves
>> > [OP: rebase and address original patch review comments]
>> > Signed-off-by: Octavian Purdila
>> > Reviewed-by: Marc-André Lureau
[...]
>> > diff --git a/chardev/char-pty.c b/chardev/char-pty.c
>> > index cc2f7617fe..5c6172ddba 100644
>> > --- a/chardev/char-pty.c
>> > +++ b/chardev/char-pty.c
>> > @@ -29,6 +29,7 @@
>> > #include "qemu/sockets.h"
>> > #include "qemu/error-report.h"
>> > #include "qemu/module.h"
>> > +#include "qemu/option.h"
>> > #include "qemu/qemu-print.h"
>> >
>> > #include "chardev/char-io.h"
>> > @@ -41,6 +42,7 @@ struct PtyChardev {
>> >
>> > int connected;
>> > GSource *timer_src;
>> > +char *symlink_path;
>> > };
>> > typedef struct PtyChardev PtyChardev;
>> >
>> > @@ -204,6 +206,12 @@ static void char_pty_finalize(Object *obj)
>> > Chardev *chr = CHARDEV(obj);
>> > PtyChardev *s = PTY_CHARDEV(obj);
>> >
>> > +/* unlink symlink */
>> > +if (s->symlink_path) {
>> > +unlink(s->symlink_path);
>> > +g_free(s->symlink_path);
>> > +}
>>
>> Runs when the chardev object is finalized.
>>
>> Doesn't run when QEMU crashes. Stale symlink left behind then. Can't
>> see how you could avoid that at reasonable cost. Troublesome all the
>> same.
>
> Do we ever guarantee that the finalizer runs ? eg dif we have
>
> error_setg(_exit,
>
> that's a clean exit, not a crash, but I don't think chardev finalizers
> will run, as we don't do atexit() hooks for it.
Point.
>> The feature feels rather doubtful to me, to be honest.
>
> On the one hand I understand the pain - long ago libvirt had to deal
> with parsing the console messages
>
> char device redirected to /dev/pts/10 (label foo)
>
> before we switched to using QMP to query this.
>
> On the other hand, in retrospect libvirt should never have used the 'pty'
> backend in the first place. The 'unix' socket backend is a choice as it
> has predictable filenames, and it has proper connection oriented semantics,
> so QEMU can reliably detect when clients disconnect, which has always been
> troublesome for the 'pty' backend.
>
> So while I can understand the desire to add a 'path' option to 'pty'
> to trigger symlink creation, I think we could choose to tell people
> to use the 'unix' socket backend instead if they want a predictable
> path. This would avoid us creating the difficult to fix bug for
> symlink deletion in error conditions.
>
> What's the key benefit of the 'pty' backend, that 'unix' doesn't
> handle ?
I think this is the question to answer.
Re: [PATCH v3] chardev: add path option for pty backend
Peter Maydell writes: > On Thu, 18 Jul 2024 at 07:15, Markus Armbruster wrote: >> >> Looks like this one fell through the cracks. >> >> Octavian Purdila writes: >> >> > Add path option to the pty char backend which will create a symbolic >> > link to the given path that points to the allocated PTY. >> > >> > This avoids having to make QMP or HMP monitor queries to find out what >> > the new PTY device path is. >> >> QMP commands chardev-add and chardev-change return the information you >> want: >> >> # @pty: name of the slave pseudoterminal device, present if and only >> # if a chardev of type 'pty' was created >> >> So does HMP command chardev-add. HMP chardev apparently doesn't, but >> that could be fixed. >> >> So, the use case is basically the command line, right? > >> The feature feels rather doubtful to me, to be honest. > > The command line is an important use-case, though. Not every > user of QEMU is libvirt with a QMP/HMP connection readily > to hand that they would prefer to use for all configuration... In general yes. But what are the use cases for this one? To me, specifying path=/mumble/symlink plus the bother of cleaning up stale ones doesn't feel like much of an improvement over reading the pty name from "info chardev". I guess I'm missing something. Tell me! If we decide we want this, then the QMP interface needs to be fixed: Call the argument @path for consistency, and document it properly. Actually straightforward, just create a new struct instead of pressing ChardevHostdev into service. Some advice on robust use of @path could be useful, in particular on guarding against QEMU leaving stale links behind. Additional decision: whether to extend the old-style syntax.
Re: [PATCH v5 0/3] vhost-user-blk: live resize additional APIs
Vladimir Sementsov-Ogievskiy writes: > ping. Markus, Eric, could someone give an ACC for QAPI part? I apologize for the delay. It was pretty bad.
Re: [PATCH v5 1/3] qdev-monitor: add option to report GenericError from find_device_state
Vladimir Sementsov-Ogievskiy writes: > Here we just prepare for the following patch, making possible to report > GenericError as recommended. > > This patch doesn't aim to prevent further use of DeviceNotFound by > future interfaces: > > - find_device_state() is used in blk_by_qdev_id() and qmp_get_blk() >functions, which may lead to spread of DeviceNotFound anyway > - also, nothing prevent simply copy-pasting find_device_state() calls >with false argument A possible way to reduce the likelihood of further spread: 1. Rename find_device_state() to find_device_state_legacy(). 2. New find_device_state() that reports GenericError. Could also be done in a follow-up. > > Signed-off-by: Vladimir Sementsov-Ogievskiy The patch does what it says on the tin, so Reviewed-by: Markus Armbruster
Re: [PATCH v5 3/3] qapi: introduce device-sync-config
Vladimir Sementsov-Ogievskiy writes: > Add command to sync config from vhost-user backend to the device. It > may be helpful when VHOST_USER_SLAVE_CONFIG_CHANGE_MSG failed or not > triggered interrupt to the guest or just not available (not supported > by vhost-user server). > > Command result is racy if allow it during migration. Let's allow the > sync only in RUNNING state. Is this still accurate? The runstate_is_running() check is gone in v4, the migration_is_running() check remains. > Signed-off-by: Vladimir Sementsov-Ogievskiy QAPI schema and QMP part: Signed-off-by: Markus Armbruster
Re: [PATCH v3] chardev: add path option for pty backend
Looks like this one fell through the cracks.
Octavian Purdila writes:
> Add path option to the pty char backend which will create a symbolic
> link to the given path that points to the allocated PTY.
>
> This avoids having to make QMP or HMP monitor queries to find out what
> the new PTY device path is.
QMP commands chardev-add and chardev-change return the information you
want:
# @pty: name of the slave pseudoterminal device, present if and only
# if a chardev of type 'pty' was created
So does HMP command chardev-add. HMP chardev apparently doesn't, but
that could be fixed.
So, the use case is basically the command line, right?
> Based on patch from Paulo Neves:
>
> https://patchew.org/QEMU/1548509635-15776-1-git-send-email-ptsne...@gmail.com/
>
> Tested with the following invocations that the link is created and
> removed when qemu stops:
>
> qemu-system-x86_64 -nodefaults -mon chardev=compat_monitor \
> -chardev pty,path=test,id=compat_monitor0
>
> qemu-system-x86_64 -nodefaults -monitor pty:test
>
> Also tested that when a link path is not passed invocations still work, e.g.:
>
> qemu-system-x86_64 -monitor pty
>
> Co-authored-by: Paulo Neves
> Signed-off-by: Paulo Neves
> [OP: rebase and address original patch review comments]
> Signed-off-by: Octavian Purdila
> Reviewed-by: Marc-André Lureau
> ---
> Changes since v2:
>
> * remove NULL path check, g_strdup() allows NULL inputs
>
> Changes since v1:
>
> * Keep the original Signed-off-by from Paulo and add one line
> description with further changes
>
> * Update commit message with justification for why the new
> functionality is useful
>
> * Don't close master_fd when symlink creation fails to avoid double
> close
>
> * Update documentation for clarity
>
> chardev/char-pty.c | 33 +
> chardev/char.c | 5 +
> qapi/char.json | 4 ++--
> qemu-options.hx| 24 ++--
> 4 files changed, 58 insertions(+), 8 deletions(-)
>
> diff --git a/chardev/char-pty.c b/chardev/char-pty.c
> index cc2f7617fe..5c6172ddba 100644
> --- a/chardev/char-pty.c
> +++ b/chardev/char-pty.c
> @@ -29,6 +29,7 @@
> #include "qemu/sockets.h"
> #include "qemu/error-report.h"
> #include "qemu/module.h"
> +#include "qemu/option.h"
> #include "qemu/qemu-print.h"
>
> #include "chardev/char-io.h"
> @@ -41,6 +42,7 @@ struct PtyChardev {
>
> int connected;
> GSource *timer_src;
> +char *symlink_path;
> };
> typedef struct PtyChardev PtyChardev;
>
> @@ -204,6 +206,12 @@ static void char_pty_finalize(Object *obj)
> Chardev *chr = CHARDEV(obj);
> PtyChardev *s = PTY_CHARDEV(obj);
>
> +/* unlink symlink */
> +if (s->symlink_path) {
> +unlink(s->symlink_path);
> +g_free(s->symlink_path);
> +}
Runs when the chardev object is finalized.
Doesn't run when QEMU crashes. Stale symlink left behind then. Can't
see how you could avoid that at reasonable cost. Troublesome all the
same.
> +
> pty_chr_state(chr, 0);
> object_unref(OBJECT(s->ioc));
> pty_chr_timer_cancel(s);
> @@ -330,6 +338,7 @@ static void char_pty_open(Chardev *chr,
> int master_fd, slave_fd;
> char pty_name[PATH_MAX];
> char *name;
> +char *symlink_path = backend->u.pty.data->device;
>
> master_fd = qemu_openpty_raw(_fd, pty_name);
> if (master_fd < 0) {
> @@ -354,12 +363,36 @@ static void char_pty_open(Chardev *chr,
> g_free(name);
> s->timer_src = NULL;
> *be_opened = false;
> +
> +/* create symbolic link */
> +if (symlink_path) {
> +int res = symlink(pty_name, symlink_path);
> +
> +if (res != 0) {
> +error_setg_errno(errp, errno, "Failed to create PTY symlink");
> +} else {
> +s->symlink_path = g_strdup(symlink_path);
> +}
> +}
> +}
> +
> +static void char_pty_parse(QemuOpts *opts, ChardevBackend *backend,
> + Error **errp)
> +{
> +const char *path = qemu_opt_get(opts, "path");
> +ChardevHostdev *dev;
> +
> +backend->type = CHARDEV_BACKEND_KIND_PTY;
> +dev = backend->u.pty.data = g_new0(ChardevHostdev, 1);
> +qemu_chr_parse_common(opts, qapi_ChardevHostdev_base(dev));
> +dev->device = g_strdup(path);
Put a pin into this line.
> }
>
> static void char_pty_class_init(ObjectClass *oc, void *data)
> {
> ChardevClass *cc = CHARDEV_CLASS(oc);
>
> +cc->parse = char_pty_parse;
> cc->open = char_pty_open;
> cc->chr_write = char_pty_chr_write;
> cc->chr_update_read_handler = pty_chr_update_read_handler;
> diff --git a/chardev/char.c b/chardev/char.c
> index 3c43fb1278..404c6b8a4f 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -428,6 +428,11 @@ QemuOpts *qemu_chr_parse_compat(const char *label, const
> char *filename,
> qemu_opt_set(opts, "path", p, _abort);
> return opts;
> }
> +if (strstart(filename, "pty:", ))
Re: [PATCH v2 6/9] qapi: convert "Example" sections without titles
John Snow writes:
> On Wed, Jul 17, 2024, 3:44 AM Markus Armbruster wrote:
>
>> John Snow writes:
>>
>> > Use the no-option form of ".. qmp-example::" to convert any Examples
>> > that do not have any form of caption or explanation whatsoever. Note
>> > that in a few cases, example sections are split into two or more
>> > separate example blocks. This is only done stylistically to create a
>> > delineation between two or more logically independent examples.
>> >
>> > See commit-3: "docs/qapidoc: create qmp-example directive", for a
>> > detailed explanation of this custom directive syntax.
>> >
>> > See commit+3: "qapi: remove "Example" doc section" for a detailed
>> > explanation of why.
>> >
>> > Note: an empty "TODO" line was added to announce-self to keep the
>> > example from floating up into the body; this will be addressed more
>> > rigorously in the new qapidoc generator.
>> >
>> > Signed-off-by: John Snow
>>
>> [...]
>>
>> > diff --git a/qapi/run-state.json b/qapi/run-state.json
>> > index 4d40c88876c..ab7116680b3 100644
>> > --- a/qapi/run-state.json
>> > +++ b/qapi/run-state.json
>>
>> [...]
>>
>> > @@ -469,7 +469,7 @@
>> > #
>> > # Since: 9.1
>> > #
>> > -# Example:
>> > +# ..qmp-example::
>>
>> Lacks a space, output is messed up. Can fix in my tree.
>
> Whoops. Wish rST was a bit pickier sometimes...
"I find it to be the Perl of ASCII-based markups." (Paolo)
>> > #
>> > # <- { "event": "GUEST_PVSHUTDOWN",
>> > # "timestamp": { "seconds": 1648245259, "microseconds": 893771
>> } }
>>
>> [...]
>>
>> R-by stands.
Re: [PATCH] MAINTAINERS: Cover guest-agent in QAPI schema
Philippe Mathieu-Daudé writes: > Signed-off-by: Philippe Mathieu-Daudé > --- > MAINTAINERS | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 7d9811458c..af4db698de 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -3219,6 +3219,7 @@ M: Eric Blake > M: Markus Armbruster > S: Supported > F: qapi/*.json > +F: qga/qapi-schema.json > T: git https://repo.or.cz/qemu/armbru.git qapi-next > > QObject I guess we provide a similar service for qga/qapi-schema.json as we do for much of qapi/: proper use of QAPI, reasonable documentation, but not the subject matter aspects. Reviewed-by: Markus Armbruster
Re: [PATCH 04/14] qapi: add a 'command-features' pragma
Daniel P. Berrangé writes: > So basically the code would always have access to all features, and > we would have no notion of "special" features any more. "Special" gets reduced to "predefined / known to the generator". > I'm happy to give that a try. Thank you!
[PULL 06/14] qapi/ui: Drop note on naming of SpiceQueryMouseMode
Doc comments are reference documentation for users of QMP.
SpiceQueryMouseMode's doc comment contains a note explaining why it's
not named SpiceMouseMode: spice/enums.h has it already. Irrelevant
for users of QMP; delete the note.
Signed-off-by: Markus Armbruster
Message-ID: <2024072228.2140606-6-arm...@redhat.com>
Reviewed-by: John Snow
---
qapi/ui.json | 2 --
1 file changed, 2 deletions(-)
diff --git a/qapi/ui.json b/qapi/ui.json
index 5bcccbfc93..df089869a5 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -273,8 +273,6 @@
# @unknown: No information is available about mouse mode used by the
# spice server.
#
-# .. note:: spice/enums.h has a SpiceMouseMode already, hence the name.
-#
# Since: 1.1
##
{ 'enum': 'SpiceQueryMouseMode',
--
2.45.0
[PULL 01/14] qapi/qom: Document feature unstable of @x-vfio-user-server
Commit 8f9a9259d32c added ObjectType member @x-vfio-user-server with feature unstable, but neglected to explain why it is unstable. Do that now. Fixes: 8f9a9259d32c (vfio-user: define vfio-user-server object) Cc: Elena Ufimtseva Cc: John G Johnson Cc: Jagannathan Raman Cc: qemu-sta...@nongnu.org Signed-off-by: Markus Armbruster Message-ID: <20240703095310.1242102-1-arm...@redhat.com> Reviewed-by: John Snow [Indentation fixed] --- qapi/qom.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/qapi/qom.json b/qapi/qom.json index 8e75a419c3..3e52aec8fb 100644 --- a/qapi/qom.json +++ b/qapi/qom.json @@ -1024,7 +1024,8 @@ # # Features: # -# @unstable: Member @x-remote-object is experimental. +# @unstable: Members @x-remote-object and @x-vfio-user-server are +# experimental. # # Since: 6.0 ## -- 2.45.0
[PULL 03/14] qapi/machine: Clean up documentation around CpuInstanceProperties
CpuInstanceProperties' doc comment describes its members as properties
to be passed to device_add when hot-plugging a CPU.
This was in fact the initial use of this type, with
query-hotpluggable-cpus: letting management applications find out what
properties need to be passed with device_add to hot-plug a CPU.
We've since added other uses: set-numa-node (commit 419fcdec3c1 and
f3be67812c2), and query-cpus-fast (commit ce74ee3dea6). These are not
about device-add.
query-hotpluggable-cpus uses CpuInstanceProperties within
HotpluggableCPU. Lift the documentation related to device-add from
CpuInstanceProperties to HotpluggableCPU.
Signed-off-by: Markus Armbruster
Message-ID: <2024072228.2140606-3-arm...@redhat.com>
Reviewed-by: Michael S. Tsirkin
Reviewed-by: John Snow
---
qapi/machine.json | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/qapi/machine.json b/qapi/machine.json
index f15ad1b43e..50ff102d56 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -960,9 +960,7 @@
##
# @CpuInstanceProperties:
#
-# List of properties to be used for hotplugging a CPU instance, it
-# should be passed by management with device_add command when a CPU is
-# being hotplugged.
+# Properties identifying a CPU.
#
# Which members are optional and which mandatory depends on the
# architecture and board.
@@ -996,9 +994,6 @@
#
# @thread-id: thread number within the core the CPU belongs to
#
-# .. note:: Management should be prepared to pass through additional
-#properties with device_add.
-#
# Since: 2.7
##
{ 'struct': 'CpuInstanceProperties',
@@ -1020,7 +1015,8 @@
#
# @type: CPU object type for usage with device_add command
#
-# @props: list of properties to be used for hotplugging CPU
+# @props: list of properties to pass for hotplugging a CPU with
+# device_add
#
# @vcpus-count: number of logical VCPU threads @HotpluggableCPU
# provides
@@ -1028,6 +1024,9 @@
# @qom-path: link to existing CPU object if CPU is present or omitted
# if CPU is not present.
#
+# .. note:: Management should be prepared to pass through additional
+#properties with device_add.
+#
# Since: 2.7
##
{ 'struct': 'HotpluggableCPU',
--
2.45.0
[PULL 11/14] qapi: convert "Example" sections without titles
From: John Snow
Use the no-option form of ".. qmp-example::" to convert any Examples
that do not have any form of caption or explanation whatsoever. Note
that in a few cases, example sections are split into two or more
separate example blocks. This is only done stylistically to create a
delineation between two or more logically independent examples.
See commit-3: "docs/qapidoc: create qmp-example directive", for a
detailed explanation of this custom directive syntax.
See commit+3: "qapi: remove "Example" doc section" for a detailed
explanation of why.
Note: an empty "TODO" line was added to announce-self to keep the
example from floating up into the body; this will be addressed more
rigorously in the new qapidoc generator.
Signed-off-by: John Snow
Message-ID: <20240717021312.606116-7-js...@redhat.com>
Reviewed-by: Markus Armbruster
[Markup fixed in one place]
Signed-off-by: Markus Armbruster
---
qapi/acpi.json | 4 +--
qapi/block-core.json | 64 +---
qapi/block.json | 18 ++-
qapi/char.json | 24 +--
qapi/control.json| 8 ++---
qapi/dump.json | 8 ++---
qapi/machine-target.json | 2 +-
qapi/machine.json| 38
qapi/migration.json | 58 ++--
qapi/misc-target.json| 22 +++---
qapi/misc.json | 32 ++--
qapi/net.json| 22 --
qapi/pci.json| 2 +-
qapi/qdev.json | 10 ---
qapi/qom.json| 8 ++---
qapi/replay.json | 8 ++---
qapi/rocker.json | 8 ++---
qapi/run-state.json | 32 ++--
qapi/tpm.json| 6 ++--
qapi/trace.json | 4 +--
qapi/transaction.json| 2 +-
qapi/ui.json | 34 ++---
qapi/vfio.json | 2 +-
qapi/virtio.json | 2 +-
qapi/yank.json | 4 +--
25 files changed, 219 insertions(+), 203 deletions(-)
diff --git a/qapi/acpi.json b/qapi/acpi.json
index aa4dbe5794..045dab6228 100644
--- a/qapi/acpi.json
+++ b/qapi/acpi.json
@@ -111,7 +111,7 @@
#
# Since: 2.1
#
-# Example:
+# .. qmp-example::
#
# -> { "execute": "query-acpi-ospm-status" }
# <- { "return": [ { "device": "d1", "slot": "0", "slot-type": "DIMM",
"source": 1, "status": 0},
@@ -131,7 +131,7 @@
#
# Since: 2.1
#
-# Example:
+# .. qmp-example::
#
# <- { "event": "ACPI_DEVICE_OST",
# "data": { "info": { "device": "d1", "slot": "0",
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 096bdbe0aa..84a020aa9e 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -764,7 +764,7 @@
#
# Since: 0.14
#
-# Example:
+# .. qmp-example::
#
# -> { "execute": "query-block" }
# <- {
@@ -1168,7 +1168,7 @@
#
# Since: 0.14
#
-# Example:
+# .. qmp-example::
#
# -> { "execute": "query-blockstats" }
# <- {
@@ -1461,7 +1461,7 @@
#
# Since: 0.14
#
-# Example:
+# .. qmp-example::
#
# -> { "execute": "block_resize",
# "arguments": { "device": "scratch", "size": 1073741824 } }
@@ -1680,7 +1680,7 @@
#
# Since: 0.14
#
-# Example:
+# .. qmp-example::
#
# -> { "execute": "blockdev-snapshot-sync",
# "arguments": { "device": "ide-hd0",
@@ -1711,7 +1711,7 @@
#
# Since: 2.5
#
-# Example:
+# .. qmp-example::
#
# -> { "execute": "blockdev-add",
# "arguments": { "driver": "qcow2",
@@ -1857,7 +1857,7 @@
#
# Since: 1.3
#
-# Example:
+# .. qmp-example::
#
# -> { "execute": "block-commit",
# "arguments": { "device": "virtio0",
@@ -1895,7 +1895,7 @@
#
# Since: 1.6
#
-# Example:
+# .. qmp-example::
#
# -> { "execute": "drive-backup",
# "arguments": { "device": "drive0",
@@ -1921,7 +1921,7 @@
#
# Since: 2.3
#
-# Example:
+# .. qmp-example::
#
# -> { "execute": "blockdev-backup",
# "arguments": { "device": "src-id",
@@ -1945,7 +1945,7 @@
#
# Since: 2.0
#
-# Example:
+# .. qmp-example::
#
# -> { "execute": "query-named-block-nodes" }
# <- { "return": [ { "ro":false,
@@ -2126,7 +2126,7 @@
#
# Since: 1.3
#
-# Example:
+# .. qmp-example::
#
# -> { "execute": "
[PULL 10/14] docs/sphinx: add CSS styling for qmp-example directive
From: Harmonie Snow
Add CSS styling for qmp-example directives to increase readability and
consistently style all example blocks.
Signed-off-by: Harmonie Snow
Signed-off-by: John Snow
Acked-by: Markus Armbruster
Message-ID: <20240717021312.606116-6-js...@redhat.com>
Signed-off-by: Markus Armbruster
---
docs/sphinx-static/theme_overrides.css | 49 ++
1 file changed, 49 insertions(+)
diff --git a/docs/sphinx-static/theme_overrides.css
b/docs/sphinx-static/theme_overrides.css
index c70ef95128..965ecac54f 100644
--- a/docs/sphinx-static/theme_overrides.css
+++ b/docs/sphinx-static/theme_overrides.css
@@ -87,6 +87,55 @@ div[class^="highlight"] pre {
padding-bottom: 1px;
}
+/* qmp-example directive styling */
+
+.rst-content .admonition-example {
+/* do not apply the standard admonition background */
+background-color: transparent;
+border: solid #ffd2ed 1px;
+}
+
+.rst-content .admonition-example > .admonition-title:before {
+content: "â·";
+}
+
+.rst-content .admonition-example > .admonition-title {
+background-color: #5980a6;
+}
+
+.rst-content .admonition-example > div[class^="highlight"] {
+/* make code boxes take up the full width of the admonition w/o margin */
+margin-left: -12px;
+margin-right: -12px;
+
+border-top: 1px solid #ffd2ed;
+border-bottom: 1px solid #ffd2ed;
+border-left: 0px;
+border-right: 0px;
+}
+
+.rst-content .admonition-example > div[class^="highlight"]:nth-child(2) {
+/* If a code box is the second element in an example admonition,
+ * it is the first child after the title. let it sit flush against
+ * the title. */
+margin-top: -12px;
+border-top: 0px;
+}
+
+.rst-content .admonition-example > div[class^="highlight"]:last-child {
+/* If a code box is the final element in an example admonition, don't
+ * render margin below it; let it sit flush with the end of the
+ * admonition box */
+margin-bottom: -12px;
+border-bottom: 0px;
+}
+
+.rst-content .admonition-example .highlight {
+background-color: #fffafd;
+}
+
+/* end qmp-example styling */
+
@media screen {
/* content column
--
2.45.0
[PULL 00/14] QAPI patches patches for 2024-07-17
The following changes since commit e2f346aa98646e84eabe0256f89d08e89b1837cf: Merge tag 'sdmmc-20240716' of https://github.com/philmd/qemu into staging (2024-07-17 07:59:31 +1000) are available in the Git repository at: https://repo.or.cz/qemu/armbru.git tags/pull-qapi-2024-07-17 for you to fetch changes up to 3c5f6114d9ffc70bc9b1a7cc072a911f966d: qapi: remove "Example" doc section (2024-07-17 10:20:54 +0200) QAPI patches patches for 2024-07-17 Harmonie Snow (1): docs/sphinx: add CSS styling for qmp-example directive John Snow (7): docs/qapidoc: factor out do_parse() docs/qapidoc: create qmp-example directive docs/qapidoc: add QMP highlighting to annotated qmp-example blocks qapi: convert "Example" sections without titles qapi: convert "Example" sections with titles qapi: convert "Example" sections with longer prose qapi: remove "Example" doc section Markus Armbruster (6): qapi/qom: Document feature unstable of @x-vfio-user-server qapi/pci: Clean up documentation around PciDeviceClass qapi/machine: Clean up documentation around CpuInstanceProperties qapi/machine: Clarify query-uuid value when none has been specified qapi/sockets: Move deprecation note out of SocketAddress doc comment qapi/ui: Drop note on naming of SpiceQueryMouseMode docs/devel/qapi-code-gen.rst | 58 --- docs/sphinx-static/theme_overrides.css | 49 + docs/sphinx/qapidoc.py | 128 ++--- qapi/acpi.json | 4 +- qapi/block-core.json | 88 --- qapi/block.json| 57 --- qapi/char.json | 24 --- qapi/control.json | 8 +-- qapi/dump.json | 8 +-- qapi/machine-target.json | 2 +- qapi/machine.json | 86 -- qapi/migration.json| 90 --- qapi/misc-target.json | 22 +++--- qapi/misc.json | 32 - qapi/net.json | 22 +++--- qapi/pci.json | 8 +-- qapi/qdev.json | 10 +-- qapi/qom.json | 19 ++--- qapi/replay.json | 8 +-- qapi/rocker.json | 8 +-- qapi/run-state.json| 32 - qapi/sockets.json | 7 +- qapi/tpm.json | 6 +- qapi/trace.json| 4 +- qapi/transaction.json | 2 +- qapi/ui.json | 47 ++-- qapi/vfio.json | 2 +- qapi/virtio.json | 45 +++- qapi/yank.json | 4 +- scripts/qapi/parser.py | 10 ++- tests/qapi-schema/doc-good.json| 19 +++-- tests/qapi-schema/doc-good.out | 26 --- tests/qapi-schema/doc-good.txt | 23 +++--- 33 files changed, 610 insertions(+), 348 deletions(-) -- 2.45.0
[PULL 05/14] qapi/sockets: Move deprecation note out of SocketAddress doc comment
Doc comments are reference documentation for users of QMP.
SocketAddress's doc comment contains a deprecation note advising
developers to use SocketAddress for new code. Irrelevant for users of
QMP. Move the note out of the doc comment.
Signed-off-by: Markus Armbruster
Message-ID: <2024072228.2140606-5-arm...@redhat.com>
Reviewed-by: John Snow
Reviewed-by: Philippe Mathieu-Daudé
---
qapi/sockets.json | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/qapi/sockets.json b/qapi/sockets.json
index 4d78d2ccb7..e76fdb9925 100644
--- a/qapi/sockets.json
+++ b/qapi/sockets.json
@@ -179,10 +179,6 @@
#
# @type: Transport type
#
-# .. note:: This type is deprecated in favor of SocketAddress. The
-#difference between SocketAddressLegacy and SocketAddress is that
-#the latter has fewer ``{}`` on the wire.
-#
# Since: 1.3
##
{ 'union': 'SocketAddressLegacy',
@@ -193,6 +189,9 @@
'unix': 'UnixSocketAddressWrapper',
'vsock': 'VsockSocketAddressWrapper',
'fd': 'FdSocketAddressWrapper' } }
+# Note: This type is deprecated in favor of SocketAddress. The
+# difference between SocketAddressLegacy and SocketAddress is that the
+# latter has fewer ``{}`` on the wire.
##
# @SocketAddressType:
--
2.45.0
[PULL 08/14] docs/qapidoc: create qmp-example directive
From: John Snow
This is a directive that creates a syntactic sugar for creating
"Example" boxes very similar to the ones already used in the bitmaps.rst
document, please see e.g.
https://www.qemu.org/docs/master/interop/bitmaps.html#creation-block-dirty-bitmap-add
In its simplest form, when a custom title is not needed or wanted, and
the example body is *solely* a QMP example:
```
.. qmp-example::
{body}
```
is syntactic sugar for:
```
.. admonition:: Example:
.. code-block:: QMP
{body}
```
When a custom, plaintext title that describes the example is desired,
this form:
```
.. qmp-example::
:title: Defrobnification
{body}
```
Is syntactic sugar for:
```
.. admonition:: Example: Defrobnification
.. code-block:: QMP
{body}
```
Lastly, when Examples are multi-step processes that require non-QMP
exposition, have lengthy titles, or otherwise involve prose with rST
markup (lists, cross-references, etc), the most complex form:
```
.. qmp-example::
:annotated:
This example shows how to use `foo-command`::
{body}
For more information, please see `frobnozz`.
```
Is desugared to:
```
.. admonition:: Example:
This example shows how to use `foo-command`::
{body}
For more information, please see `frobnozz`.
```
Note that :annotated: and :title: options can be combined together, if
desired.
The primary benefit here being documentation source consistently using
the same directive for all forms of examples to ensure consistent visual
styling, and ensuring all relevant prose is visually grouped alongside
the code literal block.
Note that as of this commit, the code-block rST syntax "::" does not
apply QMP highlighting; you would need to use ".. code-block:: QMP". The
very next commit changes this behavior to assume all "::" code blocks
within this directive are QMP blocks.
Signed-off-by: John Snow
Acked-by: Markus Armbruster
Message-ID: <20240717021312.606116-4-js...@redhat.com>
Signed-off-by: Markus Armbruster
---
docs/sphinx/qapidoc.py | 55 ++
1 file changed, 55 insertions(+)
diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
index b3be82998a..11defcfa3f 100644
--- a/docs/sphinx/qapidoc.py
+++ b/docs/sphinx/qapidoc.py
@@ -27,6 +27,7 @@
import os
import re
import textwrap
+from typing import List
from docutils import nodes
from docutils.parsers.rst import Directive, directives
@@ -35,6 +36,7 @@
from qapi.gen import QAPISchemaVisitor
from qapi.schema import QAPISchema
+from sphinx.directives.code import CodeBlock
from sphinx.errors import ExtensionError
from sphinx.util.docutils import switch_source_input
from sphinx.util.nodes import nested_parse_with_titles
@@ -538,10 +540,63 @@ def run(self):
raise ExtensionError(str(err)) from err
+class QMPExample(CodeBlock, NestedDirective):
+"""
+Custom admonition for QMP code examples.
+
+When the :annotated: option is present, the body of this directive
+is parsed as normal rST instead. Code blocks must be explicitly
+written by the user, but this allows for intermingling explanatory
+paragraphs with arbitrary rST syntax and code blocks for more
+involved examples.
+
+When :annotated: is absent, the directive body is treated as a
+simple standalone QMP code block literal.
+"""
+
+required_argument = 0
+optional_arguments = 0
+has_content = True
+option_spec = {
+"annotated": directives.flag,
+"title": directives.unchanged,
+}
+
+def admonition_wrap(self, *content) -> List[nodes.Node]:
+title = "Example:"
+if "title" in self.options:
+title = f"{title} {self.options['title']}"
+
+admon = nodes.admonition(
+"",
+nodes.title("", title),
+*content,
+classes=["admonition", "admonition-example"],
+)
+return [admon]
+
+def run_annotated(self) -> List[nodes.Node]:
+content_node: nodes.Element = nodes.section()
+self.do_parse(self.content, content_node)
+return content_node.children
+
+def run(self) -> List[nodes.Node]:
+annotated = "annotated" in self.options
+
+if annotated:
+content_nodes = self.run_annotated()
+else:
+self.arguments = ["QMP"]
+content_nodes = super().run()
+
+return self.admonition_wrap(*content_nodes)
+
+
def setup(app):
"""Register qapi-doc directive with Sphinx"""
app.add_config_value("qapidoc_srctree", None, "env")
app.add_directive("qapi-doc", QAPIDocDirective)
+app.add_directive("qmp-example", QMPExample)
return {
"version": __version__,
--
2.45.0
[PULL 14/14] qapi: remove "Example" doc section
From: John Snow
Fully eliminate the "Example" sections in QAPI doc blocks now that they
have all been converted to arbitrary rST syntax using the
".. qmp-example::" directive. Update tests to match.
Migrating to the new syntax
---
The old "Example:" or "Examples:" section syntax is now caught as an
error, but "Example::" is stil permitted as explicit rST syntax for an
un-lexed, generic preformatted text block.
('Example' is not special in this case, any sentence that ends with "::"
will start an indented code block in rST.)
Arbitrary rST for Examples is now possible, but it's strongly
recommended that documentation authors use the ".. qmp-example::"
directive for consistent visual formatting in rendered HTML docs. The
":title:" directive option may be used to add extra information into the
title bar for the example. The ":annotated:" option can be used to write
arbitrary rST instead, with nested "::" blocks applying QMP formatting
where desired.
Other choices available are ".. code-block:: QMP" which will not create
an "Example:" box, or the short-form "::" code-block syntax which will
not apply QMP highlighting when used outside of the qmp-example
directive.
Why?
This patch has several benefits:
1. Example sections can now be written more arbitrarily, mixing
explanatory paragraphs and code blocks however desired.
2. Example sections can now use fully arbitrary rST.
3. All code blocks are now lexed and validated as QMP; increasing
usability of the docs and ensuring validity of example snippets.
(To some extent - This patch only gaurantees it lexes correctly, not
that it's valid under the JSON or QMP grammars. It will catch most
small mistakes, however.)
4. Each qmp-example can be titled or annotated independently without
bypassing the QMP lexer/validator.
(i.e. code blocks are now for *code* only, so we don't have to
sacrifice exposition for having lexically valid examples.)
NOTE: As with the "Notes" conversion (d461c279737), this patch (and the
three preceding) may change the rendering order for Examples in
the current generator. The forthcoming qapidoc rewrite will fix
this by always generating documentation in source order.
Signed-off-by: John Snow
Message-ID: <20240717021312.606116-10-js...@redhat.com>
Reviewed-by: Markus Armbruster
Signed-off-by: Markus Armbruster
---
docs/devel/qapi-code-gen.rst| 58 -
scripts/qapi/parser.py | 10 +-
tests/qapi-schema/doc-good.json | 19 +++
tests/qapi-schema/doc-good.out | 26 ++-
tests/qapi-schema/doc-good.txt | 23 ++---
5 files changed, 98 insertions(+), 38 deletions(-)
diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst
index ae97b335cb..583207a8ec 100644
--- a/docs/devel/qapi-code-gen.rst
+++ b/docs/devel/qapi-code-gen.rst
@@ -899,7 +899,7 @@ Documentation markup
Documentation comments can use most rST markup. In particular,
-a ``::`` literal block can be used for examples::
+a ``::`` literal block can be used for pre-formatted text::
# ::
#
@@ -995,8 +995,8 @@ line "Features:", like this::
# @feature: Description text
A tagged section begins with a paragraph that starts with one of the
-following words: "Since:", "Example:"/"Examples:", "Returns:",
-"Errors:", "TODO:". It ends with the start of a new section.
+following words: "Since:", "Returns:", "Errors:", "TODO:". It ends with
+the start of a new section.
The second and subsequent lines of tagged sections must be indented
like this::
@@ -1020,13 +1020,53 @@ detailing a relevant error condition. For example::
A "Since: x.y.z" tagged section lists the release that introduced the
definition.
-An "Example" or "Examples" section is rendered entirely
-as literal fixed-width text. "TODO" sections are not rendered at all
-(they are for developers, not users of QMP). In other sections, the
-text is formatted, and rST markup can be used.
+"TODO" sections are not rendered (they are for developers, not users of
+QMP). In other sections, the text is formatted, and rST markup can be
+used.
+
+QMP Examples can be added by using the ``.. qmp-example::``
+directive. In its simplest form, this can be used to contain a single
+QMP code block which accepts standard JSON syntax with additional server
+directionality indicators (``->`` and ``<-``), and elisions (``...``).
+
+Optionally, a plaintext title may be provided by using the ``:title:``
+directive option. If the title is omitted, the example title will
+default to "Example:".
+
+A simple QMP example::
+
+
[PULL 04/14] qapi/machine: Clarify query-uuid value when none has been specified
When no UUID has been specified, query-uuid returns
{"UUID": "----"}
The doc comment calls this "a null UUID", which I find less than
clear. RFC 9562 calls it "the nil UUID (all zeroes)", so use that
instead.
Signed-off-by: Markus Armbruster
Message-ID: <2024072228.2140606-4-arm...@redhat.com>
Reviewed-by: Michael S. Tsirkin
Reviewed-by: John Snow
[Wording improved, commit message adjusted]
Reviewed-by: Philippe Mathieu-Daudé
---
qapi/machine.json | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/qapi/machine.json b/qapi/machine.json
index 50ff102d56..eb2fd01e95 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -305,9 +305,8 @@
#
# Since: 0.14
#
-# .. note:: If no UUID was specified for the guest, a null UUID is
-#returned.
-#
+# .. note:: If no UUID was specified for the guest, the nil UUID (all
+#zeroes) is returned.
##
{ 'struct': 'UuidInfo', 'data': {'UUID': 'str'} }
--
2.45.0
[PULL 12/14] qapi: convert "Example" sections with titles
From: John Snow
When an Example section has a brief explanation, convert it to a
qmp-example:: section using the :title: option.
Rule of thumb: If the title can fit on a single line and requires no rST
markup, it's a good candidate for using the :title: option of
qmp-example.
In this patch, trailing punctuation is removed from the title section
for consistent headline aesthetics. In just one case, specifics of the
example are removed to make the title read better.
See commit-4: "docs/qapidoc: create qmp-example directive", for a
detailed explanation of this custom directive syntax.
See commit+2: "qapi: remove "Example" doc section" for a detailed
explanation of why.
Signed-off-by: John Snow
Reviewed-by: Markus Armbruster
Message-ID: <20240717021312.606116-8-js...@redhat.com>
Signed-off-by: Markus Armbruster
---
qapi/block-core.json | 24
qapi/block.json | 13 ++---
qapi/migration.json | 25 ++---
qapi/qom.json| 8
qapi/ui.json | 11 ++-
qapi/virtio.json | 19 ++-
6 files changed, 52 insertions(+), 48 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 84a020aa9e..f400b334c8 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -5881,9 +5881,8 @@
#
# Since: 2.7
#
-# Examples:
-#
-# 1. Add a new node to a quorum
+# .. qmp-example::
+#:title: Add a new node to a quorum
#
# -> { "execute": "blockdev-add",
# "arguments": {
@@ -5897,7 +5896,8 @@
# "node": "new_node" } }
# <- { "return": {} }
#
-# 2. Delete a quorum's node
+# .. qmp-example::
+#:title: Delete a quorum's node
#
# -> { "execute": "x-blockdev-change",
# "arguments": { "parent": "disk1",
@@ -5933,16 +5933,16 @@
#
# Since: 2.12
#
-# Examples:
-#
-# 1. Move a node into an IOThread
+# .. qmp-example::
+#:title: Move a node into an IOThread
#
# -> { "execute": "x-blockdev-set-iothread",
# "arguments": { "node-name": "disk1",
# "iothread": "iothread0" } }
# <- { "return": {} }
#
-# 2. Move a node into the main loop
+# .. qmp-example::
+#:title: Move a node into the main loop
#
# -> { "execute": "x-blockdev-set-iothread",
# "arguments": { "node-name": "disk1",
@@ -6018,16 +6018,16 @@
#
# Since: 2.0
#
-# Examples:
-#
-# 1. Read operation
+# .. qmp-example::
+#:title: Read operation
#
# <- { "event": "QUORUM_REPORT_BAD",
# "data": { "node-name": "node0", "sector-num": 345435,
"sectors-count": 5,
#"type": "read" },
# "timestamp": { "seconds": 1344522075, "microseconds": 745528 } }
#
-# 2. Flush operation
+# .. qmp-example::
+#:title: Flush operation
#
# <- { "event": "QUORUM_REPORT_BAD",
# "data": { "node-name": "node0", "sector-num": 0, "sectors-count":
2097120,
diff --git a/qapi/block.json b/qapi/block.json
index c8e52bc2d2..5ddd061e96 100644
--- a/qapi/block.json
+++ b/qapi/block.json
@@ -342,9 +342,8 @@
#
# Since: 2.5
#
-# Examples:
-#
-# 1. Change a removable medium
+# .. qmp-example::
+#:title: Change a removable medium
#
# -> { "execute": "blockdev-change-medium",
# "arguments": { "id": "ide0-1-0",
@@ -352,7 +351,8 @@
# "format": "raw" } }
# <- { "return": {} }
#
-# 2. Load a read-only medium into a writable drive
+# .. qmp-example::
+#:title: Load a read-only medium into a writable drive
#
# -> { "execute": "blockdev-change-medium",
# "arguments": { "id": "floppyA",
@@ -577,9 +577,8 @@
# "boundaries-write": [1000, 5000] } }
# <- { "return": {} }
#
-# Example:
-#
-# Remove all latency histograms:
+# .. qmp-example::
+#:title: Remove all latency histograms
#
# -> { "execute": "block-latency-histogram-set",
# "arguments": { "id": "drive0" } }
diff --git a/qapi/migration.json b/qapi/migration.json
index 3c65720238..74968a1417 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -287,14 +287,14 @@
#
# Since: 0.14
#
-# Examples:
-#
-# 1. Before the first migration
+# .. qmp-ex
[PULL 13/14] qapi: convert "Example" sections with longer prose
From: John Snow
These examples require longer explanations or have explanations that
require markup to look reasonable when rendered and so use the longer
form of the ".. qmp-example::" directive.
By using the :annotated: option, the content in the example block is
assumed *not* to be a code block literal and is instead parsed as normal
rST - with the exception that any code literal blocks after `::` will
assumed to be a QMP code literal block.
Note: There's one title-less conversion in this patch that comes along
for the ride because it's part of a larger "Examples" block that was
better to convert all at once.
See commit-5: "docs/qapidoc: create qmp-example directive", for a
detailed explanation of this custom directive syntax.
See commit+1: "qapi: remove "Example" doc section" for a detailed
explanation of why.
Signed-off-by: John Snow
Reviewed-by: Markus Armbruster
Message-ID: <20240717021312.606116-9-js...@redhat.com>
Signed-off-by: Markus Armbruster
---
qapi/block.json | 30 +++---
qapi/machine.json | 30 --
qapi/migration.json | 7 +--
qapi/virtio.json| 24 ++--
4 files changed, 62 insertions(+), 29 deletions(-)
diff --git a/qapi/block.json b/qapi/block.json
index 5ddd061e96..ce9490a367 100644
--- a/qapi/block.json
+++ b/qapi/block.json
@@ -545,31 +545,37 @@
#
# Since: 4.0
#
-# Example:
+# .. qmp-example::
+#:annotated:
#
-# Set new histograms for all io types with intervals
-# [0, 10), [10, 50), [50, 100), [100, +inf):
+#Set new histograms for all io types with intervals
+#[0, 10), [10, 50), [50, 100), [100, +inf)::
#
# -> { "execute": "block-latency-histogram-set",
# "arguments": { "id": "drive0",
# "boundaries": [10, 50, 100] } }
# <- { "return": {} }
#
-# Example:
+# .. qmp-example::
+#:annotated:
#
-# Set new histogram only for write, other histograms will remain
-# not changed (or not created):
+#Set new histogram only for write, other histograms will remain
+#not changed (or not created)::
#
# -> { "execute": "block-latency-histogram-set",
# "arguments": { "id": "drive0",
# "boundaries-write": [10, 50, 100] } }
# <- { "return": {} }
#
-# Example:
+# .. qmp-example::
+#:annotated:
#
-# Set new histograms with the following intervals:
-# read, flush: [0, 10), [10, 50), [50, 100), [100, +inf)
-# write: [0, 1000), [1000, 5000), [5000, +inf)
+#Set new histograms with the following intervals:
+#
+#- read, flush: [0, 10), [10, 50), [50, 100), [100, +inf)
+#- write: [0, 1000), [1000, 5000), [5000, +inf)
+#
+#::
#
# -> { "execute": "block-latency-histogram-set",
# "arguments": { "id": "drive0",
@@ -578,7 +584,9 @@
# <- { "return": {} }
#
# .. qmp-example::
-#:title: Remove all latency histograms
+#:annotated:
+#
+#Remove all latency histograms::
#
# -> { "execute": "block-latency-histogram-set",
# "arguments": { "id": "drive0" } }
diff --git a/qapi/machine.json b/qapi/machine.json
index 193b2b7c16..f9ea6b3e97 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1045,10 +1045,11 @@
#
# Since: 2.7
#
-# Examples:
+# .. qmp-example::
+#:annotated:
#
-# For pseries machine type started with -smp 2,cores=2,maxcpus=4
-# -cpu POWER8:
+#For pseries machine type started with
+#``-smp 2,cores=2,maxcpus=4 -cpu POWER8``::
#
# -> { "execute": "query-hotpluggable-cpus" }
# <- {"return": [
@@ -1058,7 +1059,10 @@
#"vcpus-count": 1, "qom-path": "/machine/unattached/device[0]"}
#]}
#
-# For pc machine type started with -smp 1,maxcpus=2:
+# .. qmp-example::
+#:annotated:
+#
+#For pc machine type started with ``-smp 1,maxcpus=2``::
#
# -> { "execute": "query-hotpluggable-cpus" }
# <- {"return": [
@@ -1073,8 +1077,11 @@
# }
#]}
#
-# For s390x-virtio-ccw machine type started with -smp 1,maxcpus=2
-# -cpu qemu (Since: 2.11):
+# .. qmp-example::
+#:annotated:
+#
+#For s390x-virtio-ccw machine type started with
+#``-smp 1,maxcpus=2 -cpu qemu`` (Since: 2.11)::
#
# -> { "execute": "query-hotpluggable-cpus" }
# <- {"return": [
@@ -1128,12 +1135,15 @@
#
# Since: 0.14
#
-# Example:
+# .. qmp-example::
+#:annotated:
#
-# -> { "execute": "balloon", "argu
[PULL 09/14] docs/qapidoc: add QMP highlighting to annotated qmp-example blocks
From: John Snow
For any code literal blocks inside of a qmp-example directive, apply and
enforce the QMP lexer/highlighter to those blocks.
This way, you won't need to write:
```
.. qmp-example::
:annotated:
Blah blah
.. code-block:: QMP
-> { "lorem": "ipsum" }
```
But instead, simply:
```
.. qmp-example::
:annotated:
Blah blah::
-> { "lorem": "ipsum" }
```
Once the directive block is exited, whatever the previous default
highlight language was will be restored; localizing the forced QMP
lexing to exclusively this directive.
Note, if the default language is *already* QMP, this directive will not
generate and restore redundant highlight configuration nodes. We may
well decide that the default language ought to be QMP for any QAPI
reference pages, but this way the directive behaves consistently no
matter where it is used.
Signed-off-by: John Snow
Acked-by: Markus Armbruster
Message-ID: <20240717021312.606116-5-js...@redhat.com>
Signed-off-by: Markus Armbruster
---
docs/sphinx/qapidoc.py | 53 ++
1 file changed, 49 insertions(+), 4 deletions(-)
diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
index 11defcfa3f..738b2450fb 100644
--- a/docs/sphinx/qapidoc.py
+++ b/docs/sphinx/qapidoc.py
@@ -26,6 +26,7 @@
import os
import re
+import sys
import textwrap
from typing import List
@@ -36,6 +37,7 @@
from qapi.gen import QAPISchemaVisitor
from qapi.schema import QAPISchema
+from sphinx import addnodes
from sphinx.directives.code import CodeBlock
from sphinx.errors import ExtensionError
from sphinx.util.docutils import switch_source_input
@@ -545,10 +547,10 @@ class QMPExample(CodeBlock, NestedDirective):
Custom admonition for QMP code examples.
When the :annotated: option is present, the body of this directive
-is parsed as normal rST instead. Code blocks must be explicitly
-written by the user, but this allows for intermingling explanatory
-paragraphs with arbitrary rST syntax and code blocks for more
-involved examples.
+is parsed as normal rST, but with any '::' code blocks set to use
+the QMP lexer. Code blocks must be explicitly written by the user,
+but this allows for intermingling explanatory paragraphs with
+arbitrary rST syntax and code blocks for more involved examples.
When :annotated: is absent, the directive body is treated as a
simple standalone QMP code block literal.
@@ -562,6 +564,33 @@ class QMPExample(CodeBlock, NestedDirective):
"title": directives.unchanged,
}
+def _highlightlang(self) -> addnodes.highlightlang:
+"""Return the current highlightlang setting for the document"""
+node = None
+doc = self.state.document
+
+if hasattr(doc, "findall"):
+# docutils >= 0.18.1
+for node in doc.findall(addnodes.highlightlang):
+pass
+else:
+for elem in doc.traverse():
+if isinstance(elem, addnodes.highlightlang):
+node = elem
+
+if node:
+return node
+
+# No explicit directive found, use defaults
+node = addnodes.highlightlang(
+lang=self.env.config.highlight_language,
+force=False,
+# Yes, Sphinx uses this value to effectively disable line
+# numbers and not 0 or None or -1 or something. ¯\_(ã)_/¯
+linenothreshold=sys.maxsize,
+)
+return node
+
def admonition_wrap(self, *content) -> List[nodes.Node]:
title = "Example:"
if "title" in self.options:
@@ -576,8 +605,24 @@ def admonition_wrap(self, *content) -> List[nodes.Node]:
return [admon]
def run_annotated(self) -> List[nodes.Node]:
+lang_node = self._highlightlang()
+
content_node: nodes.Element = nodes.section()
+
+# Configure QMP highlighting for "::" blocks, if needed
+if lang_node["lang"] != "QMP":
+content_node += addnodes.highlightlang(
+lang="QMP",
+force=False, # "True" ignores lexing errors
+linenothreshold=lang_node["linenothreshold"],
+)
+
self.do_parse(self.content, content_node)
+
+# Restore prior language highlighting, if needed
+if lang_node["lang"] != "QMP":
+content_node += addnodes.highlightlang(**lang_node.attributes)
+
return content_node.children
def run(self) -> List[nodes.Node]:
--
2.45.0
[PULL 02/14] qapi/pci: Clean up documentation around PciDeviceClass
PciDeviceInfo's doc comment has a note on PciDeviceClass member @desc.
Since the note applies always, not just within PciDeviceInfo, merge it
into PciDeviceClass's description of member @desc.
Signed-off-by: Markus Armbruster
Message-ID: <2024072228.2140606-2-arm...@redhat.com>
Reviewed-by: Michael S. Tsirkin
Reviewed-by: John Snow
---
qapi/pci.json | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/qapi/pci.json b/qapi/pci.json
index 8287d15dd0..97179920fb 100644
--- a/qapi/pci.json
+++ b/qapi/pci.json
@@ -93,7 +93,8 @@
#
# Information about the Class of a PCI device
#
-# @desc: a string description of the device's class
+# @desc: a string description of the device's class (not stable, and
+# should only be treated as informational)
#
# @class: the class code of the device
#
@@ -146,9 +147,6 @@
#
# @regions: a list of the PCI I/O regions associated with the device
#
-# .. note:: The contents of @class_info.desc are not stable and should
-#only be treated as informational.
-#
# Since: 0.14
##
{ 'struct': 'PciDeviceInfo',
--
2.45.0
[PULL 07/14] docs/qapidoc: factor out do_parse()
From: John Snow Factor out the compatibility parser helper into a base class, so it can be shared by other directives. Signed-off-by: John Snow Reviewed-by: Markus Armbruster Message-ID: <20240717021312.606116-3-js...@redhat.com> Signed-off-by: Markus Armbruster --- docs/sphinx/qapidoc.py | 32 +++- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py index 62b39833ca..b3be82998a 100644 --- a/docs/sphinx/qapidoc.py +++ b/docs/sphinx/qapidoc.py @@ -481,7 +481,25 @@ def visit_module(self, name): super().visit_module(name) -class QAPIDocDirective(Directive): +class NestedDirective(Directive): +def run(self): +raise NotImplementedError + +def do_parse(self, rstlist, node): +""" +Parse rST source lines and add them to the specified node + +Take the list of rST source lines rstlist, parse them as +rST, and add the resulting docutils nodes as children of node. +The nodes are parsed in a way that allows them to include +subheadings (titles) without confusing the rendering of +anything else. +""" +with switch_source_input(self.state, rstlist): +nested_parse_with_titles(self.state, rstlist, node) + + +class QAPIDocDirective(NestedDirective): """Extract documentation from the specified QAPI .json file""" required_argument = 1 @@ -519,18 +537,6 @@ def run(self): # so they are displayed nicely to the user raise ExtensionError(str(err)) from err -def do_parse(self, rstlist, node): -"""Parse rST source lines and add them to the specified node - -Take the list of rST source lines rstlist, parse them as -rST, and add the resulting docutils nodes as children of node. -The nodes are parsed in a way that allows them to include -subheadings (titles) without confusing the rendering of -anything else. -""" -with switch_source_input(self.state, rstlist): -nested_parse_with_titles(self.state, rstlist, node) - def setup(app): """Register qapi-doc directive with Sphinx""" -- 2.45.0
Re: [PATCH v2 0/9] qapi: convert example sections to qmp-example rST directives
John Snow writes: > This patchset focuses on converting example sections to rST directives > using a new `.. qmp-example::` directive. Queued, thanks!
Re: [PATCH 0/5] qapi: Doc comment cleanups
Queued.
Re: [PATCH] qapi/qom: Document feature unstable of @x-vfio-user-server
Markus Armbruster writes: > Commit 8f9a9259d32c added ObjectType member @x-vfio-user-server with > feature unstable, but neglected to explain why it is unstable. Do > that now. > > Fixes: 8f9a9259d32c (vfio-user: define vfio-user-server object) > Cc: Elena Ufimtseva > Cc: John G Johnson > Cc: Jagannathan Raman > Cc: qemu-sta...@nongnu.org > Signed-off-by: Markus Armbruster > --- > qapi/qom.json | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/qapi/qom.json b/qapi/qom.json > index 8bd299265e..3c0f8c633d 100644 > --- a/qapi/qom.json > +++ b/qapi/qom.json > @@ -1012,7 +1012,8 @@ > # > # Features: > # > -# @unstable: Member @x-remote-object is experimental. > +# @unstable: Members @x-remote-object and @x-vfio-user-server are > +# experimental. Second line needs to be indented. Fixed in my tree. > # > # Since: 6.0 > ## Queued.
Re: [PATCH v2 9/9] qapi: remove "Example" doc section
John Snow writes:
> Fully eliminate the "Example" sections in QAPI doc blocks now that they
> have all been converted to arbitrary rST syntax using the
> ".. qmp-example::" directive. Update tests to match.
>
> Migrating to the new syntax
> ---
>
> The old "Example:" or "Examples:" section syntax is now caught as an
> error, but "Example::" is stil permitted as explicit rST syntax for an
> un-lexed, generic preformatted text block.
>
> ('Example' is not special in this case, any sentence that ends with "::"
> will start an indented code block in rST.)
>
> Arbitrary rST for Examples is now possible, but it's strongly
> recommended that documentation authors use the ".. qmp-example::"
> directive for consistent visual formatting in rendered HTML docs. The
> ":title:" directive option may be used to add extra information into the
> title bar for the example. The ":annotated:" option can be used to write
> arbitrary rST instead, with nested "::" blocks applying QMP formatting
> where desired.
>
> Other choices available are ".. code-block:: QMP" which will not create
> an "Example:" box, or the short-form "::" code-block syntax which will
> not apply QMP highlighting when used outside of the qmp-example
> directive.
>
> Why?
>
>
> This patch has several benefits:
>
> 1. Example sections can now be written more arbitrarily, mixing
>explanatory paragraphs and code blocks however desired.
>
> 2. Example sections can now use fully arbitrary rST.
>
> 3. All code blocks are now lexed and validated as QMP; increasing
>usability of the docs and ensuring validity of example snippets.
>
>(To some extent - This patch only gaurantees it lexes correctly, not
>that it's valid under the JSON or QMP grammars. It will catch most
>small mistakes, however.)
>
> 4. Each qmp-example can be titled or annotated independently without
>bypassing the QMP lexer/validator.
>
>(i.e. code blocks are now for *code* only, so we don't have to
>sacrifice exposition for having lexically valid examples.)
>
> NOTE: As with the "Notes" conversion (d461c279737), this patch (and the
> three preceding) may change the rendering order for Examples in
> the current generator. The forthcoming qapidoc rewrite will fix
> this by always generating documentation in source order.
>
> Signed-off-by: John Snow
Reviewed-by: Markus Armbruster
Re: [PATCH v2 6/9] qapi: convert "Example" sections without titles
John Snow writes:
> Use the no-option form of ".. qmp-example::" to convert any Examples
> that do not have any form of caption or explanation whatsoever. Note
> that in a few cases, example sections are split into two or more
> separate example blocks. This is only done stylistically to create a
> delineation between two or more logically independent examples.
>
> See commit-3: "docs/qapidoc: create qmp-example directive", for a
> detailed explanation of this custom directive syntax.
>
> See commit+3: "qapi: remove "Example" doc section" for a detailed
> explanation of why.
>
> Note: an empty "TODO" line was added to announce-self to keep the
> example from floating up into the body; this will be addressed more
> rigorously in the new qapidoc generator.
>
> Signed-off-by: John Snow
[...]
> diff --git a/qapi/run-state.json b/qapi/run-state.json
> index 4d40c88876c..ab7116680b3 100644
> --- a/qapi/run-state.json
> +++ b/qapi/run-state.json
[...]
> @@ -469,7 +469,7 @@
> #
> # Since: 9.1
> #
> -# Example:
> +# ..qmp-example::
Lacks a space, output is messed up. Can fix in my tree.
> #
> # <- { "event": "GUEST_PVSHUTDOWN",
> # "timestamp": { "seconds": 1648245259, "microseconds": 893771 } }
[...]
R-by stands.
Re: [PATCH 3/5] qapi/machine: Clarify query-uuid value when none has been specified
John Snow writes:
> On Thu, Jul 11, 2024, 7:22 AM Markus Armbruster wrote:
>
>> When no UUID has been specified, query-uuid returns
>>
>> {"UUID": "----"}
>>
>> The doc comment calls this "a null UUID", which I find less than
>> clear. Change it to "an all-zero UUID".
>>
>
> Technically it's a "nil UUID";
> https://datatracker.ietf.org/doc/html/rfc9562#name-nil-uuid
>
> If you wanted to be pedantic, you could say "the nil UUID (all zeroes) is
> returned"
Sold!
> but your rephrasing is clear even w/o using the standard name, so I'm fine
> either way.
>
>
>> Signed-off-by: Markus Armbruster
>>
>
> Reviewed-by: John Snow
Thank you!
Re: [PATCH 04/14] qapi: add a 'command-features' pragma
Sorry for the delay; too many distractions, and I needed a good think.
Daniel P. Berrangé writes:
> On Fri, Jul 12, 2024 at 10:50:54AM +0200, Markus Armbruster wrote:
>> Daniel P. Berrangé writes:
>>
>> > On Fri, Jul 12, 2024 at 10:07:34AM +0200, Markus Armbruster wrote:
>> >> Daniel P. Berrangé writes:
>> >>
>> >> > The 'command-features' pragma allows for defining additional
>> >> > special features that are unique to a particular QAPI schema
>> >> > instance and its implementation.
>> >>
>> >> So far, we have special features (predefined, known to the generator and
>> >> treated specially), and normal features (user-defined, not known to the
>> >> generator). You create a new kind in between: user-defined, not known
>> >> to the generator, yet treated specially (I guess?). Hmm.
>> >>
>> >> Could you at least hint at indented use here? What special treatment do
>> >> you have in mind?
>> >
>> > Essentially, these features are a way to attach metadata to commands that
>> > the server side impl can later query. This eliminates the need to hardcode
>> > lists of commands, such as in QGA which hardcodes a list of commands which
>> > are safe to use when filesystems are frozen. This is illustrated later in
>> > this series.
>>
>> Please update docs/devel/qapi-code-gen.rst section "Pragma directives",
>> and maybe section "Features".
Second thoughts; see below.
>> I'm not sure conflating the new kind of feature with existing special
>> features is a good idea. I need to review more of the series before I
>> can make up my mind.
>
> I originally implemented a completely separate 'tags' concept in the
> QAPI parser, before deciding I was just re-inventing 'features' for
> no obvious benefit.
>
> The other nice thing about using features is that these are exposed
> in the schema and docs. With the 'fsfreeze' restriction in code,
> there's no formal docs of what commands are allowed when frozen, and
> this is also not exposed in QAPI schema to apps. Using 'features'
> we get all that as standard.
When you need to tack a mark to one or more things for whatever purpose
*and* expose it to QMP clients, then features make sense. This is the
case here.
Initially, features were strictly an external interface annotation, and
were not meant to be used within QEMU. All features were user-defined.
This changed when I created configurable policy for deprecated and
unstable management interfaces: the policy engine needs to check for
features 'deprecated' and 'unstable'. Since the policy engine is partly
implemented in generated code, these two features need to be baked into
the generator. This makes them special.
You need less than that: a predicate "does have " for
certain features, ideally without baking them into the generator.
The command registry already tracks each command's special features for
use by the policy engine. Obvious idea: also track the features you
want to pass to the predicate.
Your series adds tracking for exactly the features you need:
* Enumerate them in the schema with new pragma command-features
Missing: documentation for the pragma.
* Generate an extension QapiSpecialFeatureCustom of existing enum
QapiSpecialFeature, which is not generated. The latter is in
qapi/util.h, the former in ${prefix}qapi-init-commands.h.
* Mark these features special for commands only, so existing registry
machinery tracks them. Do *not* make them special elsewhere, because
that would break things.
Feels like a hack. Minor trap: if you use the same feature in
multiple schemas, multiple generated headers will define the same enum
constant, possibly with different values. If you manage to include
the wrong header *and* the value differs there, you'll likely lose
hair.
* Missing: tests.
I think we can avoid supplying most of the missing bits. The main QAPI
schema uses five features: deprecated, unstable,
allow-write-only-overlay, dynamic-auto-read-only, fdset. The QGA QAPI
schema uses four, namely the four you add in this series. Why not track
all features, and dispense with the pragma? Like this:
* Change type of feature bitsets to uint64_t (it's unsigned now).
* Error out if a schema has more than 64 features.
* Move enum QapiSpecialFeature into a new generated header.
* Generate a member for each feature, not just the two predefined ones.
* Pass all command features to the registry, not just the special ones.
* Recommended: do the same elsewhere, i.e. replace
gen_special_features() by gen_features().
Thoughts?
PS: I think I spotted a number of additional minor issues in your
series, but I won't describe them here, as I hope the simplification
will make most of them go away.
Re: [PATCH v1] target/s390x: filter deprecated features based on model expansion type
Collin Walling writes: > On 7/12/24 1:23 AM, Markus Armbruster wrote: >> Collin Walling writes: >> >>> It is beneficial to provide an interface to retrieve *all* deprecated >>> features in one go. Management applications will need this information >>> to determine which features need to be disabled regardless of the >>> host-model's capabilities. >>> >>> To remedy this, deprecated features are only filtered during a static >>> expansion. All deperecated features are reported on a full expansion. >>> >>> Suggested-by: Jiri Denemark >>> Signed-off-by: Collin Walling >> >> Which command(s) exactly are affected? >> > > The query-cpu-model-expansion result will now report all deprecated > features when a user requests a full expansion. The inputs are not > affects, but the output is modified. I will make this more concise on > the v2 commit message. Yes, please. Consider including an example. >> Do they need a doc update? >> > > Yes, I forgot to add this. This is what is currently documented: > > ## > # @CpuModelInfo: > # > ... > # > # @deprecated-props: a list of properties that are flagged as deprecated > # by the CPU vendor. These props are a subset of the full model's > # definition list of properties. (since 9.1) > # > > I will change to: > > # > # @deprecated-props: a list of properties that are flagged as deprecated > # by the CPU vendor. These are a subset of the reported @props. > # (since 9.1) > # Hasn't made it into a release, so we don't have to document the old behavior. Fortunate! Separate sentences with two spaces for consistency, please. > (I will also the correct typo in my commit message). > > [...] > > Thanks!
Re: [PATCH 00/14] Improve mechanism for configuring allowed commands
Hi Daniel, got a public branch I could pull?
Re: [PATCH 04/14] qapi: add a 'command-features' pragma
Daniel P. Berrangé writes: > On Fri, Jul 12, 2024 at 10:07:34AM +0200, Markus Armbruster wrote: >> Daniel P. Berrangé writes: >> >> > The 'command-features' pragma allows for defining additional >> > special features that are unique to a particular QAPI schema >> > instance and its implementation. >> >> So far, we have special features (predefined, known to the generator and >> treated specially), and normal features (user-defined, not known to the >> generator). You create a new kind in between: user-defined, not known >> to the generator, yet treated specially (I guess?). Hmm. >> >> Could you at least hint at indented use here? What special treatment do >> you have in mind? > > Essentially, these features are a way to attach metadata to commands that > the server side impl can later query. This eliminates the need to hardcode > lists of commands, such as in QGA which hardcodes a list of commands which > are safe to use when filesystems are frozen. This is illustrated later in > this series. Please update docs/devel/qapi-code-gen.rst section "Pragma directives", and maybe section "Features". I'm not sure conflating the new kind of feature with existing special features is a good idea. I need to review more of the series before I can make up my mind.
Re: [PATCH 04/14] qapi: add a 'command-features' pragma
Daniel P. Berrangé writes: > The 'command-features' pragma allows for defining additional > special features that are unique to a particular QAPI schema > instance and its implementation. So far, we have special features (predefined, known to the generator and treated specially), and normal features (user-defined, not known to the generator). You create a new kind in between: user-defined, not known to the generator, yet treated specially (I guess?). Hmm. Could you at least hint at indented use here? What special treatment do you have in mind? > Signed-off-by: Daniel P. Berrangé > --- > scripts/qapi/parser.py | 2 ++ > scripts/qapi/source.py | 2 ++ > 2 files changed, 4 insertions(+) > > diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py > index 7b13a583ac..36a9046243 100644 > --- a/scripts/qapi/parser.py > +++ b/scripts/qapi/parser.py > @@ -243,6 +243,8 @@ def check_list_str(name: str, value: object) -> List[str]: > pragma.documentation_exceptions = check_list_str(name, value) > elif name == 'member-name-exceptions': > pragma.member_name_exceptions = check_list_str(name, value) > +elif name == 'command-features': > +pragma.command_features = check_list_str(name, value) > else: > raise QAPISemError(info, "unknown pragma '%s'" % name) > > diff --git a/scripts/qapi/source.py b/scripts/qapi/source.py > index 7b379fdc92..07c2958ac4 100644 > --- a/scripts/qapi/source.py > +++ b/scripts/qapi/source.py > @@ -28,6 +28,8 @@ def __init__(self) -> None: > self.documentation_exceptions: List[str] = [] > # Types whose member names may violate case conventions > self.member_name_exceptions: List[str] = [] > +# Arbitrary extra features recorded against commands > +self.command_features: List[str] = [] > > > class QAPISourceInfo:
Re: [PATCH 03/14] qapi: cope with special feature names containing a '-'
Daniel P. Berrangé writes:
> When we shortly allow custom special feature names to be defined, it
> will be valid to include a '-', which must be translated to a '_'
> when generating code.
>
> Signed-off-by: Daniel P. Berrangé
> ---
> scripts/qapi/gen.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
> index 9c590a1c2e..650efc59ed 100644
> --- a/scripts/qapi/gen.py
> +++ b/scripts/qapi/gen.py
> @@ -41,7 +41,7 @@
>
>
> def gen_special_features(features: Sequence[QAPISchemaFeature]) -> str:
> -special_features = [f"1u << QAPI_FEATURE_{feat.name.upper()}"
> +special_features = [f"1u <<
> QAPI_FEATURE_{feat.name.upper().replace('-','_')}"
> for feat in features if feat.is_special()]
> return ' | '.join(special_features) or '0'
I'd prefer something like
f"1 << {c_enum_const('QAPI_FEATURE', feat.name)}"
Re: [PATCH v1] target/s390x: filter deprecated features based on model expansion type
Collin Walling writes:
> It is beneficial to provide an interface to retrieve *all* deprecated
> features in one go. Management applications will need this information
> to determine which features need to be disabled regardless of the
> host-model's capabilities.
>
> To remedy this, deprecated features are only filtered during a static
> expansion. All deperecated features are reported on a full expansion.
>
> Suggested-by: Jiri Denemark
> Signed-off-by: Collin Walling
Which command(s) exactly are affected?
Do they need a doc update?
> ---
> target/s390x/cpu_models_sysemu.c | 10 +-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/target/s390x/cpu_models_sysemu.c
> b/target/s390x/cpu_models_sysemu.c
> index 977fbc6522..76d15f2e4d 100644
> --- a/target/s390x/cpu_models_sysemu.c
> +++ b/target/s390x/cpu_models_sysemu.c
> @@ -211,7 +211,15 @@ static void cpu_info_from_model(CpuModelInfo *info,
> const S390CPUModel *model,
> bitmap_zero(bitmap, S390_FEAT_MAX);
> s390_get_deprecated_features(bitmap);
>
> -bitmap_and(bitmap, bitmap, model->def->full_feat, S390_FEAT_MAX);
> +/*
> + * For static model expansion, filter out deprecated features that are
> + * not a subset of the model's feature set. Otherwise, report the entire
> + * deprecated features list.
> + */
> +if (delta_changes) {
> +bitmap_and(bitmap, bitmap, model->def->full_feat, S390_FEAT_MAX);
> +}
> +
> s390_feat_bitmap_to_ascii(bitmap, >deprecated_props,
> list_add_feat);
> info->has_deprecated_props = !!info->deprecated_props;
> }
[PATCH 4/5] qapi/sockets: Move deprecation note out of SocketAddress doc comment
Doc comments are reference documentation for users of QMP.
SocketAddress's doc comment contains a deprecation note advising
developers to use SocketAddress for new code. Irrelevant for users of
QMP. Move the note out of the doc comment.
Signed-off-by: Markus Armbruster
---
qapi/sockets.json | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/qapi/sockets.json b/qapi/sockets.json
index 4d78d2ccb7..e76fdb9925 100644
--- a/qapi/sockets.json
+++ b/qapi/sockets.json
@@ -179,10 +179,6 @@
#
# @type: Transport type
#
-# .. note:: This type is deprecated in favor of SocketAddress. The
-#difference between SocketAddressLegacy and SocketAddress is that
-#the latter has fewer ``{}`` on the wire.
-#
# Since: 1.3
##
{ 'union': 'SocketAddressLegacy',
@@ -193,6 +189,9 @@
'unix': 'UnixSocketAddressWrapper',
'vsock': 'VsockSocketAddressWrapper',
'fd': 'FdSocketAddressWrapper' } }
+# Note: This type is deprecated in favor of SocketAddress. The
+# difference between SocketAddressLegacy and SocketAddress is that the
+# latter has fewer ``{}`` on the wire.
##
# @SocketAddressType:
--
2.45.0
[PATCH 3/5] qapi/machine: Clarify query-uuid value when none has been specified
When no UUID has been specified, query-uuid returns
{"UUID": "----"}
The doc comment calls this "a null UUID", which I find less than
clear. Change it to "an all-zero UUID".
Signed-off-by: Markus Armbruster
---
qapi/machine.json | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/qapi/machine.json b/qapi/machine.json
index 50ff102d56..f40427f21a 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -305,9 +305,8 @@
#
# Since: 0.14
#
-# .. note:: If no UUID was specified for the guest, a null UUID is
-#returned.
-#
+# .. note:: If no UUID was specified for the guest, an all-zero UUID
+#is returned.
##
{ 'struct': 'UuidInfo', 'data': {'UUID': 'str'} }
--
2.45.0
[PATCH 2/5] qapi/machine: Clean up documentation around CpuInstanceProperties
CpuInstanceProperties' doc comment describes its members as properties
to be passed to device_add when hot-plugging a CPU.
This was in fact the initial use of this type, with
query-hotpluggable-cpus: letting management applications find out what
properties need to be passed with device_add to hot-plug a CPU.
We've since added other uses: set-numa-node (commit 419fcdec3c1 and
f3be67812c2), and query-cpus-fast (commit ce74ee3dea6). These are not
about device-add.
query-hotpluggable-cpus uses CpuInstanceProperties within
HotpluggableCPU. Lift the documentation related to device-add from
CpuInstanceProperties to HotpluggableCPU.
Signed-off-by: Markus Armbruster
---
qapi/machine.json | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/qapi/machine.json b/qapi/machine.json
index f15ad1b43e..50ff102d56 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -960,9 +960,7 @@
##
# @CpuInstanceProperties:
#
-# List of properties to be used for hotplugging a CPU instance, it
-# should be passed by management with device_add command when a CPU is
-# being hotplugged.
+# Properties identifying a CPU.
#
# Which members are optional and which mandatory depends on the
# architecture and board.
@@ -996,9 +994,6 @@
#
# @thread-id: thread number within the core the CPU belongs to
#
-# .. note:: Management should be prepared to pass through additional
-#properties with device_add.
-#
# Since: 2.7
##
{ 'struct': 'CpuInstanceProperties',
@@ -1020,7 +1015,8 @@
#
# @type: CPU object type for usage with device_add command
#
-# @props: list of properties to be used for hotplugging CPU
+# @props: list of properties to pass for hotplugging a CPU with
+# device_add
#
# @vcpus-count: number of logical VCPU threads @HotpluggableCPU
# provides
@@ -1028,6 +1024,9 @@
# @qom-path: link to existing CPU object if CPU is present or omitted
# if CPU is not present.
#
+# .. note:: Management should be prepared to pass through additional
+#properties with device_add.
+#
# Since: 2.7
##
{ 'struct': 'HotpluggableCPU',
--
2.45.0
[PATCH 5/5] qapi/ui: Drop note on naming of SpiceQueryMouseMode
Doc comments are reference documentation for users of QMP.
SpiceQueryMouseMode's doc comment contains a note explaining why it's
not named SpiceMouseMode: spice/enums.h has it already. Irrelevant
for users of QMP; delete the note.
Signed-off-by: Markus Armbruster
---
qapi/ui.json | 2 --
1 file changed, 2 deletions(-)
diff --git a/qapi/ui.json b/qapi/ui.json
index 5bcccbfc93..df089869a5 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -273,8 +273,6 @@
# @unknown: No information is available about mouse mode used by the
# spice server.
#
-# .. note:: spice/enums.h has a SpiceMouseMode already, hence the name.
-#
# Since: 1.1
##
{ 'enum': 'SpiceQueryMouseMode',
--
2.45.0
[PATCH 1/5] qapi/pci: Clean up documentation around PciDeviceClass
PciDeviceInfo's doc comment has a note on PciDeviceClass member @desc.
Since the note applies always, not just within PciDeviceInfo, merge it
into PciDeviceClass's description of member @desc.
Signed-off-by: Markus Armbruster
---
qapi/pci.json | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/qapi/pci.json b/qapi/pci.json
index 8287d15dd0..97179920fb 100644
--- a/qapi/pci.json
+++ b/qapi/pci.json
@@ -93,7 +93,8 @@
#
# Information about the Class of a PCI device
#
-# @desc: a string description of the device's class
+# @desc: a string description of the device's class (not stable, and
+# should only be treated as informational)
#
# @class: the class code of the device
#
@@ -146,9 +147,6 @@
#
# @regions: a list of the PCI I/O regions associated with the device
#
-# .. note:: The contents of @class_info.desc are not stable and should
-#only be treated as informational.
-#
# Since: 0.14
##
{ 'struct': 'PciDeviceInfo',
--
2.45.0
[PATCH 0/5] qapi: Doc comment cleanups
Markus Armbruster (5): qapi/pci: Clean up documentation around PciDeviceClass qapi/machine: Clean up documentation around CpuInstanceProperties qapi/machine: Clarify query-uuid value when none has been specified qapi/sockets: Move deprecation note out of SocketAddress doc comment qapi/ui: Drop note on naming of SpiceQueryMouseMode qapi/machine.json | 18 -- qapi/pci.json | 6 ++ qapi/sockets.json | 7 +++ qapi/ui.json | 2 -- 4 files changed, 13 insertions(+), 20 deletions(-) -- 2.45.0
Re: [PATCH 8/8] qapi: remove "Example" doc section
John Snow writes:
> On Tue, Jul 9, 2024 at 6:52 AM Markus Armbruster wrote:
>
>> John Snow writes:
>>
>> > Fully eliminate the "Example" sections in QAPI doc blocks now that they
>> > have all been converted to arbitrary rST syntax using the
>> > ".. qmp-example::" directive. Update tests to match.
>> >
>> > Migrating to the new syntax
>> > ---
>> >
>> > The old "Example:" or "Examples:" section syntax is now caught as an
>> > error, but "Example::" is stil permitted as explicit rST syntax for an
>> > un-lexed, generic preformatted text block.
>> >
>> > ('Example' is not special in this case, any sentence that ends with "::"
>> > will start an indented code block in rST.)
>> >
>> > Arbitrary rST for Examples is now possible, but it's strongly
>> > recommended that documentation authors use the ".. qmp-example::"
>> > directive for consistent visual formatting in rendered HTML docs. The
>> > ":title:" directive option may be used to add extra information into the
>> > title bar for the example. The ":annotated:" option can be used to write
>> > arbitrary rST instead, with nested "::" blocks applying QMP formatting
>> > where desired.
>> >
>> > Other choices available are ".. code-block:: QMP" which will not create
>> > an "Example:" box, or the short-form "::" code-block syntax which will
>> > not apply QMP highlighting when used outside of the qmp-example
>> > directive.
>> >
>> > Why?
>> >
>> >
>> > This patch has several benefits:
>> >
>> > 1. Example sections can now be written more arbitrarily, mixing
>> >explanatory paragraphs and code blocks however desired.
>> >
>> > 2. Example sections can now use fully arbitrary rST.
>> >
>> > 3. All code blocks are now lexed and validated as QMP; increasing
>> >usability of the docs and ensuring validity of example snippets.
>> >
>> >(To some extent - This patch only gaurantees it lexes correctly, not
>> >that it's valid under the JSON or QMP grammars. It will catch most
>> >small mistakes, however.)
>> >
>> > 4. Each qmp-example can be titled or annotated independently without
>> >bypassing the QMP lexer/validator.
>> >
>> >(i.e. code blocks are now for *code* only, so we don't have to
>> >sacrifice exposition for having lexically valid examples.)
>> >
>> > NOTE: As with the "Notes" conversion patch,
>>
>> Commit d461c279737 (qapi: convert "Note" sections to plain rST).
>>
>
> Didn't have a stable commit ID at the time, will work it in if/when the
> notes patches hit main.
They have.
>> > this patch (and those
>> > preceding) may change the rendering order for Examples in the
>>
>> The three preceding ones, to be precise.
>>
>> > current generator. The forthcoming qapidoc rewrite will fix this
>> > by always generating documentation in source order.
>>
>> Conversions from "Example" section to plain reST may change order. This
>> patch converts a test, and the preceding three convert the real uses.
>>
>> Does any of the patches actually change order?
>
> I do not actually know ...! It has the *potential* in the same exact way
> that the notes patch did, but I don't actually know if it *did*. My hunch
> is "no" because there's only one intermediate section we identified with
> the notes series, but I didn't exhaustively prove it. That's why I used the
> "may" weasel wording.
Alright, I checked.
In documentation of command announce-self, the example moves from after
the arguments to before. Unwanted change.
I can keep it in place if I insert a TODO before the example like this:
diff --git a/qapi/net.json b/qapi/net.json
index 9a723e56b5..50bfd5b681 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -930,6 +930,8 @@
# switches. This can be useful when network bonds fail-over the
# active slave.
#
+# TODO: This line is a hack to separate the example from the body
+#
# .. qmp-example::
#
# -> { "execute": "announce-self",
I had to delete the .doctrees cache to make sphinx-build generate
corrected output.
>> > Signed-off-by: John Snow
>> > ---
&
Re: [PATCH 4/8] docs/sphinx: add CSS styling for qmp-example directive
John Snow writes: > On Tue, Jul 9, 2024 at 6:34 AM Markus Armbruster wrote: > >> John Snow writes: >> >> > From: Harmonie Snow >> > >> > Add CSS styling for qmp-example directives to increase readability and >> > consistently style all example blocks. >> > >> > Signed-off-by: Harmonie Snow >> > Signed-off-by: John Snow >> >> Same sadness as for the previous patch. >> > > Should we do anything about that? In the long run, I don't expect anyone > will actually need to care about what this directive looked like in some > intermediate state before we ever used it. If you want to evaluate the > directive in the in-between states, I recommend modifying a document and > seeing what it does; but I didn't really intend for anyone to really see it > that way. > > (Isn't it a bit overboard to write unit tests for intermediate tree > states...?) I'm not asking for temporary tests, I just wonder why you delay permanent ones until "[PATCH 8/8] qapi: remove "Example" doc section". No big deal, thus: >> Acked-by: Markus Armbruster
Re: [PATCH 0/8] qapi: convert example sections to qmp-example rST directives
You achieved a clean & consistent look for notes and examples in the browser. Love it!
Re: [PATCH] qdev-monitor: QAPIfy QMP device_add
Stefan Hajnoczi writes:
> The QMP device_add monitor command converts the QDict arguments to
> QemuOpts and then back again to QDict. This process only supports scalar
> types. Device properties like virtio-blk-pci's iothread-vq-mapping (an
> array of objects) are silently dropped by qemu_opts_from_qdict() during
> the QemuOpts conversion even though QAPI is capable of validating them.
> As a result, hotplugging virtio-blk-pci devices with the
> iothread-vq-mapping property does not work as expected (the property is
> ignored). It's time to QAPIfy QMP device_add!
This patch doesn't fully QAPIfy device_add: we still lack a schema
and use 'gen': false. It gets us closer, though.
> Get rid of the QemuOpts conversion in qmp_device_add() and call
> qdev_device_add_from_qdict() with from_json=true. Using the QMP
> command's QDict arguments directly allows non-scalar properties.
>
> The HMP is also adjusted since qmp_device_add()'s now expects properly
> typed JSON arguments and cannot be used from HMP anymore. Move the code
> that was previously in qmp_device_add() (with QemuOpts conversion and
> from_json=false) into hmp_device_add() so that its behavior is
> unchanged.
>
> This patch changes the behavior of QMP device_add but not HMP
> device_add. QMP clients that sent incorrectly typed device_add QMP
> commands no longer work. This is a breaking change but clients should be
> using the correct types already. See the netdev_add QAPIfication in
> commit db2a380c8457 for similar reasoning.
Another one is 9151e59a8b6e: it QAPIfied object-add.
Both commits eliminated the roundtrip through QemuOpts, and weaned the
command off 'gen': false.
This commit eliminates the roundtrip, but keeps 'gen': false. Best we
can do now, but I'd like the commit message to make this clear.
> Markus helped me figure this out and even provided a draft patch. The
> code ended up very close to what he suggested.
>
> Suggested-by: Markus Armbruster
> Cc: Daniel P. Berrangé
> Signed-off-by: Stefan Hajnoczi
> ---
> system/qdev-monitor.c | 41 -
> 1 file changed, 28 insertions(+), 13 deletions(-)
>
> diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
> index 6af6ef7d66..1427aa173c 100644
> --- a/system/qdev-monitor.c
> +++ b/system/qdev-monitor.c
> @@ -849,18 +849,9 @@ void hmp_info_qdm(Monitor *mon, const QDict *qdict)
>
> void qmp_device_add(QDict *qdict, QObject **ret_data, Error **errp)
> {
> -QemuOpts *opts;
> DeviceState *dev;
>
> -opts = qemu_opts_from_qdict(qemu_find_opts("device"), qdict, errp);
> -if (!opts) {
> -return;
> -}
> -if (!monitor_cur_is_qmp() && qdev_device_help(opts)) {
> -qemu_opts_del(opts);
> -return;
> -}
> -dev = qdev_device_add(opts, errp);
> +dev = qdev_device_add_from_qdict(qdict, true, errp);
> if (!dev) {
> /*
> * Drain all pending RCU callbacks. This is done because
> @@ -872,8 +863,6 @@ void qmp_device_add(QDict *qdict, QObject **ret_data,
> Error **errp)
> * to the user
> */
> drain_call_rcu();
> -
> -qemu_opts_del(opts);
> return;
> }
> object_unref(OBJECT(dev));
> @@ -967,8 +956,34 @@ void qmp_device_del(const char *id, Error **errp)
> void hmp_device_add(Monitor *mon, const QDict *qdict)
> {
> Error *err = NULL;
> +QemuOpts *opts;
> +DeviceState *dev;
>
> -qmp_device_add((QDict *)qdict, NULL, );
> +opts = qemu_opts_from_qdict(qemu_find_opts("device"), qdict, );
> +if (!opts) {
> +goto out;
> +}
> +if (qdev_device_help(opts)) {
> +qemu_opts_del(opts);
> +return;
> +}
The part above is moved from qmp_device_add(). The part below is
copied. The duplication is a bid sad. Could we factor it out into
qdev_device_add_from_qdict()?
> +dev = qdev_device_add(opts, );
> +if (!dev) {
> +/*
> + * Drain all pending RCU callbacks. This is done because
> + * some bus related operations can delay a device removal
> + * (in this case this can happen if device is added and then
> + * removed due to a configuration error)
> + * to a RCU callback, but user might expect that this interface
> + * will finish its job completely once qmp command returns result
> + * to the user
> + */
> +drain_call_rcu();
> +
> +qemu_opts_del(opts);
> +}
> +object_unref(OBJECT(dev));
> +out:
> hmp_handle_error(mon, err);
> }
Have a look at this TODO in vl.c:
QTAILQ_FOREACH(opt, _opts, next) {
DeviceState *dev;
Re: [PATCH 7/8] qapi: convert "Example" sections with longer prose
John Snow writes:
> These examples require longer explanations or have explanations that
> require markup to look reasonable when rendered and so use the longer
> form of the ".. qmp-example::" directive.
>
> By using the :annotated: option, the content in the example block is
> assumed *not* to be a code block literal and is instead parsed as normal
> rST - with the exception that any code literal blocks after `::` will
> assumed to be a QMP code literal block.
>
> Note: There's one title-less conversion in this patch that comes along
> for the ride because it's part of a larger "Examples" block that was
> better to convert all at once.
>
> See commit-5: "docs/qapidoc: create qmp-example directive", for a
> detailed explanation of this custom directive syntax.
>
> See commit+1: "qapi: remove "Example" doc section" for a detailed
> explanation of why.
>
> Signed-off-by: John Snow
> ---
> qapi/block.json | 26 --
> qapi/machine.json | 30 --
> qapi/migration.json | 7 +--
> qapi/virtio.json| 24 ++--
> 4 files changed, 59 insertions(+), 28 deletions(-)
>
> diff --git a/qapi/block.json b/qapi/block.json
> index 5ddd061e964..d95e9fd8140 100644
> --- a/qapi/block.json
> +++ b/qapi/block.json
> @@ -545,31 +545,37 @@
> #
> # Since: 4.0
> #
> -# Example:
> +# .. qmp-example::
> +#:annotated:
> #
> -# Set new histograms for all io types with intervals
> -# [0, 10), [10, 50), [50, 100), [100, +inf):
> +#Set new histograms for all io types with intervals
> +#[0, 10), [10, 50), [50, 100), [100, +inf)::
> #
> # -> { "execute": "block-latency-histogram-set",
> # "arguments": { "id": "drive0",
> # "boundaries": [10, 50, 100] } }
> # <- { "return": {} }
> #
> -# Example:
> +# .. qmp-example::
> +#:annotated:
> #
> -# Set new histogram only for write, other histograms will remain
> -# not changed (or not created):
> +#Set new histogram only for write, other histograms will remain
> +#not changed (or not created)::
> #
> # -> { "execute": "block-latency-histogram-set",
> # "arguments": { "id": "drive0",
> # "boundaries-write": [10, 50, 100] } }
> # <- { "return": {} }
> #
> -# Example:
> +# .. qmp-example::
> +#:annotated:
> #
> -# Set new histograms with the following intervals:
> -# read, flush: [0, 10), [10, 50), [50, 100), [100, +inf)
> -# write: [0, 1000), [1000, 5000), [5000, +inf)
> +#Set new histograms with the following intervals:
> +#
> +#- read, flush: [0, 10), [10, 50), [50, 100), [100, +inf)
> +#- write: [0, 1000), [1000, 5000), [5000, +inf)
> +#
> +#::
> #
> # -> { "execute": "block-latency-histogram-set",
> # "arguments": { "id": "drive0",
# "boundaries": [10, 50, 100],
# "boundaries-write": [1000, 5000] } }
# <- { "return": {} }
#
# .. qmp-example::
#:title: Remove all latency histograms
#
# -> { "execute": "block-latency-histogram-set",
# "arguments": { "id": "drive0" } }
# <- { "return": {} }
##
I think using :annotated: for this one as well will look better.
[...]
Reviewed-by: Markus Armbruster
Re: [PATCH 6/8] qapi: convert "Example" sections with titles
John Snow writes: > When an Example section has a brief explanation, convert it to a > qmp-example:: section using the :title: option. > > Rule of thumb: If the title can fit on a single line and requires no rST > markup, it's a good candidate for using the :title: option of > qmp-example. > > In this patch, trailing punctuation is removed from the title section > for consistent headline aesthetics. In just one case, specifics of the > example are removed to make the title read better. > > See commit-4: "docs/qapidoc: create qmp-example directive", for a > detailed explanation of this custom directive syntax. > > See commit+2: "qapi: remove "Example" doc section" for a detailed > explanation of why. > > Signed-off-by: John Snow Reviewed-by: Markus Armbruster
Re: [PATCH 5/8] qapi: convert "Example" sections without titles
John Snow writes:
> On Sat, Jul 6, 2024, 10:42 AM Markus Armbruster wrote:
>
>> John Snow writes:
>>
>> > Use the no-option form of ".. qmp-example::" to convert any Examples
>> > that do not have any form of caption or explanation whatsoever. Note
>> > that in a few cases, example sections are split into two or more
>> > separate example blocks. This is only done stylistically to create a
>> > delineation between two or more logically independent examples.
>> >
>> > See commit-3: "docs/qapidoc: create qmp-example directive", for a
>> > detailed explanation of this custom directive syntax.
>> >
>> > See commit+3: "qapi: remove "Example" doc section" for a detailed
>> > explanation of why.
>> >
>> > Signed-off-by: John Snow
>>
>> [...]
>>
>> > diff --git a/qapi/run-state.json b/qapi/run-state.json
>> > index 252d7d6afa7..718a3c958e9 100644
>> > --- a/qapi/run-state.json
>> > +++ b/qapi/run-state.json
>>
>> [...]
>>
>> > @@ -453,7 +453,7 @@
>> > #
>> > # Since: 5.0
>> > #
>> > -# Example:
>> > +# .. qmp-example::
>> > #
>> > # <- { "event": "GUEST_CRASHLOADED",
>> > # "data": { "action": "run" },
>>
>> Trivial semantic conflict, we need
>>
>
> Caught on rebase late Fri, already fixed locally and will be in v2 (which I
> rebased on top of my sphinx 3.x patches, which change the do_parse() stuff
> too.)
With that fix
Reviewed-by: Markus Armbruster
[...]
Re: [PATCH 8/8] qapi: remove "Example" doc section
John Snow writes:
> Fully eliminate the "Example" sections in QAPI doc blocks now that they
> have all been converted to arbitrary rST syntax using the
> ".. qmp-example::" directive. Update tests to match.
>
> Migrating to the new syntax
> ---
>
> The old "Example:" or "Examples:" section syntax is now caught as an
> error, but "Example::" is stil permitted as explicit rST syntax for an
> un-lexed, generic preformatted text block.
>
> ('Example' is not special in this case, any sentence that ends with "::"
> will start an indented code block in rST.)
>
> Arbitrary rST for Examples is now possible, but it's strongly
> recommended that documentation authors use the ".. qmp-example::"
> directive for consistent visual formatting in rendered HTML docs. The
> ":title:" directive option may be used to add extra information into the
> title bar for the example. The ":annotated:" option can be used to write
> arbitrary rST instead, with nested "::" blocks applying QMP formatting
> where desired.
>
> Other choices available are ".. code-block:: QMP" which will not create
> an "Example:" box, or the short-form "::" code-block syntax which will
> not apply QMP highlighting when used outside of the qmp-example
> directive.
>
> Why?
>
>
> This patch has several benefits:
>
> 1. Example sections can now be written more arbitrarily, mixing
>explanatory paragraphs and code blocks however desired.
>
> 2. Example sections can now use fully arbitrary rST.
>
> 3. All code blocks are now lexed and validated as QMP; increasing
>usability of the docs and ensuring validity of example snippets.
>
>(To some extent - This patch only gaurantees it lexes correctly, not
>that it's valid under the JSON or QMP grammars. It will catch most
>small mistakes, however.)
>
> 4. Each qmp-example can be titled or annotated independently without
>bypassing the QMP lexer/validator.
>
>(i.e. code blocks are now for *code* only, so we don't have to
>sacrifice exposition for having lexically valid examples.)
>
> NOTE: As with the "Notes" conversion patch,
Commit d461c279737 (qapi: convert "Note" sections to plain rST).
> this patch (and those
> preceding) may change the rendering order for Examples in the
The three preceding ones, to be precise.
> current generator. The forthcoming qapidoc rewrite will fix this
> by always generating documentation in source order.
Conversions from "Example" section to plain reST may change order. This
patch converts a test, and the preceding three convert the real uses.
Does any of the patches actually change order?
> Signed-off-by: John Snow
> ---
> docs/devel/qapi-code-gen.rst| 58 -
> scripts/qapi/parser.py | 10 +-
> tests/qapi-schema/doc-good.json | 19 +++
> tests/qapi-schema/doc-good.out | 26 ++-
> tests/qapi-schema/doc-good.txt | 23 ++---
> 5 files changed, 98 insertions(+), 38 deletions(-)
>
> diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst
> index ae97b335cbf..2e10a3cbd69 100644
> --- a/docs/devel/qapi-code-gen.rst
> +++ b/docs/devel/qapi-code-gen.rst
> @@ -899,7 +899,7 @@ Documentation markup
>
>
> Documentation comments can use most rST markup. In particular,
> -a ``::`` literal block can be used for examples::
> +a ``::`` literal block can be used for pre-formatted text::
>
> # ::
> #
> @@ -995,8 +995,8 @@ line "Features:", like this::
># @feature: Description text
>
> A tagged section begins with a paragraph that starts with one of the
> -following words: "Since:", "Example:"/"Examples:", "Returns:",
> -"Errors:", "TODO:". It ends with the start of a new section.
> +following words: "Since:", "Returns:", "Errors:", "TODO:". It ends with
> +the start of a new section.
>
> The second and subsequent lines of tagged sections must be indented
> like this::
> @@ -1020,13 +1020,53 @@ detailing a relevant error condition. For example::
> A "Since: x.y.z" tagged section lists the release that introduced the
> definition.
>
> -An "Example" or "Examples" section is rendered entirely
> -as literal fixed-width text. "TODO" sections are not rendered at all
> -(they are for developers, not users of QMP). In other sections, the
> -text is formatted, and rST markup can be used.
> +"TODO" sections are not rendered at all (they are for developers, not
Drop "at all"?
> +users of QMP). In other sections, the text is formatted, and rST markup
> +can be used.
> +
> +QMP Examples can be added by using the ``.. qmp-example::``
> +directive. In its simplest form, this can be used to contain a single
> +QMP code block which accepts standard JSON syntax with additional server
> +directionality indicators (``->`` and ``<-``), and elisions (``...``).
> +
> +Optionally, a plaintext title may be provided by using the ``:title:``
>
Re: [PATCH 4/8] docs/sphinx: add CSS styling for qmp-example directive
John Snow writes: > From: Harmonie Snow > > Add CSS styling for qmp-example directives to increase readability and > consistently style all example blocks. > > Signed-off-by: Harmonie Snow > Signed-off-by: John Snow Same sadness as for the previous patch. Acked-by: Markus Armbruster
Re: [PATCH 3/8] docs/qapidoc: add QMP highlighting to annotated qmp-example blocks
John Snow writes:
> For any code literal blocks inside of a qmp-example directive, apply and
> enforce the QMP lexer/highlighter to those blocks.
>
> This way, you won't need to write:
>
> ```
> .. qmp-example::
>:annotated:
>
>Blah blah
>
>.. code-block:: QMP
>
> -> { "lorem": "ipsum" }
> ```
>
> But instead, simply:
>
> ```
> .. qmp-example::
>:annotated:
>
>Blah blah::
>
> -> { "lorem": "ipsum" }
> ```
>
> Once the directive block is exited, whatever the previous default
> highlight language was will be restored; localizing the forced QMP
> lexing to exclusively this directive.
>
> Note, if the default language is *already* QMP, this directive will not
> generate and restore redundant highlight configuration nodes. We may
> well decide that the default language ought to be QMP for any QAPI
> reference pages, but this way the directive behaves consistently no
> matter where it is used.
>
> Signed-off-by: John Snow
Sadly, the previous patch didn't add tests, so this patch's effect
isn't as easy to observe as it could be.
Acked-by: Markus Armbruster
Re: [PATCH 2/8] docs/qapidoc: create qmp-example directive
John Snow writes:
> This is a directive that creates a syntactic sugar for creating
> "Example" boxes very similar to the ones already used in the bitmaps.rst
> document, please see e.g.
> https://www.qemu.org/docs/master/interop/bitmaps.html#creation-block-dirty-bitmap-add
>
> In its simplest form, when a custom title is not needed or wanted, and
> the example body is *solely* a QMP example:
>
> ```
> .. qmp-example::
>
>{body}
> ```
>
> is syntactic sugar for:
>
> ```
> .. admonition:: Example:
>
>.. code-block:: QMP
>
> {body}
> ```
>
> When a custom, plaintext title that describes the example is desired,
> this form:
>
> ```
> .. qmp-example::
>:title: Defrobnification
>
>{body}
> ```
>
> Is syntactic sugar for:
>
> ```
> .. admonition:: Example: Defrobnification
>
>.. code-block:: QMP
>
> {body}
> ```
>
> Lastly, when Examples are multi-step processes that require non-QMP
> exposition, have lengthy titles, or otherwise involve prose with rST
> markup (lists, cross-references, etc), the most complex form:
>
> ```
> .. qmp-example::
>:annotated:
>
>This example shows how to use `foo-command`::
>
> {body}
>
>For more information, please see `frobnozz`.
> ```
>
> Is desugared to:
>
> ```
> .. admonition:: Example:
>
>This example shows how to use `foo-command`::
>
> {body}
>
>For more information, please see `frobnozz`.
> ```
>
> Note that :annotated: and :title: options can be combined together, if
> desired.
>
> The primary benefit here being documentation source consistently using
> the same directive for all forms of examples to ensure consistent visual
> styling, and ensuring all relevant prose is visually grouped alongside
> the code literal block.
>
> Note that as of this commit, the code-block rST syntax "::" does not
> apply QMP highlighting; you would need to use ".. code-block:: QMP". The
> very next commit changes this behavior to assume all "::" code blocks
> within this directive are QMP blocks.
>
> Signed-off-by: John Snow
Acked-by: Markus Armbruster
Re: [PATCH 1/8] docs/qapidoc: factor out do_parse()
John Snow writes: > On Sat, Jul 6, 2024, 10:47 AM Markus Armbruster wrote: > >> John Snow writes: >> >> > Factor out the compatibility parser helper into a base class, so it can >> > be shared by other directives. >> > >> > Signed-off-by: John Snow >> >> R-by stands. > > Assuming true even if I rebase on top of the 3.x patches and do_parse() > becomes quite a bit more trivial? If you think the changes don't warrant a fresh review, keep my R-by. You may get one anyway ;)
Re: QEMU Community Call Agenda Items (July 9th, 2024)
Cc: Block layer core maintainers Philippe Mathieu-Daudé writes: > On 8/7/24 16:58, Alex Bennée wrote: >> Hi, >> The KVM/QEMU community call is at: >>https://meet.jit.si/kvmcallmeeting >>@ >>9/7/2024 14:00 UTC >> Are there any agenda items for the sync-up? > > - I don't remember who mentioned "3 phase reset and KVM", > maybe Daniel Barboza or Peter Xu. > > - Questions for block team: > > . Are there plan to remove the legacy DriveInfo? What should > we use instead, blk_by_name() and/or blk_by_qdev_id()? > > . Are there plan to move away from the IF_FOO (see blockdev::if_name) > or is it OK to use them and keep them in mind with new designs? > > . To model one HW with multiple drives (at least 3), is there > a recommended way to create that from the CLI?
Re: [PATCH 1/8] docs/qapidoc: factor out do_parse()
John Snow writes: > Factor out the compatibility parser helper into a base class, so it can > be shared by other directives. > > Signed-off-by: John Snow R-by stands.
Re: [PATCH 5/8] qapi: convert "Example" sections without titles
John Snow writes:
> Use the no-option form of ".. qmp-example::" to convert any Examples
> that do not have any form of caption or explanation whatsoever. Note
> that in a few cases, example sections are split into two or more
> separate example blocks. This is only done stylistically to create a
> delineation between two or more logically independent examples.
>
> See commit-3: "docs/qapidoc: create qmp-example directive", for a
> detailed explanation of this custom directive syntax.
>
> See commit+3: "qapi: remove "Example" doc section" for a detailed
> explanation of why.
>
> Signed-off-by: John Snow
[...]
> diff --git a/qapi/run-state.json b/qapi/run-state.json
> index 252d7d6afa7..718a3c958e9 100644
> --- a/qapi/run-state.json
> +++ b/qapi/run-state.json
[...]
> @@ -453,7 +453,7 @@
> #
> # Since: 5.0
> #
> -# Example:
> +# .. qmp-example::
> #
> # <- { "event": "GUEST_CRASHLOADED",
> # "data": { "action": "run" },
Trivial semantic conflict, we need
@@ -469,7 +469,7 @@
#
# Since: 9.1
#
-# Example:
+# .. qmp-example::
#
# <- { "event": "GUEST_PVSHUTDOWN",
# "timestamp": { "seconds": 1648245259, "microseconds": 893771 } }
> @@ -597,7 +597,7 @@
> #
> # Since: 5.2
> #
> -# Example:
> +# .. qmp-example::
> #
> # <- { "event": "MEMORY_FAILURE",
> # "data": { "recipient": "hypervisor",
[...]
[PULL 06/13] docs/qapidoc: fix nested parsing under untagged sections
From: John Snow
Sphinx does not like sections without titles, because it wants to
convert every section into a reference. When there is no title, it
struggles to do this and transforms the tree inproperly.
Depending on the rST used, this may result in an assertion error deep in
the docutils HTMLWriter.
(Observed when using ".. admonition:: Notes" under such a section - When
this is transformed with its own element, Sphinx is fooled into
believing this title belongs to the section and incorrect mutates the
docutils tree, leading to errors during rendering time.)
When parsing an untagged section (free paragraphs), skip making a hollow
section and instead append the parse results to the prior section.
Many Bothans died to bring us this information.
The resulting output changes are basically invisible.
Signed-off-by: John Snow
Acked-by: Markus Armbruster
Message-ID: <20240626222128.406106-8-js...@redhat.com>
[Mention output changes in commit message]
Signed-off-by: Markus Armbruster
---
docs/sphinx/qapidoc.py | 16 +++-
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
index f9683444b1..efcd84656f 100644
--- a/docs/sphinx/qapidoc.py
+++ b/docs/sphinx/qapidoc.py
@@ -272,14 +272,20 @@ def _nodes_for_sections(self, doc):
if section.tag and section.tag == 'TODO':
# Hide TODO: sections
continue
+
+if not section.tag:
+# Sphinx cannot handle sectionless titles;
+# Instead, just append the results to the prior section.
+container = nodes.container()
+self._parse_text_into_node(section.text, container)
+nodelist += container.children
+continue
+
snode = self._make_section(section.tag)
-if section.tag and section.tag.startswith('Example'):
+if section.tag.startswith('Example'):
snode += self._nodes_for_example(dedent(section.text))
else:
-self._parse_text_into_node(
-dedent(section.text) if section.tag else section.text,
-snode,
-)
+self._parse_text_into_node(dedent(section.text), snode)
nodelist.append(snode)
return nodelist
--
2.45.0
[PULL 05/13] qapi/parser: fix comment parsing immediately following a doc block
From: John Snow
If a comment immediately follows a doc block, the parser doesn't ignore
that token appropriately. Fix that.
e.g.
> ##
> # = Hello World!
> ##
>
> # I'm a comment!
will break the parser, because it does not properly ignore the comment
token if it immediately follows a doc block.
Fixes: 3d035cd2cca6 (qapi: Rewrite doc comment parser)
Signed-off-by: John Snow
Reviewed-by: Markus Armbruster
Message-ID: <20240626222128.406106-7-js...@redhat.com>
Signed-off-by: Markus Armbruster
---
scripts/qapi/parser.py | 2 +-
tests/qapi-schema/doc-good.json | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index 1ef1f85b02..c3d20cc01b 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -583,7 +583,7 @@ def get_doc(self) -> 'QAPIDoc':
line = self.get_doc_line()
first = False
-self.accept(False)
+self.accept()
doc.end()
return doc
diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json
index de38a386e8..8b39eb946a 100644
--- a/tests/qapi-schema/doc-good.json
+++ b/tests/qapi-schema/doc-good.json
@@ -55,6 +55,8 @@
# - {braces}
##
+# Not a doc comment
+
##
# @Enum:
#
--
2.45.0
[PULL 10/13] qapi: update prose in note blocks
From: John Snow Where I've noticed, rephrase the note to read more fluently. Signed-off-by: John Snow Reviewed-by: Markus Armbruster Message-ID: <20240626222128.406106-12-js...@redhat.com> Signed-off-by: Markus Armbruster --- qapi/block-core.json | 4 ++-- qga/qapi-schema.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/qapi/block-core.json b/qapi/block-core.json index cacedfb771..9ef23ec02a 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -6048,9 +6048,9 @@ # # @name: the name of the internal snapshot to be created # -# .. note:: In transaction, if @name is empty, or any snapshot matching +# .. note:: In a transaction, if @name is empty or any snapshot matching #@name exists, the operation will fail. Only some image formats -#support it, for example, qcow2, and rbd. +#support it; for example, qcow2, and rbd. # # Since: 1.7 ## diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json index 57598331c5..1273d85bb5 100644 --- a/qga/qapi-schema.json +++ b/qga/qapi-schema.json @@ -480,7 +480,7 @@ # # Returns: Number of file systems thawed by this call # -# .. note:: If return value does not match the previous call to +# .. note:: If the return value does not match the previous call to #guest-fsfreeze-freeze, this likely means some freezable filesystems #were unfrozen before this call, and that the filesystem state may #have changed before issuing this command. -- 2.45.0
[PULL 08/13] qapi: nail down convention that Errors sections are lists
From: John Snow By unstated convention, Errors sections are rST lists. Document the convention, and make the one exception conform. Signed-off-by: John Snow Reviewed-by: Markus Armbruster Message-ID: <20240626222128.406106-10-js...@redhat.com> Signed-off-by: Markus Armbruster --- docs/devel/qapi-code-gen.rst | 7 +++ qapi/transaction.json| 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst index f453bd3546..cee43222f1 100644 --- a/docs/devel/qapi-code-gen.rst +++ b/docs/devel/qapi-code-gen.rst @@ -1011,6 +1011,13 @@ like this:: "Returns" and "Errors" sections are only valid for commands. They document the success and the error response, respectively. +"Errors" sections should be formatted as an rST list, each entry +detailing a relevant error condition. For example:: + + # Errors: + # - If @device does not exist, DeviceNotFound + # - Any other error returns a GenericError. + A "Since: x.y.z" tagged section lists the release that introduced the definition. diff --git a/qapi/transaction.json b/qapi/transaction.json index 5749c133d4..07afc269d5 100644 --- a/qapi/transaction.json +++ b/qapi/transaction.json @@ -235,7 +235,7 @@ # additional detail. # # Errors: -# Any errors from commands in the transaction +# - Any errors from commands in the transaction # # Note: The transaction aborts on the first failure. Therefore, there # will be information on only one failed operation returned in an -- 2.45.0
[PULL 04/13] qapi/parser: preserve indentation in QAPIDoc sections
From: John Snow
Change get_doc_indented() to preserve indentation on all subsequent text
lines, and create a compatibility dedent() function for qapidoc.py that
removes indentation the same way get_doc_indented() did.
This is being done for the benefit of a new qapidoc generator which
requires that indentation in argument and features sections are
preserved.
Prior to this patch, a section like this:
```
@name: lorem ipsum
dolor sit amet
consectetur adipiscing elit
```
would have its body text be parsed into:
```
lorem ipsum
dolor sit amet
consectetur adipiscing elit
```
We want to preserve the indentation for even the first body line so that
the entire block can be parsed directly as rST. This patch would now
parse that segment into:
```
lorem ipsum
dolor sit amet
consectetur adipiscing elit
```
This is helpful for formatting arguments and features as field lists in
rST, where the new generator will format this information as:
```
:arg type name: lorem ipsum
dolor sit amet
consectetur apidiscing elit
```
...and can be formed by the simple concatenation of the field list
construct and the body text. The indents help preserve the continuation
of a block-level element, and further allow the use of additional rST
block-level constructs such as code blocks, lists, and other such
markup.
This understandably breaks the existing qapidoc.py; so a new function is
added there to dedent the text for compatibility. Once the new generator
is merged, this function will not be needed any longer and can be
dropped.
I verified this patch changes absolutely nothing by comparing the
md5sums of the QMP ref html pages both before and after the change, so
it's certified inert. QAPI test output has been updated to reflect the
new strategy of preserving indents for rST.
Signed-off-by: John Snow
Reviewed-by: Markus Armbruster
Message-ID: <20240626222128.406106-6-js...@redhat.com>
[Lost commit message paragraph restored]
Signed-off-by: Markus Armbruster
---
docs/sphinx/qapidoc.py | 27 ++-
scripts/qapi/parser.py | 4 ++--
tests/qapi-schema/doc-good.out | 32
3 files changed, 40 insertions(+), 23 deletions(-)
diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
index 659e507353..f9683444b1 100644
--- a/docs/sphinx/qapidoc.py
+++ b/docs/sphinx/qapidoc.py
@@ -26,6 +26,7 @@
import os
import re
+import textwrap
from docutils import nodes
from docutils.parsers.rst import Directive, directives
@@ -53,6 +54,19 @@
__version__ = "1.0"
+def dedent(text: str) -> str:
+# Adjust indentation to make description text parse as paragraph.
+
+lines = text.splitlines(True)
+if re.match(r"\s+", lines[0]):
+# First line is indented; description started on the line after
+# the name. dedent the whole block.
+return textwrap.dedent(text)
+
+# Descr started on same line. Dedent line 2+.
+return lines[0] + textwrap.dedent("".join(lines[1:]))
+
+
# Disable black auto-formatter until re-enabled:
# fmt: off
@@ -164,7 +178,7 @@ def _nodes_for_members(self, doc, what, base=None,
branches=None):
term = self._nodes_for_one_member(section.member)
# TODO drop fallbacks when undocumented members are outlawed
if section.text:
-defn = section.text
+defn = dedent(section.text)
else:
defn = [nodes.Text('Not documented')]
@@ -202,7 +216,7 @@ def _nodes_for_enum_values(self, doc):
termtext.extend(self._nodes_for_ifcond(section.member.ifcond))
# TODO drop fallbacks when undocumented members are outlawed
if section.text:
-defn = section.text
+defn = dedent(section.text)
else:
defn = [nodes.Text('Not documented')]
@@ -237,7 +251,7 @@ def _nodes_for_features(self, doc):
dlnode = nodes.definition_list()
for section in doc.features.values():
dlnode += self._make_dlitem(
-[nodes.literal('', section.member.name)], section.text)
+[nodes.literal('', section.member.name)], dedent(section.text))
seen_item = True
if not seen_item:
@@ -260,9 +274,12 @@ def _nodes_for_sections(self, doc):
continue
snode = self._make_section(section.tag)
if section.tag and section.tag.startswith('Example'):
-snode += self._nodes_for_example(section.text)
+snode += self._nodes_for_example(dedent(section.text))
else:
-self._parse_text_into_node(section.text, snode)
+self._parse_text_into_node(
+dedent(section.text) if section.tag else section.text,
+snode,
+)
nodelist.append(snode)
return nodelis
[PULL 12/13] qapi/parser: don't parse rST markup as section headers
From: John Snow
The double-colon synax is rST formatting that precedes a literal code
block. We do not want to capture these as QAPI-specific sections.
Coerce blocks that start with e.g. "Example::" to be parsed as untagged
paragraphs instead of special tagged sections.
Signed-off-by: John Snow
Message-ID: <20240626222128.406106-14-js...@redhat.com>
Reviewed-by: Markus Armbruster
[Indentation tweaked for consistency]
Signed-off-by: Markus Armbruster
---
scripts/qapi/parser.py | 9 +++--
tests/qapi-schema/doc-good.json | 3 +++
tests/qapi-schema/doc-good.out | 3 +++
tests/qapi-schema/doc-good.txt | 3 +++
4 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index 0a13f0f541..6ad5663e54 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -544,10 +544,15 @@ def get_doc(self) -> 'QAPIDoc':
line = self.get_doc_indented(doc)
no_more_args = True
elif match := re.match(
-r'(Returns|Errors|Since|Notes?|Examples?|TODO): *',
-line):
+r'(Returns|Errors|Since|Notes?|Examples?|TODO)'
+r'(?!::): *',
+line,
+):
# tagged section
+# Note: "sections" with two colons are left alone as
+# rST markup and not interpreted as a section heading.
+
# TODO: Remove this error sometime in 2025 or so
# after we've fully transitioned to the new qapidoc
# generator.
diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json
index 32ff910b4f..b565895858 100644
--- a/tests/qapi-schema/doc-good.json
+++ b/tests/qapi-schema/doc-good.json
@@ -181,6 +181,9 @@
# - *verbatim*
# - {braces}
#
+# Note::
+# Ceci n'est pas une note
+#
# Since: 2.10
##
{ 'command': 'cmd',
diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
index 631dc9f8da..a8e9456f60 100644
--- a/tests/qapi-schema/doc-good.out
+++ b/tests/qapi-schema/doc-good.out
@@ -190,6 +190,9 @@ frobnicate
section=Examples
- *verbatim*
- {braces}
+section=None
+Note::
+Ceci n'est pas une note
section=Since
2.10
doc symbol=cmd-boxed
diff --git a/tests/qapi-schema/doc-good.txt b/tests/qapi-schema/doc-good.txt
index d8bfa742c2..30d457e548 100644
--- a/tests/qapi-schema/doc-good.txt
+++ b/tests/qapi-schema/doc-good.txt
@@ -231,6 +231,9 @@ Examples
- *verbatim*
- {braces}
+Note::
+ Ceci n'est pas une note
+
Since
~
--
2.45.0
[PULL 13/13] sphinx/qapidoc: Fix to generate doc for explicit, unboxed arguments
When a command's arguments are specified as an explicit type T,
generated documentation points to the members of T.
Example:
##
# @announce-self:
#
# Trigger generation of broadcast RARP frames to update network
[...]
##
{ 'command': 'announce-self', 'boxed': true,
'data' : 'AnnounceParameters'}
generates
"announce-self" (Command)
-
Trigger generation of broadcast RARP frames to update network
[...]
Arguments
~
The members of "AnnounceParameters"
Except when the command takes its arguments unboxed , i.e. it doesn't
have 'boxed': true, we generate *nothing*. A few commands have a
reference in their doc comment to compensate, but most don't.
Example:
##
# @blockdev-snapshot-sync:
#
# Takes a synchronous snapshot of a block device.
#
# For the arguments, see the documentation of BlockdevSnapshotSync.
[...]
##
{ 'command': 'blockdev-snapshot-sync',
'data': 'BlockdevSnapshotSync',
'allow-preconfig': true }
generates
"blockdev-snapshot-sync" (Command)
~~
Takes a synchronous snapshot of a block device.
For the arguments, see the documentation of BlockdevSnapshotSync.
[...]
Same for event data.
Fix qapidoc.py to generate the reference regardless of boxing. Delete
now redundant references in the doc comments.
Fixes: 4078ee5469e5 (docs/sphinx: Add new qapi-doc Sphinx extension)
Cc: qemu-sta...@nongnu.org
Signed-off-by: Markus Armbruster
Message-ID: <20240628112756.794237-1-arm...@redhat.com>
Reviewed-by: John Snow
---
docs/sphinx/qapidoc.py | 12 +---
qapi/block-core.json | 7 ---
2 files changed, 5 insertions(+), 14 deletions(-)
diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
index efcd84656f..2b06750a3c 100644
--- a/docs/sphinx/qapidoc.py
+++ b/docs/sphinx/qapidoc.py
@@ -230,15 +230,15 @@ def _nodes_for_enum_values(self, doc):
section += dlnode
return [section]
-def _nodes_for_arguments(self, doc, boxed_arg_type):
+def _nodes_for_arguments(self, doc, arg_type):
"""Return list of doctree nodes for the arguments section"""
-if boxed_arg_type:
+if arg_type and not arg_type.is_implicit():
assert not doc.args
section = self._make_section('Arguments')
dlnode = nodes.definition_list()
dlnode += self._make_dlitem(
[nodes.Text('The members of '),
- nodes.literal('', boxed_arg_type.name)],
+ nodes.literal('', arg_type.name)],
None)
section += dlnode
return [section]
@@ -352,8 +352,7 @@ def visit_command(self, name, info, ifcond, features,
arg_type,
allow_preconfig, coroutine):
doc = self._cur_doc
self._add_doc('Command',
- self._nodes_for_arguments(doc,
-arg_type if boxed else None)
+ self._nodes_for_arguments(doc, arg_type)
+ self._nodes_for_features(doc)
+ self._nodes_for_sections(doc)
+ self._nodes_for_if_section(ifcond))
@@ -361,8 +360,7 @@ def visit_command(self, name, info, ifcond, features,
arg_type,
def visit_event(self, name, info, ifcond, features, arg_type, boxed):
doc = self._cur_doc
self._add_doc('Event',
- self._nodes_for_arguments(doc,
-arg_type if boxed else None)
+ self._nodes_for_arguments(doc, arg_type)
+ self._nodes_for_features(doc)
+ self._nodes_for_sections(doc)
+ self._nodes_for_if_section(ifcond))
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 9ef23ec02a..096bdbe0aa 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1675,8 +1675,6 @@
#
# Takes a synchronous snapshot of a block device.
#
-# For the arguments, see the documentation of BlockdevSnapshotSync.
-#
# Errors:
# - If @device is not a valid block device, DeviceNotFound
#
@@ -1705,8 +1703,6 @@
# device, the block device changes to using 'overlay' as its new
# active image.
#
-# For the arguments, see the documentation of BlockdevSnapshot.
-#
# Features:
#
# @allow-write-only-overlay: If present, the check whether this
@@ -6065,9 +6061,6 @@
# string, or a snapshot with name already exists, the operation will
# fail.
#
-# For the arguments, see the documentation of
-# BlockdevSnapshotInternal.
-#
# Errors:
# - If @device is not a valid block device, GenericError
# - If any snapshot matching @name exists, or @name is empty,
--
2.45.0
[PULL 07/13] qapi: fix non-compliant JSON examples
From: John Snow
The new QMP documentation generator wants to parse all examples as
"QMP". We have an existing QMP lexer in docs/sphinx/qmp_lexer.py (Seen
in-use here: https://qemu-project.gitlab.io/qemu/interop/bitmaps.html)
that allows the use of "->", "<-" and "..." tokens to denote QMP
protocol flow with elisions, but otherwise defers to the JSON lexer.
To utilize this lexer for the existing QAPI documentation, we need them
to conform to a standard so that they lex and render correctly. Once the
QMP lexer is active for examples, errant QMP/JSON will produce warning
messages and fail the build.
Fix any invalid JSON found in QAPI documentation (identified by
attempting to lex all examples as QMP; see subsequent
commits). Additionally, elisions must be standardized for the QMP lexer;
they must be represented as the value "...", so three examples have been
adjusted to support that format here.
Signed-off-by: John Snow
Reviewed-by: Markus Armbruster
Message-ID: <20240626222128.406106-9-js...@redhat.com>
Signed-off-by: Markus Armbruster
---
qapi/control.json | 3 ++-
qapi/machine.json | 2 +-
qapi/migration.json | 2 +-
qapi/misc.json | 3 ++-
qapi/net.json | 6 +++---
qapi/rocker.json| 2 +-
qapi/ui.json| 2 +-
7 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/qapi/control.json b/qapi/control.json
index 6bdbf077c2..10c906fa0e 100644
--- a/qapi/control.json
+++ b/qapi/control.json
@@ -145,7 +145,8 @@
# },
# {
#"name":"system_powerdown"
-# }
+# },
+# ...
# ]
#}
#
diff --git a/qapi/machine.json b/qapi/machine.json
index 2fd3e9c3d5..a982c94503 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1057,7 +1057,7 @@
#"vcpus-count": 1 },
# { "props": { "core-id": 0 }, "type": "POWER8-spapr-cpu-core",
#"vcpus-count": 1, "qom-path": "/machine/unattached/device[0]"}
-#]}'
+#]}
#
# For pc machine type started with -smp 1,maxcpus=2:
#
diff --git a/qapi/migration.json b/qapi/migration.json
index 0f24206bce..9ec9ef36c4 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -2106,7 +2106,7 @@
# Example:
#
# -> {"execute": "calc-dirty-rate", "arguments": {"calc-time": 1,
-# 'sample-pages': 512} }
+# "sample-pages": 512} }
# <- { "return": {} }
#
# Measure dirty rate using dirty bitmap for 500 milliseconds:
diff --git a/qapi/misc.json b/qapi/misc.json
index ec30e5c570..4b41e15dcd 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -287,7 +287,8 @@
#
# Example:
#
-# -> { "execute": "get-win32-socket", "arguments": { "info": "abcd123..",
fdname": "skclient" } }
+# -> { "execute": "get-win32-socket",
+# "arguments": { "info": "abcd123..", "fdname": "skclient" } }
# <- { "return": {} }
##
{ 'command': 'get-win32-socket', 'data': {'info': 'str', 'fdname': 'str'},
'if': 'CONFIG_WIN32' }
diff --git a/qapi/net.json b/qapi/net.json
index 0f5a259475..c19df435a5 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -1003,9 +1003,9 @@
#
# Example:
#
-# <- { 'event': 'NETDEV_STREAM_DISCONNECTED',
-# 'data': {'netdev-id': 'netdev0'},
-# 'timestamp': {'seconds': 1663330937, 'microseconds': 526695} }
+# <- { "event": "NETDEV_STREAM_DISCONNECTED",
+# "data": {"netdev-id": "netdev0"},
+# "timestamp": {"seconds": 1663330937, "microseconds": 526695} }
##
{ 'event': 'NETDEV_STREAM_DISCONNECTED',
'data': { 'netdev-id': 'str' } }
diff --git a/qapi/rocker.json b/qapi/rocker.json
index 5635cf174f..f5225eb62c 100644
--- a/qapi/rocker.json
+++ b/qapi/rocker.json
@@ -250,7 +250,7 @@
# "action": {"goto-tbl": 10},
# "mask": {"in-pport": 4294901760}
# },
-# {...more...},
+# {...},
#]}
##
{ 'command': 'query-rocker-of-dpa-flows',
diff --git a/qapi/ui.json b/qapi/ui.json
index f610bce118..c12f529257 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -361,7 +361,7 @@
#"channel-id": 0,
#"tls": false
# },
-# [ ... more channels follow ... ]
+# ...
# ]
# }
#}
--
2.45.0
[PULL 03/13] docs/qapidoc: delint a tiny portion of the module
From: John Snow
In a forthcoming series that adds a new QMP documentation generator, it
will be helpful to have a linting baseline. However, there's no need to
shuffle around the deck chairs too much, because most of this code will
be removed once that new qapidoc generator (the "transmogrifier") is in
place.
To ease my pain: just turn off the black auto-formatter for most, but
not all, of qapidoc.py. This will help ensure that *new* code follows a
coding standard without bothering too much with cleaning up the existing
code.
Code that I intend to keep is still subject to the delinting beam.
Signed-off-by: John Snow
Reviewed-by: Markus Armbruster
Message-ID: <20240626222128.406106-5-js...@redhat.com>
Signed-off-by: Markus Armbruster
---
docs/sphinx/qapidoc.py | 62 +-
1 file changed, 37 insertions(+), 25 deletions(-)
diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
index 3c0565d0ce..659e507353 100644
--- a/docs/sphinx/qapidoc.py
+++ b/docs/sphinx/qapidoc.py
@@ -28,26 +28,33 @@
import re
from docutils import nodes
+from docutils.parsers.rst import Directive, directives
from docutils.statemachine import ViewList
-from docutils.parsers.rst import directives, Directive
+from qapi.error import QAPIError, QAPISemError
+from qapi.gen import QAPISchemaVisitor
+from qapi.schema import QAPISchema
+
+import sphinx
from sphinx.errors import ExtensionError
from sphinx.util.nodes import nested_parse_with_titles
-import sphinx
-from qapi.gen import QAPISchemaVisitor
-from qapi.error import QAPIError, QAPISemError
-from qapi.schema import QAPISchema
# Sphinx up to 1.6 uses AutodocReporter; 1.7 and later
# use switch_source_input. Check borrowed from kerneldoc.py.
-Use_SSI = sphinx.__version__[:3] >= '1.7'
-if Use_SSI:
+USE_SSI = sphinx.__version__[:3] >= "1.7"
+if USE_SSI:
from sphinx.util.docutils import switch_source_input
else:
-from sphinx.ext.autodoc import AutodocReporter
+from sphinx.ext.autodoc import ( # pylint: disable=no-name-in-module
+AutodocReporter,
+)
-__version__ = '1.0'
+__version__ = "1.0"
+
+
+# Disable black auto-formatter until re-enabled:
+# fmt: off
class QAPISchemaGenRSTVisitor(QAPISchemaVisitor):
@@ -441,6 +448,10 @@ def get_document_nodes(self):
return self._top_node.children
+# Turn the black formatter on for the rest of the file.
+# fmt: on
+
+
class QAPISchemaGenDepVisitor(QAPISchemaVisitor):
"""A QAPI schema visitor which adds Sphinx dependencies each module
@@ -448,34 +459,34 @@ class QAPISchemaGenDepVisitor(QAPISchemaVisitor):
that the generated documentation output depends on the input
schema file associated with each module in the QAPI input.
"""
+
def __init__(self, env, qapidir):
self._env = env
self._qapidir = qapidir
def visit_module(self, name):
if name != "./builtin":
-qapifile = self._qapidir + '/' + name
+qapifile = self._qapidir + "/" + name
self._env.note_dependency(os.path.abspath(qapifile))
super().visit_module(name)
class QAPIDocDirective(Directive):
"""Extract documentation from the specified QAPI .json file"""
+
required_argument = 1
optional_arguments = 1
-option_spec = {
-'qapifile': directives.unchanged_required
-}
+option_spec = {"qapifile": directives.unchanged_required}
has_content = False
def new_serialno(self):
"""Return a unique new ID string suitable for use as a node's ID"""
env = self.state.document.settings.env
-return 'qapidoc-%d' % env.new_serialno('qapidoc')
+return "qapidoc-%d" % env.new_serialno("qapidoc")
def run(self):
env = self.state.document.settings.env
-qapifile = env.config.qapidoc_srctree + '/' + self.arguments[0]
+qapifile = env.config.qapidoc_srctree + "/" + self.arguments[0]
qapidir = os.path.dirname(qapifile)
try:
@@ -513,13 +524,14 @@ def do_parse(self, rstlist, node):
# plain self.state.nested_parse(), and so we can drop the saving
# of title_styles and section_level that kerneldoc.py does,
# because nested_parse_with_titles() does that for us.
-if Use_SSI:
+if USE_SSI:
with switch_source_input(self.state, rstlist):
nested_parse_with_titles(self.state, rstlist, node)
else:
save = self.state.memo.reporter
self.state.memo.reporter = AutodocReporter(
-rstlist, self.state.memo.reporter)
+rstlist, self.state.memo.reporter
+)
try:
nested_parse_with_titles(self.state, rstlist, node)
fi
[PULL 11/13] qapi: add markup to note blocks
From: John Snow
Generally, surround command-line options with ``literal`` markup to help
it stand out from prose in rendered HTML, and add cross-references to
replace "see also" messages.
References to types, values, and other QAPI definitions are not yet
adjusted here; they will be converted en masse in a subsequent patch
after the new QAPI doc generator is merged.
Signed-off-by: John Snow
Reviewed-by: Markus Armbruster
Message-ID: <20240626222128.406106-13-js...@redhat.com>
Signed-off-by: Markus Armbruster
---
qapi/control.json | 4 ++--
qapi/misc.json | 8
qapi/qdev.json | 2 +-
qapi/run-state.json | 2 +-
qapi/sockets.json | 2 +-
qapi/ui.json| 2 +-
6 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/qapi/control.json b/qapi/control.json
index 59d5e00c15..fe2af45120 100644
--- a/qapi/control.json
+++ b/qapi/control.json
@@ -24,8 +24,8 @@
#
# .. note:: This command is valid exactly when first connecting: it must
#be issued before any other command will be accepted, and will fail
-#once the monitor is accepting other commands. (see qemu
-#docs/interop/qmp-spec.rst)
+#once the monitor is accepting other commands.
+#(see :doc:`/interop/qmp-spec`)
#
# .. note:: The QMP client needs to explicitly enable QMP capabilities,
#otherwise all the QMP capabilities will be turned off by default.
diff --git a/qapi/misc.json b/qapi/misc.json
index 13ea82f525..b04efbadec 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -104,7 +104,7 @@
# Returns a list of information about each iothread.
#
# .. note:: This list excludes the QEMU main loop thread, which is not
-#declared using the -object iothread command-line option. It is
+#declared using the ``-object iothread`` command-line option. It is
#always the main thread of the process.
#
# Returns: a list of @IOThreadInfo for each iothread
@@ -138,8 +138,8 @@
#
# .. note:: This function will succeed even if the guest is already in
#the stopped state. In "inmigrate" state, it will ensure that the
-#guest remains paused once migration finishes, as if the -S option
-#was passed on the command line.
+#guest remains paused once migration finishes, as if the ``-S``
+#option was passed on the command line.
#
#In the "suspended" state, it will completely stop the VM and cause
#a transition to the "paused" state. (Since 9.0)
@@ -161,7 +161,7 @@
# .. note:: This command will succeed if the guest is currently running.
#It will also succeed if the guest is in the "inmigrate" state; in
#this case, the effect of the command is to make sure the guest
-#starts once migration finishes, removing the effect of the -S
+#starts once migration finishes, removing the effect of the ``-S``
#command line option if it was passed.
#
#If the VM was previously suspended, and not been reset or woken,
diff --git a/qapi/qdev.json b/qapi/qdev.json
index f5b35a814f..d031fc3590 100644
--- a/qapi/qdev.json
+++ b/qapi/qdev.json
@@ -59,7 +59,7 @@
#the 'docs/qdev-device-use.txt' file.
#
# 3. It's possible to list device properties by running QEMU with
-#the "-device DEVICE,help" command-line argument, where DEVICE
+#the ``-device DEVICE,help`` command-line argument, where DEVICE
#is the device's name.
#
# Example:
diff --git a/qapi/run-state.json b/qapi/run-state.json
index 30cd25d3c9..4d40c88876 100644
--- a/qapi/run-state.json
+++ b/qapi/run-state.json
@@ -146,7 +146,7 @@
# @reason: The @ShutdownCause which resulted in the SHUTDOWN.
# (since 4.0)
#
-# .. note:: If the command-line option "-no-shutdown" has been
+# .. note:: If the command-line option ``-no-shutdown`` has been
#specified, qemu will not exit, and a STOP event will eventually
#follow the SHUTDOWN event.
#
diff --git a/qapi/sockets.json b/qapi/sockets.json
index 3970118bf4..4d78d2ccb7 100644
--- a/qapi/sockets.json
+++ b/qapi/sockets.json
@@ -181,7 +181,7 @@
#
# .. note:: This type is deprecated in favor of SocketAddress. The
#difference between SocketAddressLegacy and SocketAddress is that
-#the latter has fewer {} on the wire.
+#the latter has fewer ``{}`` on the wire.
#
# Since: 1.3
##
diff --git a/qapi/ui.json b/qapi/ui.json
index a165e4..5bcccbfc93 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1268,7 +1268,7 @@
# Since: 2.6
#
# .. note:: The consoles are visible in the qom tree, under
-#/backend/console[$index]. They have a device link and head
+#``/backend/console[$index]``. They have a device link and head
#property, so it is possible to map which console belongs to which
#device and display.
#
--
2.45.0
[PULL 01/13] qapi: linter fixups
From: John Snow
Fix minor irritants to pylint/flake8 et al.
(Yes, these need to be guarded by the Python tests. That's a work in
progress, a series that's quite likely to follow once I finish this
Sphinx project. Please pardon the temporary irritation.)
Signed-off-by: John Snow
Reviewed-by: Markus Armbruster
Message-ID: <20240626222128.406106-3-js...@redhat.com>
Signed-off-by: Markus Armbruster
---
scripts/qapi/introspect.py | 8
scripts/qapi/schema.py | 6 +++---
scripts/qapi/visit.py | 5 +++--
3 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index 86c075a6ad..ac14b20f30 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -27,8 +27,8 @@
from .schema import (
QAPISchema,
QAPISchemaAlternatives,
-QAPISchemaBranches,
QAPISchemaArrayType,
+QAPISchemaBranches,
QAPISchemaBuiltinType,
QAPISchemaEntity,
QAPISchemaEnumMember,
@@ -233,9 +233,9 @@ def _use_type(self, typ: QAPISchemaType) -> str:
typ = type_int
elif (isinstance(typ, QAPISchemaArrayType) and
typ.element_type.json_type() == 'int'):
-type_intList = self._schema.lookup_type('intList')
-assert type_intList
-typ = type_intList
+type_intlist = self._schema.lookup_type('intList')
+assert type_intlist
+typ = type_intlist
# Add type to work queue if new
if typ not in self._used_types:
self._used_types.append(typ)
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 721c470d2b..d65c35f6ee 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -730,6 +730,7 @@ def set_defined_in(self, name: str) -> None:
for v in self.variants:
v.set_defined_in(name)
+# pylint: disable=unused-argument
def check(
self, schema: QAPISchema, seen: Dict[str, QAPISchemaMember]
) -> None:
@@ -1166,7 +1167,7 @@ def _def_definition(self, defn: QAPISchemaDefinition) ->
None:
defn.info, "%s is already defined" % other_defn.describe())
self._entity_dict[defn.name] = defn
-def lookup_entity(self,name: str) -> Optional[QAPISchemaEntity]:
+def lookup_entity(self, name: str) -> Optional[QAPISchemaEntity]:
return self._entity_dict.get(name)
def lookup_type(self, name: str) -> Optional[QAPISchemaType]:
@@ -1302,11 +1303,10 @@ def _make_implicit_object_type(
name = 'q_obj_%s-%s' % (name, role)
typ = self.lookup_entity(name)
if typ:
-assert(isinstance(typ, QAPISchemaObjectType))
+assert isinstance(typ, QAPISchemaObjectType)
# The implicit object type has multiple users. This can
# only be a duplicate definition, which will be flagged
# later.
-pass
else:
self._def_definition(QAPISchemaObjectType(
name, info, None, ifcond, None, None, members, None))
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index e766acaac9..12f92e429f 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -280,8 +280,9 @@ def gen_visit_alternate(name: str,
abort();
default:
assert(visit_is_input(v));
-error_setg(errp, "Invalid parameter type for '%%s', expected:
%(name)s",
- name ? name : "null");
+error_setg(errp,
+ "Invalid parameter type for '%%s', expected: %(name)s",
+ name ? name : "null");
/* Avoid passing invalid *obj to qapi_free_%(c_name)s() */
g_free(*obj);
*obj = NULL;
--
2.45.0
[PULL 09/13] qapi: convert "Note" sections to plain rST
From: John Snow We do not need a dedicated section for notes. By eliminating a specially parsed section, these notes can be treated as normal rST paragraphs in the new QMP reference manual, and can be placed and styled much more flexibly. Convert all existing "Note" and "Notes" sections to pure rST. As part of the conversion, capitalize the first letter of each sentence and add trailing punctuation where appropriate to ensure notes look sensible and consistent in rendered HTML documentation. Markup is also re-aligned to the de-facto standard of 3 spaces for directives. Update docs/devel/qapi-code-gen.rst to reflect the new paradigm, and update the QAPI parser to prohibit "Note" sections while suggesting a new syntax. The exact formatting to use is a matter of taste, but a good candidate is simply: .. note:: lorem ipsum ... ... dolor sit amet ... ... consectetur adipiscing elit ... ... but there are other choices, too. The Sphinx readthedocs theme offers theming for the following forms (capitalization unimportant); all are adorned with a (!) symbol () in the title bar for rendered HTML docs. See https://sphinx-rtd-theme.readthedocs.io/en/stable/demo/demo.html#admonitions for examples of each directive/admonition in use. These are rendered in orange: .. Attention:: ... .. Caution:: ... .. WARNING:: ... These are rendered in red: .. DANGER:: ... .. Error:: ... These are rendered in green: .. Hint:: ... .. Important:: ... .. Tip:: ... These are rendered in blue: .. Note:: ... .. admonition:: custom title admonition body text This patch uses ".. note::" almost everywhere, with just two "caution" directives. Several instances of "Notes:" have been converted to merely ".. note::", or multiple ".. note::" where appropriate. ".. admonition:: notes" is used in a few places where we had an ordered list of multiple notes that would not make sense as standalone/separate admonitions. Two "Note:" following "Example:" have been turned into ordinary paragraphs within the example. NOTE: Because qapidoc.py does not attempt to preserve source ordering of sections, the conversion of Notes from a "tagged section" to an "untagged section" means that rendering order for some notes *may change* as a result of this patch. The forthcoming qapidoc.py rewrite strictly preserves source ordering in the rendered documentation, so this issue will be rectified in the new generator. Signed-off-by: John Snow Acked-by: Stefan Hajnoczi [for block*.json] Message-ID: <20240626222128.406106-11-js...@redhat.com> Reviewed-by: Markus Armbruster [Commit message clarified slightly, period added to one more note] Signed-off-by: Markus Armbruster --- docs/devel/qapi-code-gen.rst | 7 +- qapi/block-core.json | 28 +++--- qapi/block.json | 2 +- qapi/char.json| 12 +-- qapi/control.json | 17 ++-- qapi/dump.json| 2 +- qapi/introspect.json | 6 +- qapi/machine-target.json | 26 +++--- qapi/machine.json | 47 +- qapi/migration.json | 14 +-- qapi/misc.json| 88 +-- qapi/net.json | 6 +- qapi/pci.json | 8 +- qapi/qdev.json| 28 +++--- qapi/qom.json | 17 ++-- qapi/rocker.json | 16 ++-- qapi/run-state.json | 18 ++-- qapi/sockets.json | 10 +-- qapi/stats.json | 22 ++--- qapi/transaction.json | 8 +- qapi/ui.json | 29 +++--- qapi/virtio.json | 12 +-- qga/qapi-schema.json | 48 +- scripts/qapi/parser.py| 15 tests/qapi-schema/doc-empty-section.err | 2 +- tests/qapi-schema/doc-empty-section.json | 2 +- tests/qapi-schema/doc-good.json | 4 +- tests/qapi-schema/doc-good.out| 8 +- tests/qapi-schema/doc-good.txt| 10 +-- .../qapi-schema/doc-interleaved-section.json | 2 +- 30 files changed, 261 insertions(+), 253 deletions(-) diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst index cee43222f1..ae97b335cb 100644 --- a/docs/devel/qapi-code-gen.rst +++ b/docs/devel/qapi-code-gen.rst @@ -995,14 +995,13 @@ line "Features:", like this:: # @feature: Description text A tagged section begins with a paragraph that starts with one of the -following words: "Note:"/"Notes:
[PULL 00/13] QAPI patches patches for 2024-07-06
The following changes since commit f2cb4026fccfe073f84a4b440e41d3ed0c3134f6: Merge tag 'pull-maintainer-july24-050724-1' of https://gitlab.com/stsquad/qemu into staging (2024-07-05 09:15:48 -0700) are available in the Git repository at: https://repo.or.cz/qemu/armbru.git tags/pull-qapi-2024-07-06 for you to fetch changes up to e389929d19a543ea5b34d02553b355f9f1c03162: sphinx/qapidoc: Fix to generate doc for explicit, unboxed arguments (2024-07-06 08:58:24 +0200) QAPI patches patches for 2024-07-06 John Snow (12): qapi: linter fixups docs/qapidoc: remove unused intersperse function docs/qapidoc: delint a tiny portion of the module qapi/parser: preserve indentation in QAPIDoc sections qapi/parser: fix comment parsing immediately following a doc block docs/qapidoc: fix nested parsing under untagged sections qapi: fix non-compliant JSON examples qapi: nail down convention that Errors sections are lists qapi: convert "Note" sections to plain rST qapi: update prose in note blocks qapi: add markup to note blocks qapi/parser: don't parse rST markup as section headers Markus Armbruster (1): sphinx/qapidoc: Fix to generate doc for explicit, unboxed arguments docs/devel/qapi-code-gen.rst | 14 ++- docs/sphinx/qapidoc.py | 115 +++-- qapi/block-core.json | 35 +++- qapi/block.json| 2 +- qapi/char.json | 12 +-- qapi/control.json | 20 ++--- qapi/dump.json | 2 +- qapi/introspect.json | 6 +- qapi/machine-target.json | 26 +++--- qapi/machine.json | 49 +-- qapi/migration.json| 16 ++-- qapi/misc.json | 91 ++- qapi/net.json | 12 +-- qapi/pci.json | 8 +- qapi/qdev.json | 30 +++ qapi/qom.json | 17 ++-- qapi/rocker.json | 18 ++-- qapi/run-state.json| 18 ++-- qapi/sockets.json | 10 +-- qapi/stats.json| 22 ++--- qapi/transaction.json | 10 +-- qapi/ui.json | 31 --- qapi/virtio.json | 12 +-- qga/qapi-schema.json | 48 +-- scripts/qapi/introspect.py | 8 +- scripts/qapi/parser.py | 30 +-- scripts/qapi/schema.py | 6 +- scripts/qapi/visit.py | 5 +- tests/qapi-schema/doc-empty-section.err| 2 +- tests/qapi-schema/doc-empty-section.json | 2 +- tests/qapi-schema/doc-good.json| 9 +- tests/qapi-schema/doc-good.out | 43 + tests/qapi-schema/doc-good.txt | 13 ++- tests/qapi-schema/doc-interleaved-section.json | 2 +- 34 files changed, 397 insertions(+), 347 deletions(-) -- 2.45.0
[PULL 02/13] docs/qapidoc: remove unused intersperse function
From: John Snow This function has been unused since since commit fd62bff901b (sphinx/qapidoc: Drop code to generate doc for simple union tag). Signed-off-by: John Snow Message-ID: <20240626222128.406106-4-js...@redhat.com> Reviewed-by: Markus Armbruster [Commit message tweaked] Signed-off-by: Markus Armbruster --- docs/sphinx/qapidoc.py | 10 -- 1 file changed, 10 deletions(-) diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py index f270b494f0..3c0565d0ce 100644 --- a/docs/sphinx/qapidoc.py +++ b/docs/sphinx/qapidoc.py @@ -50,16 +50,6 @@ __version__ = '1.0' -# Function borrowed from pydash, which is under the MIT license -def intersperse(iterable, separator): -"""Yield the members of *iterable* interspersed with *separator*.""" -iterable = iter(iterable) -yield next(iterable) -for item in iterable: -yield separator -yield item - - class QAPISchemaGenRSTVisitor(QAPISchemaVisitor): """A QAPI schema visitor which generates docutils/Sphinx nodes -- 2.45.0
[PATCH] qapi/qom: Document feature unstable of @x-vfio-user-server
Commit 8f9a9259d32c added ObjectType member @x-vfio-user-server with feature unstable, but neglected to explain why it is unstable. Do that now. Fixes: 8f9a9259d32c (vfio-user: define vfio-user-server object) Cc: Elena Ufimtseva Cc: John G Johnson Cc: Jagannathan Raman Cc: qemu-sta...@nongnu.org Signed-off-by: Markus Armbruster --- qapi/qom.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/qapi/qom.json b/qapi/qom.json index 8bd299265e..3c0f8c633d 100644 --- a/qapi/qom.json +++ b/qapi/qom.json @@ -1012,7 +1012,8 @@ # # Features: # -# @unstable: Member @x-remote-object is experimental. +# @unstable: Members @x-remote-object and @x-vfio-user-server are +# experimental. # # Since: 6.0 ## -- 2.45.0
Re: [PATCH 0/2] docs/python: bump minimum Sphinx version
John Snow writes: > On Tue, Jul 2, 2024 at 3:59 PM John Snow wrote: > >> With recent deprecations, we can advance our minimum sphinx version >> safely. This is heavily motivated by new qapidoc work which is much >> easier to maintain cross-version compatibility for - see difficulties in >> our dbus documentation which only works on sphinx >= 4. >> >> We can only guarantee >= 3.4.3 now, but that's still vastly easier than >> maintaining compatibility all the way back to 1.x. >> >> GitLab: https://gitlab.com/jsnow/qemu/-/pipelines/1357902509 >> >> (failures appear to be unrelated to the series.) >> >> John Snow (2): >> Python: bump minimum sphinx version to 3.4.3 >> docs: remove Sphinx 1.x compatibility code >> >> docs/conf.py | 7 +++ >> docs/sphinx/hxtool.py| 21 - >> docs/sphinx/kerneldoc.py | 38 -- >> docs/sphinx/kernellog.py | 28 >> docs/sphinx/qapidoc.py | 29 +++-- >> pythondeps.toml | 2 +- >> 6 files changed, 23 insertions(+), 102 deletions(-) >> delete mode 100644 docs/sphinx/kernellog.py >> >> -- >> 2.45.0 >> > > Bleurgh. I meant to shuffle some of the changes in patch 1 into patch 2, I was wondering about the split :) > I'll fix that on re-spin. If you want to review it anyway, just presume > I'll fix that next go-around. Acked-by: Markus Armbruster
Re: [PATCH v2 10/21] qapi: convert "Note" sections to plain rST
John Snow writes: > We do not need a dedicated section for notes. By eliminating a specially > parsed section, these notes can be treated as normal rST paragraphs in > the new QMP reference manual, and can be placed and styled much more > flexibly. > > Convert all existing "Note" and "Notes" sections to pure rST. As part of > the conversion, capitalize the first letter of each sentence and add > trailing punctuation where appropriate to ensure notes look sensible and > consistent in rendered HTML documentation. Markup is also re-aligned to > the de-facto standard of 3 spaces for directives. > > Update docs/devel/qapi-code-gen.rst to reflect the new paradigm, and > update the QAPI parser to prohibit "Note" sections while suggesting a > new syntax. The exact formatting to use is a matter of taste, but a good > candidate is simply: > > .. note:: lorem ipsum ... >... dolor sit amet ... >... consectetur adipiscing elit ... > > ... but there are other choices, too. The Sphinx readthedocs theme > offers theming for the following forms (capitalization unimportant); all > are adorned with a (!) symbol () in the title bar for rendered HTML > docs. > > See > https://sphinx-rtd-theme.readthedocs.io/en/stable/demo/demo.html#admonitions > for examples of each directive/admonition in use. > > These are rendered in orange: > > .. Attention:: ... > .. Caution:: ... > .. WARNING:: ... > > These are rendered in red: > > .. DANGER:: ... > .. Error:: ... > > These are rendered in green: > > .. Hint:: ... > .. Important:: ... > .. Tip:: ... > > These are rendered in blue: > > .. Note:: ... > .. admonition:: custom title > >admonition body text > > This patch uses ".. note::" almost everywhere, with just two "caution" > directives. Several instances of "Notes:" have been converted to merely > ".. note::" where appropriate, but ".. admonition:: notes" is used in a > few places where we had an ordered list of multiple notes that would not > make sense as standalone/separate admonitions. > > NOTE: Because qapidoc.py does not attempt to preserve source ordering of > sections, the conversion of Notes from a "tagged section" to an > "untagged section" means that rendering order for some notes *may > change* as a result of this patch. The forthcoming qapidoc.py rewrite > strictly preserves source ordering in the rendered documentation, so > this issue will be rectified in the new generator. > > Signed-off-by: John Snow > Acked-by: Stefan Hajnoczi [for block*.json] [...] > diff --git a/qapi/migration.json b/qapi/migration.json > index 9ec9ef36c47..26ad5e5e7a3 100644 > --- a/qapi/migration.json > +++ b/qapi/migration.json > @@ -1456,8 +1456,8 @@ > # > # Cancel the current executing migration process. > # > -# Notes: This command succeeds even if there is no migration process > -# running. > +# .. note:: This command succeeds even if there is no migration process > +#running. > # > # Since: 0.14 > # > @@ -1589,16 +1589,16 @@ > # > # Since: 0.14 > # > -# Notes: > +# .. admonition:: Notes > # > # 1. The 'query-migrate' command should be used to check > #migration's progress and final result (this information is > #provided by the 'status' member) Missing period, will touch up in my tree. > # > -# 2. All boolean arguments default to false > +# 2. All boolean arguments default to false. > # > # 3. The user Monitor's "detach" argument is invalid in QMP and > -#should not be used > +#should not be used. > # > # 4. The uri argument should have the Uniform Resource Identifier > #of default destination VM. This connection will be bound to > @@ -1672,7 +1672,7 @@ > # > # Since: 2.3 > # > -# Notes: > +# .. admonition:: Notes > # > # 1. It's a bad idea to use a string for the uri, but it needs to > #stay compatible with -incoming and the format of the uri is [...]
